lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20221122185534.308643-1-dima@arista.com>
Date:   Tue, 22 Nov 2022 18:55:29 +0000
From:   Dmitry Safonov <dima@...sta.com>
To:     linux-kernel@...r.kernel.org, David Ahern <dsahern@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Dmitry Safonov <dima@...sta.com>, Ard Biesheuvel <ardb@...nel.org>,
        Bob Gilligan <gilligan@...sta.com>,
        "David S. Miller" <davem@...emloft.net>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Francesco Ruggeri <fruggeri@...sta.com>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Jason Baron <jbaron@...mai.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Salam Noureddine <noureddine@...sta.com>,
        Steven Rostedt <rostedt@...dmis.org>, netdev@...r.kernel.org
Subject: [PATCH v5 0/5] net/tcp: Dynamically disable TCP-MD5 static key

Changes from v4:
- Used rcu_dereference_protected() for tp->md5sig_info in
  tcp_md5_do_add() and tcp_md5_key_copy() fail paths to make sure
  there won't be false-positives from sparse (Jakub)
- Added Acked-by: Jakub Kicinski

Changes from v3:
- Used atomic_try_cmpxchg() as suggested by Peter Zijlstra
- Renamed static_key_fast_inc() => static_key_fast_inc_not_negative()
  (addressing Peter Zijlstra's review)
- Based on linux-tip/master
- tcp_md5_key_copy() now does net_warn_ratelimited()
  (addressing Peter Zijlstra's review)
  tcp_md5_do_add() does not as it returns -EUSERS from setsockopt()
  syscall back to the userspace
- Corrected WARN_ON_ONCE(!static_key_fast_inc(key))
  (Spotted by Jason Baron)
- Moved declaration of static_key_fast_inc_not_negative() and its
  EXPORT_SYMBOL_GPL() to the patch 3 that uses it,
  "net/tcp: Disable TCP-MD5 static key on tcp_md5sig_info destruction"
  (addressing Peter Zijlstra's review)
- Added patch 4 that destroys the newly created request socket
  if md5 info allocation or static_key increment was unsuccessful.
  Instead of proceeding to add a socket without TCP-MD5 keys.
- Added patch 5 that separates helper tcp_time_wait_init()
  and converts BUG_ON() to WARN_ON_ONCE().

Changes from v2:
- Prevent key->enabled from turning negative by overflow from
  static_key_slow_inc() or static_key_fast_inc()
  (addressing Peter Zijlstra's review)
- Added checks if static_branch_inc() and static_key_fast_int()
  were successful to TCP-MD5 code.

Changes from v1:
- Add static_key_fast_inc() helper rather than open-coded atomic_inc()
  (as suggested by Eric Dumazet)

Version 4:
https://lore.kernel.org/all/20221115211905.1685426-1-dima@arista.com/T/#u
Version 3:
https://lore.kernel.org/all/20221111212320.1386566-1-dima@arista.com/T/#u
Version 2: 
https://lore.kernel.org/all/20221103212524.865762-1-dima@arista.com/T/#u
Version 1: 
https://lore.kernel.org/all/20221102211350.625011-1-dima@arista.com/T/#u

The static key introduced by commit 6015c71e656b ("tcp: md5: add
tcp_md5_needed jump label") is a fast-path optimization aimed at
avoiding a cache line miss.
Once an MD5 key is introduced in the system the static key is enabled
and never disabled. Address this by disabling the static key when
the last tcp_md5sig_info in system is destroyed.

Previously it was submitted as a part of TCP-AO patches set [1].
Now in attempt to split 36 patches submission, I send this independently.

Cc: Ard Biesheuvel <ardb@...nel.org>
Cc: Bob Gilligan <gilligan@...sta.com>
Cc: David Ahern <dsahern@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>
Cc: Dmitry Safonov <0x7f454c46@...il.com>
Cc: Eric Dumazet <edumazet@...gle.com>
Cc: Francesco Ruggeri <fruggeri@...sta.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
Cc: Jakub Kicinski <kuba@...nel.org>
Cc: Jason Baron <jbaron@...mai.com>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Paolo Abeni <pabeni@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Salam Noureddine <noureddine@...sta.com>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: netdev@...r.kernel.org
Cc: linux-kernel@...r.kernel.org

[1]: https://lore.kernel.org/all/20221027204347.529913-1-dima@arista.com/T/#u

Thanks,
            Dmitry

Dmitry Safonov (5):
  jump_label: Prevent key->enabled int overflow
  net/tcp: Separate tcp_md5sig_info allocation into
    tcp_md5sig_info_add()
  net/tcp: Disable TCP-MD5 static key on tcp_md5sig_info destruction
  net/tcp: Do cleanup on tcp_md5_key_copy() failure
  net/tcp: Separate initialization of twsk

 include/linux/jump_label.h | 21 +++++++--
 include/net/tcp.h          | 10 ++--
 kernel/jump_label.c        | 55 +++++++++++++++++-----
 net/ipv4/tcp.c             |  5 +-
 net/ipv4/tcp_ipv4.c        | 96 +++++++++++++++++++++++++++++---------
 net/ipv4/tcp_minisocks.c   | 61 +++++++++++++++---------
 net/ipv4/tcp_output.c      |  4 +-
 net/ipv6/tcp_ipv6.c        | 21 ++++-----
 8 files changed, 193 insertions(+), 80 deletions(-)


base-commit: 771a207d1ee9f38da8c0cee1412228f18b900bac
-- 
2.38.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ