lkml.org 
[lkml]   [2021]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] tcp: Initial support for RFC5925 auth option
    From
    Date
    On 11/3/21 5:18 AM, David Ahern wrote:
    > On 11/1/21 10:34 AM, Leonard Crestez wrote:
    >> This is similar to TCP MD5 in functionality but it's sufficiently
    >> different that wire formats are incompatible. Compared to TCP-MD5 more
    >> algorithms are supported and multiple keys can be used on the same
    >> connection but there is still no negotiation mechanism.
    >>
    >> Expected use-case is protecting long-duration BGP/LDP connections
    >> between routers using pre-shared keys. The goal of this series is to
    >> allow routers using the linux TCP stack to interoperate with vendors
    >> such as Cisco and Juniper.
    >>
    >> Both algorithms described in RFC5926 are implemented but the code is not
    >> very easily extensible beyond that. In particular there are several code
    >> paths making stack allocations based on RFC5926 maximum, those would
    >> have to be increased.
    >>
    >> This version implements SNE and l3mdev awareness and adds more tests.
    >> Here are some known flaws and limitations:
    >>
    >> * Interaction with TCP-MD5 not tested in all corners
    >> * Interaction with FASTOPEN not tested and unlikely to work because
    >> sequence number assumptions for syn/ack.
    >> * Not clear if crypto_shash_setkey might sleep. If some implementation
    >> do that then maybe they could be excluded through alloc flags.
    >> * Traffic key is not cached (reducing performance)
    >> * User is responsible for ensuring keys do not overlap.
    >> * There is no useful way to list keys, making userspace debug difficult.
    >> * There is no prefixlen support equivalent to md5. This is used in
    >> some complex FRR configs.
    >>
    >> Test suite was added to tools/selftests/tcp_authopt. Tests are written
    >> in python using pytest and scapy and check the API in some detail and
    >> validate packet captures. Python code is already used in linux and in
    >> kselftests but virtualenvs not very much, this particular test suite
    >> uses `pip` to create a private virtualenv and hide dependencies.
    >>
    >> This actually forms the bulk of the series by raw line-count. Since
    >> there is a lot of code it was mostly split on "functional area" so most
    >> files are only affected by a single code. A lot of those tests are
    >> relevant to TCP-MD5 so perhaps it might help to split into a separate
    >> series?
    >>
    >> Some testing support is included in nettest and fcnal-test.sh, similar
    >> to the current level of tcp-md5 testing.
    >>
    >> SNE was tested by creating connections in a loop until a large SEQ is
    >> randomly selected and then making it rollover. The "connect in a loop"
    >> step ran into timewait overflow and connection failure on port reuse.
    >> After spending some time on this issue and my conclusion is that AO
    >> makes it impossible to kill remainders of old connections in a manner
    >> similar to unsigned or md5sig, this is because signatures are dependent
    >> on ISNs. This means that if a timewait socket is closed improperly then
    >> information required to RST the peer is lost.
    >>
    >> The fact that AO completely breaks all connection-less RSTs is
    >> acknowledged in the RFC and the workaround of "respect timewait" seems
    >> acceptable.
    >>
    >> Changes for frr (old): https://github.com/FRRouting/frr/pull/9442
    >> That PR was made early for ABI feedback, it has many issues.
    >>
    >
    > overall looks ok to me. I did not wade through the protocol details.
    >
    > I did see the comment about no prefixlen support in the tests. A lot of
    > patches to absorb, perhaps I missed it. Does AuthOpt support for
    > prefixes? If not, you should consider adding that as a quick follow on
    > (within the same dev cycle). MD5 added prefix support for scalability;
    > seems like AO should be concerned about the same.

    I just skipped it because it's not required for core functionality.

    It's very straight forward so I will add it to the next version.

    --
    Regards,
    Leonard

    \
     
     \ /
      Last update: 2021-11-03 23:23    [W:2.100 / U:1.656 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site