uams: harden DHX2, DHX, SRP key exchange and session verifier#2932
uams: harden DHX2, DHX, SRP key exchange and session verifier#2932rdmark wants to merge 3 commits into
Conversation
🤖 Augment PR SummarySummary: This PR hardens the DHX2 UAM key exchange by making wire serialization and state handling more robust. Changes:
Technical Notes: The changes align wire formatting with Apple’s expectations and harden against clients that skip required exchange steps (inspired by afp-perl fixes). 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
Zero-pad the DH prime p when serializing for the wire, matching the existing treatment of g and Ma. Also guard logincont2 and changepw_3 against being called before the shared key K is established by logincont1, preventing a NULL pointer dereference if a client skips the key exchange step. Add error handling for all gcry_mpi_print calls when serializing g, p, and Ma in dhx2_setup. If serialization fails, bail out via the existing error path instead of using a stale nwritten value for the zero-padding logic. Inspired by bug fixes in afp-perl by Derrik Pates
Guard logincont and changepw against being called before the shared key K is established, preventing a NULL pointer dereference if the login step was skipped or failed. Also release K immediately after serializing it to the local key buffer, fixing a leak that left key material in memory indefinitely after authentication. Add error handling for all gcry_mpi_print calls in DHX UAMs. In the setup functions, bail out via the existing error path if serialization of K or Mb fails. In logincont and changepw, release K and return an error instead of using a stale nwritten value for the zero-padding logic. Inspired by bug fixes in afp-perl by Derrik Pates
Check that the SRP session verifier is set before proceeding in srp_logincont, preventing a NULL pointer dereference if the login step was skipped or failed.
rdmark
force-pushed
the
rdmark-dhx2-hardening
branch
from
63758fa to
08a96e4
Compare
|
🔥 Spectest (AFP 3.4) - Flamegraph (AFP_ASSERT active)Commit: 🔥 Open interactive Flamegraph (SVG) 🔝 Top 10 leaf functions
|
|
the heavy code repetition is a recurring concern with these UAMs -- we've had a few corner case bugs in the past because of this. I have been pondering alternative solutions: such as a static library with abstractions for the repeated logic, or maybe even merge the respective PAM and passwd libraries into one and make the authentication backend a runtime option. |
rdmark
changed the title
Yes we should try to have shared code if we can - this is going to be prone to issues as you say, which is not a good thing for the front security layer |
2 participants
DHX2: Zero-pad the DH prime p when serializing for the wire, matching the existing treatment of g and Ma. Also guard logincont2 and changepw_3 against being called before the shared key K is established by logincont1, preventing a NULL pointer dereference if a client skips the key exchange step.
DHX: Guard logincont and changepw against being called before the shared key K is established, preventing a NULL pointer dereference if the login step was skipped or failed. Also release K immediately after serializing it to the local key buffer, fixing a leak that left key material in memory indefinitely after authentication.
SRP: Check that the SRP session verifier is set before proceeding in srp_logincont, preventing a NULL pointer dereference if the login step was skipped or failed.
Inspired by bug fixes in afp-perl by Derrik Pates