diff mbox

[FFmpeg-devel,1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file

Message ID 20180922215328.18053-1-cus@passwd.hu
State Accepted
Headers show

Commit Message

Marton Balint Sept. 22, 2018, 9:53 p.m. UTC
These are based on the very similar UDP and RTP protocol functions.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/ip.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libavformat/ip.h |  74 +++++++++++++++++++++++++
 2 files changed, 239 insertions(+)
 create mode 100644 libavformat/ip.c
 create mode 100644 libavformat/ip.h

Comments

Michael Niedermayer Sept. 23, 2018, 8:54 p.m. UTC | #1
On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote:
> These are based on the very similar UDP and RTP protocol functions.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/ip.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/ip.h |  74 +++++++++++++++++++++++++
>  2 files changed, 239 insertions(+)
>  create mode 100644 libavformat/ip.c
>  create mode 100644 libavformat/ip.h
[...]
> +/**
> + * Parses the address[,address] source list in buf and adds it to the filters
> + * in the IPSourceFilters structure.
> + * @param buf is the source list, which is not changed, but must be writable
> + * @return 0 on success, < 0 AVERROR code on error.
> + */
> +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters);
> +
> +/**
> + * Parses the address[,address] source block list in buf and adds it to the
> + * filters in the IPSourceFilters structure.
> + * @param buf is the source list, which is not changed, but must be writable

if its not changed, why does it need to be writable ?

thx

[...]
Marton Balint Sept. 23, 2018, 10:04 p.m. UTC | #2
On Sun, 23 Sep 2018, Michael Niedermayer wrote:

> On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote:
>> These are based on the very similar UDP and RTP protocol functions.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/ip.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  libavformat/ip.h |  74 +++++++++++++++++++++++++
>>  2 files changed, 239 insertions(+)
>>  create mode 100644 libavformat/ip.c
>>  create mode 100644 libavformat/ip.h
> [...]
>> +/**
>> + * Parses the address[,address] source list in buf and adds it to the filters
>> + * in the IPSourceFilters structure.
>> + * @param buf is the source list, which is not changed, but must be writable
>> + * @return 0 on success, < 0 AVERROR code on error.
>> + */
>> +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters);
>> +
>> +/**
>> + * Parses the address[,address] source block list in buf and adds it to the
>> + * filters in the IPSourceFilters structure.
>> + * @param buf is the source list, which is not changed, but must be writable
>
> if its not changed, why does it need to be writable ?

Becasue the during the parsing the ',' is replaced with '\0' temporarily.

Regards,
Marton
James Almer Sept. 23, 2018, 10:24 p.m. UTC | #3
On 9/23/2018 7:04 PM, Marton Balint wrote:
> 
> 
> On Sun, 23 Sep 2018, Michael Niedermayer wrote:
> 
>> On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote:
>>> These are based on the very similar UDP and RTP protocol functions.
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  libavformat/ip.c | 165
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  libavformat/ip.h |  74 +++++++++++++++++++++++++
>>>  2 files changed, 239 insertions(+)
>>>  create mode 100644 libavformat/ip.c
>>>  create mode 100644 libavformat/ip.h
>> [...]
>>> +/**
>>> + * Parses the address[,address] source list in buf and adds it to
>>> the filters
>>> + * in the IPSourceFilters structure.
>>> + * @param buf is the source list, which is not changed, but must be
>>> writable
>>> + * @return 0 on success, < 0 AVERROR code on error.
>>> + */
>>> +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters
>>> *filters);
>>> +
>>> +/**
>>> + * Parses the address[,address] source block list in buf and adds it
>>> to the
>>> + * filters in the IPSourceFilters structure.
>>> + * @param buf is the source list, which is not changed, but must be
>>> writable
>>
>> if its not changed, why does it need to be writable ?
> 
> Becasue the during the parsing the ',' is replaced with '\0' temporarily.

Why not use av_get_token() or similar instead, to split it into separate
strings? That way the buffer passed to the function can be const.

> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Marton Balint Sept. 23, 2018, 11:07 p.m. UTC | #4
On Sun, 23 Sep 2018, James Almer wrote:

> On 9/23/2018 7:04 PM, Marton Balint wrote:
>> 
>> 
>> On Sun, 23 Sep 2018, Michael Niedermayer wrote:
>> 
>>> On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote:
>>>> These are based on the very similar UDP and RTP protocol functions.
>>>>
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>>  libavformat/ip.c | 165
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  libavformat/ip.h |  74 +++++++++++++++++++++++++
>>>>  2 files changed, 239 insertions(+)
>>>>  create mode 100644 libavformat/ip.c
>>>>  create mode 100644 libavformat/ip.h
>>> [...]
>>>> +/**
>>>> + * Parses the address[,address] source list in buf and adds it to
>>>> the filters
>>>> + * in the IPSourceFilters structure.
>>>> + * @param buf is the source list, which is not changed, but must be
>>>> writable
>>>> + * @return 0 on success, < 0 AVERROR code on error.
>>>> + */
>>>> +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters
>>>> *filters);
>>>> +
>>>> +/**
>>>> + * Parses the address[,address] source block list in buf and adds it
>>>> to the
>>>> + * filters in the IPSourceFilters structure.
>>>> + * @param buf is the source list, which is not changed, but must be
>>>> writable
>>>
>>> if its not changed, why does it need to be writable ?
>> 
>> Becasue the during the parsing the ',' is replaced with '\0' temporarily.
>
> Why not use av_get_token() or similar instead, to split it into separate
> strings? That way the buffer passed to the function can be const.

That would cause some unnecesary mallocs/frees because we don't store the 
parsed address strings. Also this patch is mostly factorization, I 
tried to keep functional changes to the mimnium...

Can be changed if any of you still prefer that.

Regards,
Marton
Michael Niedermayer Sept. 23, 2018, 11:12 p.m. UTC | #5
On Mon, Sep 24, 2018 at 12:04:12AM +0200, Marton Balint wrote:
> 
> 
> On Sun, 23 Sep 2018, Michael Niedermayer wrote:
> 
> >On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote:
> >>These are based on the very similar UDP and RTP protocol functions.
> >>
> >>Signed-off-by: Marton Balint <cus@passwd.hu>
> >>---
> >> libavformat/ip.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> libavformat/ip.h |  74 +++++++++++++++++++++++++
> >> 2 files changed, 239 insertions(+)
> >> create mode 100644 libavformat/ip.c
> >> create mode 100644 libavformat/ip.h
> >[...]
> >>+/**
> >>+ * Parses the address[,address] source list in buf and adds it to the filters
> >>+ * in the IPSourceFilters structure.
> >>+ * @param buf is the source list, which is not changed, but must be writable
> >>+ * @return 0 on success, < 0 AVERROR code on error.
> >>+ */
> >>+int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters);
> >>+
> >>+/**
> >>+ * Parses the address[,address] source block list in buf and adds it to the
> >>+ * filters in the IPSourceFilters structure.
> >>+ * @param buf is the source list, which is not changed, but must be writable
> >
> >if its not changed, why does it need to be writable ?
> 
> Becasue the during the parsing the ',' is replaced with '\0' temporarily.

this implies that the buffer cannot be shared with another thread.
(which while unlikely could if its done result in very hard to reproduce
 bugs)
This should be at least documented more clearly but i think it would be best
to avoid these temporary writes as they are unexpected.
 

[...]
diff mbox

Patch

diff --git a/libavformat/ip.c b/libavformat/ip.c
new file mode 100644
index 0000000000..d88055ebda
--- /dev/null
+++ b/libavformat/ip.c
@@ -0,0 +1,165 @@ 
+/*
+ * IP common code
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with FFmpeg; if not, write to the Free Software * Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "ip.h"
+
+static int compare_addr(const struct sockaddr_storage *a,
+                        const struct sockaddr_storage *b)
+{
+    if (a->ss_family != b->ss_family)
+        return 1;
+    if (a->ss_family == AF_INET) {
+        return (((const struct sockaddr_in *)a)->sin_addr.s_addr !=
+                ((const struct sockaddr_in *)b)->sin_addr.s_addr);
+    }
+
+#if HAVE_STRUCT_SOCKADDR_IN6
+    if (a->ss_family == AF_INET6) {
+        const uint8_t *s6_addr_a = ((const struct sockaddr_in6 *)a)->sin6_addr.s6_addr;
+        const uint8_t *s6_addr_b = ((const struct sockaddr_in6 *)b)->sin6_addr.s6_addr;
+        return memcmp(s6_addr_a, s6_addr_b, 16);
+    }
+#endif
+    return 1;
+}
+
+int ff_ip_check_source_lists(struct sockaddr_storage *source_addr_ptr, IPSourceFilters *s)
+{
+    int i;
+    if (s->nb_exclude_addrs) {
+        for (i = 0; i < s->nb_exclude_addrs; i++) {
+            if (!compare_addr(source_addr_ptr, &s->exclude_addrs[i]))
+                return 1;
+        }
+    }
+    if (s->nb_include_addrs) {
+        for (i = 0; i < s->nb_include_addrs; i++) {
+            if (!compare_addr(source_addr_ptr, &s->include_addrs[i]))
+                return 0;
+        }
+        return 1;
+    }
+    return 0;
+}
+
+struct addrinfo *ff_ip_resolve_host(void *log_ctx,
+                                    const char *hostname, int port,
+                                    int type, int family, int flags)
+{
+    struct addrinfo hints = { 0 }, *res = 0;
+    int error;
+    char sport[16];
+    const char *node = 0, *service = "0";
+
+    if (port > 0) {
+        snprintf(sport, sizeof(sport), "%d", port);
+        service = sport;
+    }
+    if ((hostname) && (hostname[0] != '\0') && (hostname[0] != '?')) {
+        node = hostname;
+    }
+    hints.ai_socktype = type;
+    hints.ai_family   = family;
+    hints.ai_flags    = flags;
+    if ((error = getaddrinfo(node, service, &hints, &res))) {
+        res = NULL;
+        av_log(log_ctx, AV_LOG_ERROR, "getaddrinfo(%s, %s): %s\n",
+               node ? node : "unknown",
+               service,
+               gai_strerror(error));
+    }
+
+    return res;
+}
+
+
+static int ip_parse_addr_list(void *log_ctx, char *buf,
+                              struct sockaddr_storage **address_list_ptr,
+                              int *address_list_size_ptr)
+{
+    struct addrinfo *ai = NULL;
+    char tmp = '\0', *p = buf, *next;
+
+    /* Resolve all of the IPs */
+
+    while (p && p[0]) {
+        next = strchr(p, ',');
+
+        if (next) {
+            tmp = *next;
+            *next = '\0';
+        }
+
+        ai = ff_ip_resolve_host(log_ctx, p, 0, SOCK_DGRAM, AF_UNSPEC, 0);
+
+        if (next) {
+            *next = tmp;
+            p = next + 1;
+        } else {
+            p = NULL;
+        }
+
+        if (ai) {
+            struct sockaddr_storage source_addr = {0};
+            memcpy(&source_addr, ai->ai_addr, ai->ai_addrlen);
+            freeaddrinfo(ai);
+            av_dynarray2_add((void **)address_list_ptr, address_list_size_ptr, sizeof(source_addr), (uint8_t *)&source_addr);
+            if (!*address_list_ptr)
+                return AVERROR(ENOMEM);
+        } else {
+            return AVERROR(EINVAL);
+        }
+    }
+
+    return 0;
+}
+
+static int ip_parse_sources_and_blocks(void *log_ctx, char *buf, IPSourceFilters *filters, int parse_include_list)
+{
+    int ret;
+    if (parse_include_list)
+        ret = ip_parse_addr_list(log_ctx, buf, &filters->include_addrs, &filters->nb_include_addrs);
+    else
+        ret = ip_parse_addr_list(log_ctx, buf, &filters->exclude_addrs, &filters->nb_exclude_addrs);
+
+    if (ret >= 0 && filters->nb_include_addrs && filters->nb_exclude_addrs) {
+        av_log(log_ctx, AV_LOG_ERROR, "Simultaneously including and excluding sources is not supported.\n");
+        return AVERROR(EINVAL);
+    }
+    return ret;
+}
+
+int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters)
+{
+    return ip_parse_sources_and_blocks(log_ctx, buf, filters, 1);
+}
+
+int ff_ip_parse_blocks(void *log_ctx, char *buf, IPSourceFilters *filters)
+{
+    return ip_parse_sources_and_blocks(log_ctx, buf, filters, 0);
+}
+
+void ff_ip_reset_filters(IPSourceFilters *filters)
+{
+    av_freep(&filters->exclude_addrs);
+    av_freep(&filters->include_addrs);
+    filters->nb_include_addrs = 0;
+    filters->nb_exclude_addrs = 0;
+}
diff --git a/libavformat/ip.h b/libavformat/ip.h
new file mode 100644
index 0000000000..380143ff58
--- /dev/null
+++ b/libavformat/ip.h
@@ -0,0 +1,74 @@ 
+/*
+ * IP common code
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with FFmpeg; if not, write to the Free Software * Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVFORMAT_IP_H
+#define AVFORMAT_IP_H
+
+#include "network.h"
+
+/**
+ * Structure for storing IP (UDP) source filters or block lists.
+ */
+typedef struct IPSourceFilters {
+    int nb_include_addrs;
+    int nb_exclude_addrs;
+    struct sockaddr_storage *include_addrs;
+    struct sockaddr_storage *exclude_addrs;
+} IPSourceFilters;
+
+/**
+ * Checks the source address against a given IP source filter.
+ * @return 0 if packet should be processed based on the filter, 1 if the packet
+ *         can be dropped.
+ */
+int ff_ip_check_source_lists(struct sockaddr_storage *source_addr_ptr, IPSourceFilters *s);
+
+/**
+ * Resolves hostname into an addrinfo structure.
+ * @return addrinfo structure which should be freed by the user, NULL in case
+ *         of error.
+ */
+struct addrinfo *ff_ip_resolve_host(void *log_ctx,
+                                    const char *hostname, int port,
+                                    int type, int family, int flags);
+
+/**
+ * Parses the address[,address] source list in buf and adds it to the filters
+ * in the IPSourceFilters structure.
+ * @param buf is the source list, which is not changed, but must be writable
+ * @return 0 on success, < 0 AVERROR code on error.
+ */
+int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters);
+
+/**
+ * Parses the address[,address] source block list in buf and adds it to the
+ * filters in the IPSourceFilters structure.
+ * @param buf is the source list, which is not changed, but must be writable
+ * @return 0 on success, < 0 AVERROR code on error.
+ */
+int ff_ip_parse_blocks(void *log_ctx, char *buf, IPSourceFilters *filters);
+
+/**
+ * Resets the IP filter list and frees the internal fields of an
+ * IPSourceFilters structure.
+ */
+void ff_ip_reset_filters(IPSourceFilters *filters);
+
+#endif /* AVFORMAT_IP_H */