Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-29 Thread Philip Prindeville

> On Sep 29, 2017, at 3:49 AM, Matthias May  wrote:
> 
> The link from Philip Prindeville shows quite well why this removal was 
> required:
> [quote]
> check-response-ttl= Takes a boolean value ("yes" or "no"). If set to "yes", 
> an additional security check is activated:
> incoming IP packets will be ignored unless the IP TTL is 255. Earlier mDNS 
> specifications required this check. Since
> this feature may be incompatible with newer implementations of mDNS it 
> defaults to "no". On the other hand it provides
> extra security.
> [/quote]


Not actually my quote.

I think it was Christian.



___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-29 Thread Bjørn Mork
Matthias May  writes:

> While unfortunate that the actual patch which got merged didn't have the 
> explanation why the patch was done, if you look
> at the mailing list archive you will see that there was a thread discussing 
> this topic:
> http://lists.infradead.org/pipermail/lede-dev/2017-September/009004.html

This fails to discuss the reason that TTL restriction was there in the
first place, as well as any security implications of the change.

Please see
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-6520
https://www.kb.cert.org/vuls/id/550620
and more.  This is a well known can of worms.

As said before: You should disuss such issues with your proposed
patches.  Not doing so gives the impression that you either

 a) don't understand the implications, or
 b) don't care about security

I hope neither is true.  Please reassure me by fixing this up.


Bjørn

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-29 Thread Matthias May
On 29/09/17 10:28, Syrone Wong wrote:
> The sad truth is it has been pushed via
> https://github.com/lede-project/source/commit/00e9a7aacb66b3f00df2002e8210bdb5086d2e0c
> 
> 
> Best Regards,
> Syrone Wong
> 
> 
> On Fri, Sep 29, 2017 at 3:52 PM, Bjørn Mork  wrote:
>> Note that security is the usual (only?) reason one would enforce TTL=255.
>> Requiring TTL=255 is the same as guaranteeing that the packet source is
>> in the same L2 domain.  This prevents any direct remote attack.
>>
>> Please do not propose any patches removing such a restriction without at
>> least explaining why this can be done without negative security
>> implications. Thanks
>>
>>
>>
>> Bjørn
>>
>> ___
>> Lede-dev mailing list
>> Lede-dev@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev
> 
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
> 

Why are you sad that this got merged?
It fixes compatibility with current implementations of mDNS.

The link from Philip Prindeville shows quite well why this removal was required:
[quote]
check-response-ttl= Takes a boolean value ("yes" or "no"). If set to "yes", an 
additional security check is activated:
incoming IP packets will be ignored unless the IP TTL is 255. Earlier mDNS 
specifications required this check. Since
this feature may be incompatible with newer implementations of mDNS it defaults 
to "no". On the other hand it provides
extra security.
[/quote]

Since most people update their distributions, thus have a "newer implementation 
of mDNS", umdns was kind of broken in
this regards.

While unfortunate that the actual patch which got merged didn't have the 
explanation why the patch was done, if you look
at the mailing list archive you will see that there was a thread discussing 
this topic:
http://lists.infradead.org/pipermail/lede-dev/2017-September/009004.html

The restriction of IP TTL == 255 applies to responses, but not to queries.
See RFC6762 chapter 11 for more.

BR
Matthias

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-29 Thread Syrone Wong
The sad truth is it has been pushed via
https://github.com/lede-project/source/commit/00e9a7aacb66b3f00df2002e8210bdb5086d2e0c


Best Regards,
Syrone Wong


On Fri, Sep 29, 2017 at 3:52 PM, Bjørn Mork  wrote:
> Note that security is the usual (only?) reason one would enforce TTL=255.
> Requiring TTL=255 is the same as guaranteeing that the packet source is
> in the same L2 domain.  This prevents any direct remote attack.
>
> Please do not propose any patches removing such a restriction without at
> least explaining why this can be done without negative security
> implications. Thanks
>
>
>
> Bjørn
>
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-29 Thread Bjørn Mork
Note that security is the usual (only?) reason one would enforce TTL=255.
Requiring TTL=255 is the same as guaranteeing that the packet source is
in the same L2 domain.  This prevents any direct remote attack.

Please do not propose any patches removing such a restriction without at
least explaining why this can be done without negative security
implications. Thanks



Bjørn

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-28 Thread Philip Prindeville


> On Sep 28, 2017, at 2:32 PM, Christian Lamparter  
> wrote:
> 
>> On Thursday, September 28, 2017 1:36:52 PM CEST Philip Prindeville wrote:
>> Why was this test there and equally why are we removing it?
> I guess it was there so umdns would ignore any forwarded mdns?
> This would stop two mDNS reflectors to ping-pong each other. 
> The avahi-daemon manpages speaks about this in:
> 
> 

So why remove it in that case?


> 
> Regards,
> Christian
> 
>> 
>>> On Sep 28, 2017, at 1:09 AM, Philipp Meier  
>>> wrote:
>>> 
>>> Signed-off-by: Philipp Meier 
>>> ---
>>> interface.c | 6 --
>>> 1 file changed, 6 deletions(-)
>>> 
>>> diff --git a/interface.c b/interface.c
>>> index 3904c89..7f814d2 100644
>>> --- a/interface.c
>>> +++ b/interface.c
>>> @@ -233,9 +233,6 @@ read_socket4(struct uloop_fd *u, unsigned int events)
>>>   }
>>>   }
>>> 
>>> -if (ttl != 255)
>>> -return;
>>> -
>>>   if (debug > 1) {
>>>   char buf[256];
>>> 
>>> @@ -310,9 +307,6 @@ read_socket6(struct uloop_fd *u, unsigned int events)
>>>   }
>>>   }
>>> 
>>> -if (ttl != 255)
>>> -return;
>>> -
>>>   if (debug > 1) {
>>>   char buf[256];
>>> 
>> 
>> 
>> ___
>> Lede-dev mailing list
>> Lede-dev@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev
>> 
> 
> 


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-28 Thread Christian Lamparter via Lede-dev
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
On Thursday, September 28, 2017 1:36:52 PM CEST Philip Prindeville wrote:
> Why was this test there and equally why are we removing it?
I guess it was there so umdns would ignore any forwarded mdns?
This would stop two mDNS reflectors to ping-pong each other. 
The avahi-daemon manpages speaks about this in:



Regards,
Christian

> 
> > On Sep 28, 2017, at 1:09 AM, Philipp Meier  
> > wrote:
> > 
> > Signed-off-by: Philipp Meier 
> > ---
> > interface.c | 6 --
> > 1 file changed, 6 deletions(-)
> > 
> > diff --git a/interface.c b/interface.c
> > index 3904c89..7f814d2 100644
> > --- a/interface.c
> > +++ b/interface.c
> > @@ -233,9 +233,6 @@ read_socket4(struct uloop_fd *u, unsigned int events)
> >}
> >}
> > 
> > -if (ttl != 255)
> > -return;
> > -
> >if (debug > 1) {
> >char buf[256];
> > 
> > @@ -310,9 +307,6 @@ read_socket6(struct uloop_fd *u, unsigned int events)
> >}
> >}
> > 
> > -if (ttl != 255)
> > -return;
> > -
> >if (debug > 1) {
> >char buf[256];
> > 
> 
> 
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
> 



--- End Message ---
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-28 Thread Philip Prindeville
Why was this test there and equally why are we removing it?

> On Sep 28, 2017, at 1:09 AM, Philipp Meier  wrote:
> 
> Signed-off-by: Philipp Meier 
> ---
> interface.c | 6 --
> 1 file changed, 6 deletions(-)
> 
> diff --git a/interface.c b/interface.c
> index 3904c89..7f814d2 100644
> --- a/interface.c
> +++ b/interface.c
> @@ -233,9 +233,6 @@ read_socket4(struct uloop_fd *u, unsigned int events)
>}
>}
> 
> -if (ttl != 255)
> -return;
> -
>if (debug > 1) {
>char buf[256];
> 
> @@ -310,9 +307,6 @@ read_socket6(struct uloop_fd *u, unsigned int events)
>}
>}
> 
> -if (ttl != 255)
> -return;
> -
>if (debug > 1) {
>char buf[256];
> 
> -- 
> 2.7.4
> 
> 
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


[LEDE-DEV] [PATCH 1/3] Remove ttl==255 restriction for queries

2017-09-28 Thread Philipp Meier
Signed-off-by: Philipp Meier 
---
 interface.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/interface.c b/interface.c
index 3904c89..7f814d2 100644
--- a/interface.c
+++ b/interface.c
@@ -233,9 +233,6 @@ read_socket4(struct uloop_fd *u, unsigned int events)
}
}
 
-   if (ttl != 255)
-   return;
-
if (debug > 1) {
char buf[256];
 
@@ -310,9 +307,6 @@ read_socket6(struct uloop_fd *u, unsigned int events)
}
}
 
-   if (ttl != 255)
-   return;
-
if (debug > 1) {
char buf[256];
 
-- 
2.7.4


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev