Hi,
I am reporting a regression with respect to use of TCP_MD5SIG/TCP_MD5SIG_EXT
on established sockets. It is observed by a customer.
This issue is introduced by this commit:
commit 721230326891 "tcp: md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on established sockets"
The intent of this commit appears to be to fix a use of uninitialized value in
tcp_parse_options(). The change introduced by this commit is to disallow setting
the TCP_MD5SIG{,_EXT} socket options on an established socket.
The justification for this change appears in the commit message:
"I believe this was caused by a TCP_MD5SIG being set on live
flow.
This is highly unexpected, since TCP option space is limited.
For instance, presence of TCP MD5 option automatically disables
TCP TimeStamp option at SYN/SYNACK time, which we can not do
once flow has been established.
Really, adding/deleting an MD5 key only makes sense on sockets
in CLOSE or LISTEN state."
However, reading through RFC2385 [1], this justification does not appear
correct. Quoting to the RFC:
"This password never appears in the connection stream, and the actual
form of the password is up to the application. It could even change
during the lifetime of a particular connection so long as this change
was synchronized on both ends"
The paragraph above clearly underlines that changing the MD5 signature of
a live TCP socket is allowed.
I also do not understand why it would be invalid to transition an established
TCP socket from no-MD5 to MD5, or transition from MD5 to no-MD5. Quoting the
RFC:
"The total header size is also an issue. The TCP header specifies
where segment data starts with a 4-bit field which gives the total
size of the header (including options) in 32-byte words. This means
that the total size of the header plus option must be less than or
equal to 60 bytes -- this leaves 40 bytes for options."
The paragraph above seems to be the only indication that some TCP options
cannot be combined on a given TCP socket: if the resulting header size does
not fit. However, I do not see anything in the specification preventing any
of the following use-cases on an established TCP socket:
- Transition from no-MD5 to MD5,
- Transition from MD5 to no-MD5,
- Changing the MD5 key associated with a socket.
As long as the resulting combination of options does not exceed the available
header space.
Can we please fix this KASAN report in a way that does not break user-space
applications expectations about Linux' implementation of RFC2385 ?
Thanks,
Mathieu
[1] RFC2385: https://tools.ietf.org/html/rfc2385
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
I do not think we want to transition sockets in the middle. since
packets can be re-ordered in the network.
MD5 is about security (and a loose form of it), so better make sure
all packets have it from the beginning of the flow.
A flow with TCP TS on can not suddenly be sending packets without TCP TS.
Clearly, trying to support this operation is a can of worms, I do not
want to maintain such atrocity.
RFC can state whatever it wants, sometimes reality forces us to have
sane operations.
Thanks.
On Wed, May 13, 2020 at 12:38 PM Mathieu Desnoyers
<[email protected]> wrote:
>
> Hi,
>
> I am reporting a regression with respect to use of TCP_MD5SIG/TCP_MD5SIG_EXT
> on established sockets. It is observed by a customer.
>
> This issue is introduced by this commit:
>
> commit 721230326891 "tcp: md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on established sockets"
>
> The intent of this commit appears to be to fix a use of uninitialized value in
> tcp_parse_options(). The change introduced by this commit is to disallow setting
> the TCP_MD5SIG{,_EXT} socket options on an established socket.
>
> The justification for this change appears in the commit message:
>
> "I believe this was caused by a TCP_MD5SIG being set on live
> flow.
>
> This is highly unexpected, since TCP option space is limited.
>
> For instance, presence of TCP MD5 option automatically disables
> TCP TimeStamp option at SYN/SYNACK time, which we can not do
> once flow has been established.
>
> Really, adding/deleting an MD5 key only makes sense on sockets
> in CLOSE or LISTEN state."
>
> However, reading through RFC2385 [1], this justification does not appear
> correct. Quoting to the RFC:
>
> "This password never appears in the connection stream, and the actual
> form of the password is up to the application. It could even change
> during the lifetime of a particular connection so long as this change
> was synchronized on both ends"
>
> The paragraph above clearly underlines that changing the MD5 signature of
> a live TCP socket is allowed.
>
> I also do not understand why it would be invalid to transition an established
> TCP socket from no-MD5 to MD5, or transition from MD5 to no-MD5. Quoting the
> RFC:
>
> "The total header size is also an issue. The TCP header specifies
> where segment data starts with a 4-bit field which gives the total
> size of the header (including options) in 32-byte words. This means
> that the total size of the header plus option must be less than or
> equal to 60 bytes -- this leaves 40 bytes for options."
>
> The paragraph above seems to be the only indication that some TCP options
> cannot be combined on a given TCP socket: if the resulting header size does
> not fit. However, I do not see anything in the specification preventing any
> of the following use-cases on an established TCP socket:
>
> - Transition from no-MD5 to MD5,
> - Transition from MD5 to no-MD5,
> - Changing the MD5 key associated with a socket.
>
> As long as the resulting combination of options does not exceed the available
> header space.
>
> Can we please fix this KASAN report in a way that does not break user-space
> applications expectations about Linux' implementation of RFC2385 ?
>
> Thanks,
>
> Mathieu
>
> [1] RFC2385: https://tools.ietf.org/html/rfc2385
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
On Wed, May 13, 2020 at 12:49 PM Eric Dumazet <[email protected]> wrote:
>
> I do not think we want to transition sockets in the middle. since
> packets can be re-ordered in the network.
>
> MD5 is about security (and a loose form of it), so better make sure
> all packets have it from the beginning of the flow.
>
> A flow with TCP TS on can not suddenly be sending packets without TCP TS.
>
> Clearly, trying to support this operation is a can of worms, I do not
> want to maintain such atrocity.
>
> RFC can state whatever it wants, sometimes reality forces us to have
> sane operations.
>
> Thanks.
Also the RFC states :
"This password never appears in the connection stream, and the actual
form of the password is up to the application. It could even change
during the lifetime of a particular connection so long as this change
was synchronized on both ends"
It means the key can be changed, but this does not imply the option
can be turned on/off dynamically.
>
> On Wed, May 13, 2020 at 12:38 PM Mathieu Desnoyers
> <[email protected]> wrote:
> >
> > Hi,
> >
> > I am reporting a regression with respect to use of TCP_MD5SIG/TCP_MD5SIG_EXT
> > on established sockets. It is observed by a customer.
> >
> > This issue is introduced by this commit:
> >
> > commit 721230326891 "tcp: md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on established sockets"
> >
> > The intent of this commit appears to be to fix a use of uninitialized value in
> > tcp_parse_options(). The change introduced by this commit is to disallow setting
> > the TCP_MD5SIG{,_EXT} socket options on an established socket.
> >
> > The justification for this change appears in the commit message:
> >
> > "I believe this was caused by a TCP_MD5SIG being set on live
> > flow.
> >
> > This is highly unexpected, since TCP option space is limited.
> >
> > For instance, presence of TCP MD5 option automatically disables
> > TCP TimeStamp option at SYN/SYNACK time, which we can not do
> > once flow has been established.
> >
> > Really, adding/deleting an MD5 key only makes sense on sockets
> > in CLOSE or LISTEN state."
> >
> > However, reading through RFC2385 [1], this justification does not appear
> > correct. Quoting to the RFC:
> >
> > "This password never appears in the connection stream, and the actual
> > form of the password is up to the application. It could even change
> > during the lifetime of a particular connection so long as this change
> > was synchronized on both ends"
> >
> > The paragraph above clearly underlines that changing the MD5 signature of
> > a live TCP socket is allowed.
> >
> > I also do not understand why it would be invalid to transition an established
> > TCP socket from no-MD5 to MD5, or transition from MD5 to no-MD5. Quoting the
> > RFC:
> >
> > "The total header size is also an issue. The TCP header specifies
> > where segment data starts with a 4-bit field which gives the total
> > size of the header (including options) in 32-byte words. This means
> > that the total size of the header plus option must be less than or
> > equal to 60 bytes -- this leaves 40 bytes for options."
> >
> > The paragraph above seems to be the only indication that some TCP options
> > cannot be combined on a given TCP socket: if the resulting header size does
> > not fit. However, I do not see anything in the specification preventing any
> > of the following use-cases on an established TCP socket:
> >
> > - Transition from no-MD5 to MD5,
> > - Transition from MD5 to no-MD5,
> > - Changing the MD5 key associated with a socket.
> >
> > As long as the resulting combination of options does not exceed the available
> > header space.
> >
> > Can we please fix this KASAN report in a way that does not break user-space
> > applications expectations about Linux' implementation of RFC2385 ?
> >
> > Thanks,
> >
> > Mathieu
> >
> > [1] RFC2385: https://tools.ietf.org/html/rfc2385
> >
> > --
> > Mathieu Desnoyers
> > EfficiOS Inc.
> > http://www.efficios.com
----- On May 13, 2020, at 3:56 PM, Eric Dumazet [email protected] wrote:
> On Wed, May 13, 2020 at 12:49 PM Eric Dumazet <[email protected]> wrote:
>>
>>
>> On Wed, May 13, 2020 at 12:38 PM Mathieu Desnoyers
>> <[email protected]> wrote:
>> >
>> > Hi,
>> >
>> > I am reporting a regression with respect to use of TCP_MD5SIG/TCP_MD5SIG_EXT
>> > on established sockets. It is observed by a customer.
>> >
>> > This issue is introduced by this commit:
>> >
>> > commit 721230326891 "tcp: md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on
>> > established sockets"
>> >
>> > The intent of this commit appears to be to fix a use of uninitialized value in
>> > tcp_parse_options(). The change introduced by this commit is to disallow setting
>> > the TCP_MD5SIG{,_EXT} socket options on an established socket.
>> >
>> > The justification for this change appears in the commit message:
>> >
>> > "I believe this was caused by a TCP_MD5SIG being set on live
>> > flow.
>> >
>> > This is highly unexpected, since TCP option space is limited.
>> >
>> > For instance, presence of TCP MD5 option automatically disables
>> > TCP TimeStamp option at SYN/SYNACK time, which we can not do
>> > once flow has been established.
>> >
>> > Really, adding/deleting an MD5 key only makes sense on sockets
>> > in CLOSE or LISTEN state."
>> >
>> > However, reading through RFC2385 [1], this justification does not appear
>> > correct. Quoting to the RFC:
>> >
>> > "This password never appears in the connection stream, and the actual
>> > form of the password is up to the application. It could even change
>> > during the lifetime of a particular connection so long as this change
>> > was synchronized on both ends"
>> >
>> > The paragraph above clearly underlines that changing the MD5 signature of
>> > a live TCP socket is allowed.
>> >
>> > I also do not understand why it would be invalid to transition an established
>> > TCP socket from no-MD5 to MD5, or transition from MD5 to no-MD5. Quoting the
>> > RFC:
>> >
>> > "The total header size is also an issue. The TCP header specifies
>> > where segment data starts with a 4-bit field which gives the total
>> > size of the header (including options) in 32-byte words. This means
>> > that the total size of the header plus option must be less than or
>> > equal to 60 bytes -- this leaves 40 bytes for options."
>> >
>> > The paragraph above seems to be the only indication that some TCP options
>> > cannot be combined on a given TCP socket: if the resulting header size does
>> > not fit. However, I do not see anything in the specification preventing any
>> > of the following use-cases on an established TCP socket:
>> >
>> > - Transition from no-MD5 to MD5,
>> > - Transition from MD5 to no-MD5,
>> > - Changing the MD5 key associated with a socket.
>> >
>> > As long as the resulting combination of options does not exceed the available
>> > header space.
>> >
>> > Can we please fix this KASAN report in a way that does not break user-space
>> > applications expectations about Linux' implementation of RFC2385 ?
[...]
>> > [1] RFC2385: https://tools.ietf.org/html/rfc2385
>>
>>
>> I do not think we want to transition sockets in the middle. since
>> packets can be re-ordered in the network.
>>
>> MD5 is about security (and a loose form of it), so better make sure
>> all packets have it from the beginning of the flow.
>>
>> A flow with TCP TS on can not suddenly be sending packets without TCP TS.
>>
>> Clearly, trying to support this operation is a can of worms, I do not
>> want to maintain such atrocity.
>>
>> RFC can state whatever it wants, sometimes reality forces us to have
>> sane operations.
>>
>> Thanks.
>>
> Also the RFC states :
>
> "This password never appears in the connection stream, and the actual
> form of the password is up to the application. It could even change
> during the lifetime of a particular connection so long as this change
> was synchronized on both ends"
>
> It means the key can be changed, but this does not imply the option
> can be turned on/off dynamically.
>
The change discussed previously (introduced by commit 721230326891 "tcp:
md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on established sockets") breaks
user-space ABI. As an example, the following BGP application uses
setsockopt TCP_MD5SIG on a live TCP socket:
https://github.com/IPInfusion/SDN-IP
In addition to break user-space, it also breaks network protocol
expectations for network equipment vendors implementing RFC2385.
Considering that the goal of these protocols is interaction between
different network equipment, breaking compatibility on that side
is unexpected as well. Requiring to bring down/up the connection
just to change the TCP MD5 password is a no-go in networks with
high availability requirements. Changing the BGP authentication
password must be allowed without tearing down and re-establishing
the TCP sockets. Otherwise it doesn't scale for large network
operators to have to individually manage each individual TCP socket
in their network. However, based on the feedback I received, it
would be acceptable to tear-down the TCP connections and re-establish
them when enabling or disabling the MD5 option.
Here is a list of a few network vendors along with their behavior
with respect to TCP MD5:
- Cisco: Allows for password to be changed, but within the hold-down
timer (~180 seconds).
- Juniper: When password is initially set on active connection it will
reset, but after that any subsequent password changes no network
resets.
- Nokia: No notes on if they flap the tcp connection or not.
- Ericsson/RedBack: Allows for 2 password (old/new) to co-exist until
both sides are ok with new passwords.
- Meta-Switch: Expects the password to be set before a connection is
attempted, but no further info on whether they reset the TCP
connection on a change.
- Avaya: Disable the neighbor, then set password, then re-enable.
- Zebos: Would normally allow the change when socket connected.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
On Mon, Jun 29, 2020 at 12:43 PM Mathieu Desnoyers
<[email protected]> wrote:
>
> ----- On May 13, 2020, at 3:56 PM, Eric Dumazet [email protected] wrote:
>
> > On Wed, May 13, 2020 at 12:49 PM Eric Dumazet <[email protected]> wrote:
> >>
> >>
> >> On Wed, May 13, 2020 at 12:38 PM Mathieu Desnoyers
> >> <[email protected]> wrote:
> >> >
> >> > Hi,
> >> >
> >> > I am reporting a regression with respect to use of TCP_MD5SIG/TCP_MD5SIG_EXT
> >> > on established sockets. It is observed by a customer.
> >> >
> >> > This issue is introduced by this commit:
> >> >
> >> > commit 721230326891 "tcp: md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on
> >> > established sockets"
> >> >
> >> > The intent of this commit appears to be to fix a use of uninitialized value in
> >> > tcp_parse_options(). The change introduced by this commit is to disallow setting
> >> > the TCP_MD5SIG{,_EXT} socket options on an established socket.
> >> >
> >> > The justification for this change appears in the commit message:
> >> >
> >> > "I believe this was caused by a TCP_MD5SIG being set on live
> >> > flow.
> >> >
> >> > This is highly unexpected, since TCP option space is limited.
> >> >
> >> > For instance, presence of TCP MD5 option automatically disables
> >> > TCP TimeStamp option at SYN/SYNACK time, which we can not do
> >> > once flow has been established.
> >> >
> >> > Really, adding/deleting an MD5 key only makes sense on sockets
> >> > in CLOSE or LISTEN state."
> >> >
> >> > However, reading through RFC2385 [1], this justification does not appear
> >> > correct. Quoting to the RFC:
> >> >
> >> > "This password never appears in the connection stream, and the actual
> >> > form of the password is up to the application. It could even change
> >> > during the lifetime of a particular connection so long as this change
> >> > was synchronized on both ends"
> >> >
> >> > The paragraph above clearly underlines that changing the MD5 signature of
> >> > a live TCP socket is allowed.
> >> >
> >> > I also do not understand why it would be invalid to transition an established
> >> > TCP socket from no-MD5 to MD5, or transition from MD5 to no-MD5. Quoting the
> >> > RFC:
> >> >
> >> > "The total header size is also an issue. The TCP header specifies
> >> > where segment data starts with a 4-bit field which gives the total
> >> > size of the header (including options) in 32-byte words. This means
> >> > that the total size of the header plus option must be less than or
> >> > equal to 60 bytes -- this leaves 40 bytes for options."
> >> >
> >> > The paragraph above seems to be the only indication that some TCP options
> >> > cannot be combined on a given TCP socket: if the resulting header size does
> >> > not fit. However, I do not see anything in the specification preventing any
> >> > of the following use-cases on an established TCP socket:
> >> >
> >> > - Transition from no-MD5 to MD5,
> >> > - Transition from MD5 to no-MD5,
> >> > - Changing the MD5 key associated with a socket.
> >> >
> >> > As long as the resulting combination of options does not exceed the available
> >> > header space.
> >> >
> >> > Can we please fix this KASAN report in a way that does not break user-space
> >> > applications expectations about Linux' implementation of RFC2385 ?
> [...]
> >> > [1] RFC2385: https://tools.ietf.org/html/rfc2385
> >>
> >>
> >> I do not think we want to transition sockets in the middle. since
> >> packets can be re-ordered in the network.
> >>
> >> MD5 is about security (and a loose form of it), so better make sure
> >> all packets have it from the beginning of the flow.
> >>
> >> A flow with TCP TS on can not suddenly be sending packets without TCP TS.
> >>
> >> Clearly, trying to support this operation is a can of worms, I do not
> >> want to maintain such atrocity.
> >>
> >> RFC can state whatever it wants, sometimes reality forces us to have
> >> sane operations.
> >>
> >> Thanks.
> >>
> > Also the RFC states :
> >
> > "This password never appears in the connection stream, and the actual
> > form of the password is up to the application. It could even change
> > during the lifetime of a particular connection so long as this change
> > was synchronized on both ends"
> >
> > It means the key can be changed, but this does not imply the option
> > can be turned on/off dynamically.
> >
>
> The change discussed previously (introduced by commit 721230326891 "tcp:
> md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on established sockets") breaks
> user-space ABI. As an example, the following BGP application uses
> setsockopt TCP_MD5SIG on a live TCP socket:
>
> https://github.com/IPInfusion/SDN-IP
>
> In addition to break user-space, it also breaks network protocol
> expectations for network equipment vendors implementing RFC2385.
> Considering that the goal of these protocols is interaction between
> different network equipment, breaking compatibility on that side
> is unexpected as well. Requiring to bring down/up the connection
> just to change the TCP MD5 password is a no-go in networks with
> high availability requirements. Changing the BGP authentication
> password must be allowed without tearing down and re-establishing
> the TCP sockets. Otherwise it doesn't scale for large network
> operators to have to individually manage each individual TCP socket
> in their network. However, based on the feedback I received, it
> would be acceptable to tear-down the TCP connections and re-establish
> them when enabling or disabling the MD5 option.
>
> Here is a list of a few network vendors along with their behavior
> with respect to TCP MD5:
>
> - Cisco: Allows for password to be changed, but within the hold-down
> timer (~180 seconds).
> - Juniper: When password is initially set on active connection it will
> reset, but after that any subsequent password changes no network
> resets.
> - Nokia: No notes on if they flap the tcp connection or not.
> - Ericsson/RedBack: Allows for 2 password (old/new) to co-exist until
> both sides are ok with new passwords.
> - Meta-Switch: Expects the password to be set before a connection is
> attempted, but no further info on whether they reset the TCP
> connection on a change.
> - Avaya: Disable the neighbor, then set password, then re-enable.
> - Zebos: Would normally allow the change when socket connected.
>
>
If you want to be able to _change_ md5 key, this is fine by me, please
send a patch.
We can not dynamically turn on MD5, this is mentioned briefly in
tcp_synack_options().
If you want to turn on MD5 on an established flow, then you must
ensure that both SACK and TS were not enabled in the 3WHS,
and then make sure nothing blows up in the stack.
On Mon, Jun 29, 2020 at 1:47 PM Eric Dumazet <[email protected]> wrote:
>
> If you want to be able to _change_ md5 key, this is fine by me, please
> send a patch.
Eric, if this change breaks existing users, then it gets reverted.
That's just fundamental.
No RFC's are in the lreast relevant when compared to "this broke
existing users".
If you're not willing to do the work to fix it, I will revert that
commit. Because that's how it works - you can't ask other people to
fix the breakage you introduced.
It really is that simple. We do not allow developers to break things
and then step away and say "not my problem".
Linus
On Tue, Jun 30, 2020 at 12:43 PM Linus Torvalds
<[email protected]> wrote:
>
> If you're not willing to do the work to fix it, I will revert that
> commit.
Hmm. I only now noticed that that commit is over two years old.
So I think it's still wrong (clearly others do change passwords
outside of listening state), but considering that it apparently took
people two years to notice, at least some of the onus on figuring out
a better morel is on people who didn't even bother to test things in a
timely manner.
At some point "entreprise vendor kernels" or whatever who stay with
legacy kernels for a long time only have themselves to blame.
Linus
From: Linus Torvalds <[email protected]>
Date: Tue, 30 Jun 2020 12:43:21 -0700
> If you're not willing to do the work to fix it, I will revert that
> commit.
Please let me handle this situation instead of making threats, this
just got reported.
Thank you.
On Tue, Jun 30, 2020 at 1:21 PM David Miller <[email protected]> wrote:
>
> From: Linus Torvalds <[email protected]>
> Date: Tue, 30 Jun 2020 12:43:21 -0700
>
> > If you're not willing to do the work to fix it, I will revert that
> > commit.
>
> Please let me handle this situation instead of making threats, this
> just got reported.
>
> Thank you.
>
Also keep in mind the commit fixed a security issue, since we were
sending on the wire
garbage bytes from the kernel.
We can not simply revert it and hope for the best.
I find quite alarming vendors still use TCP MD5 "for security
reasons", but none of them have contributed to it in linux kernel
since 2018
(Time of the 'buggy patch')
----- On Jun 30, 2020, at 3:52 PM, Linus Torvalds [email protected] wrote:
> On Tue, Jun 30, 2020 at 12:43 PM Linus Torvalds
> <[email protected]> wrote:
>>
[...]
> So I think it's still wrong (clearly others do change passwords
> outside of listening state), but considering that it apparently took
> people two years to notice, at least some of the onus on figuring out
> a better morel is on people who didn't even bother to test things in a
> timely manner.
I'm fully willing to work with Eric on finding a way forward with a
fix which addresses the original issue Eric's patch was trying to
fix while preserving ABI compatibility.
The main thing we need to agree on at this stage is what is our goal. We
can either choose to restore the original ABI behavior entirely, or only
focus on what appears to be the most important use-cases.
AFAIU, restoring full ABI compatibility would require to re-enable all
the following scenarios:
A) Transition of live socket from no key -> MD5 key.
B) Transition of live socket from MD5 key -> no key.
C) Transition of live socket from MD5 key to a different MD5 key.
Scenario (C) appears to be the most important use-case, and probably the
easiest to restore to its original behavior.
AFAIU restoring scenarios A and B would require us to validate how
much header space is needed by each SACK, TS and MD5 option enabled
on the socket, and reject enabling any option that adds header space
requirement exceeding the available space.
I welcome advice on what should be the end goal here.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
On Tue, Jun 30, 2020 at 1:34 PM Mathieu Desnoyers
<[email protected]> wrote:
>
> ----- On Jun 30, 2020, at 3:52 PM, Linus Torvalds [email protected] wrote:
>
> > On Tue, Jun 30, 2020 at 12:43 PM Linus Torvalds
> > <[email protected]> wrote:
> >>
> [...]
> > So I think it's still wrong (clearly others do change passwords
> > outside of listening state), but considering that it apparently took
> > people two years to notice, at least some of the onus on figuring out
> > a better morel is on people who didn't even bother to test things in a
> > timely manner.
>
> I'm fully willing to work with Eric on finding a way forward with a
> fix which addresses the original issue Eric's patch was trying to
> fix while preserving ABI compatibility.
>
> The main thing we need to agree on at this stage is what is our goal. We
> can either choose to restore the original ABI behavior entirely, or only
> focus on what appears to be the most important use-cases.
>
> AFAIU, restoring full ABI compatibility would require to re-enable all
> the following scenarios:
>
> A) Transition of live socket from no key -> MD5 key.
> B) Transition of live socket from MD5 key -> no key.
> C) Transition of live socket from MD5 key to a different MD5 key.
>
> Scenario (C) appears to be the most important use-case, and probably the
> easiest to restore to its original behavior.
>
> AFAIU restoring scenarios A and B would require us to validate how
> much header space is needed by each SACK, TS and MD5 option enabled
> on the socket, and reject enabling any option that adds header space
> requirement exceeding the available space.
>
> I welcome advice on what should be the end goal here.
>
The (C) & (B) case are certainly doable.
A) case is more complex, I have no idea of breakages of various TCP
stacks if a flow got SACK
at some point (in 3WHS) but suddenly becomes Reno.
----- On Jun 30, 2020, at 4:30 PM, Eric Dumazet [email protected] wrote:
> On Tue, Jun 30, 2020 at 1:21 PM David Miller <[email protected]> wrote:
>>
>> From: Linus Torvalds <[email protected]>
>> Date: Tue, 30 Jun 2020 12:43:21 -0700
>>
>> > If you're not willing to do the work to fix it, I will revert that
>> > commit.
>>
>> Please let me handle this situation instead of making threats, this
>> just got reported.
>>
>> Thank you.
>>
>
> Also keep in mind the commit fixed a security issue, since we were
> sending on the wire
> garbage bytes from the kernel.
>
> We can not simply revert it and hope for the best.
>
> I find quite alarming vendors still use TCP MD5 "for security
> reasons", but none of them have contributed to it in linux kernel
> since 2018
> (Time of the 'buggy patch')
I'm helping a customer increase their contributions and feedback to upstream.
As we can see, they have accumulated some backlog over time.
Clearly reverting a security fix is not acceptable here. Coming up with a
proper ABI-compatible fix should not be out of our reach though.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: Eric Dumazet <[email protected]>
Date: Tue, 30 Jun 2020 13:39:27 -0700
> The (C) & (B) case are certainly doable.
>
> A) case is more complex, I have no idea of breakages of various TCP
> stacks if a flow got SACK
> at some point (in 3WHS) but suddenly becomes Reno.
I agree that C and B are the easiest to implement without having to
add complicated code to handle various negotiated TCP option
scenerios.
It does seem to be that some entities do A, or did I misread your
behavioral analysis of various implementations Mathieu?
Thanks.
On Tue, Jun 30, 2020 at 1:44 PM David Miller <[email protected]> wrote:
>
> From: Eric Dumazet <[email protected]>
> Date: Tue, 30 Jun 2020 13:39:27 -0700
>
> > The (C) & (B) case are certainly doable.
> >
> > A) case is more complex, I have no idea of breakages of various TCP
> > stacks if a flow got SACK
> > at some point (in 3WHS) but suddenly becomes Reno.
>
> I agree that C and B are the easiest to implement without having to
> add complicated code to handle various negotiated TCP option
> scenerios.
>
> It does seem to be that some entities do A, or did I misread your
> behavioral analysis of various implementations Mathieu?
>
> Thanks.
Yes, another question about Mathieu cases is do determine the behavior
of all these stacks vs :
SACK option
TCP TS option.
----- On Jun 30, 2020, at 4:56 PM, Eric Dumazet [email protected] wrote:
> On Tue, Jun 30, 2020 at 1:44 PM David Miller <[email protected]> wrote:
>>
>> From: Eric Dumazet <[email protected]>
>> Date: Tue, 30 Jun 2020 13:39:27 -0700
>>
>> > The (C) & (B) case are certainly doable.
>> >
>> > A) case is more complex, I have no idea of breakages of various TCP
>> > stacks if a flow got SACK
>> > at some point (in 3WHS) but suddenly becomes Reno.
>>
>> I agree that C and B are the easiest to implement without having to
>> add complicated code to handle various negotiated TCP option
>> scenerios.
>>
>> It does seem to be that some entities do A, or did I misread your
>> behavioral analysis of various implementations Mathieu?
>>
>> Thanks.
>
> Yes, another question about Mathieu cases is do determine the behavior
> of all these stacks vs :
> SACK option
> TCP TS option.
I will ask my customer's networking team to investigate these behaviors,
which will allow me to prepare a thorough reply to the questions raised
by Eric and David. I expect to have an answer within 2-3 weeks at most.
Thank you!
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com