Skip to content

Fix issues in pass_util.cc password handling#3032

Open
justsmth wants to merge 1 commit intoaws:mainfrom
justsmth:cleanup-pass_util
Open

Fix issues in pass_util.cc password handling#3032
justsmth wants to merge 1 commit intoaws:mainfrom
justsmth:cleanup-pass_util

Conversation

@justsmth
Copy link
Contributor

Description of changes:

Addresses multiple issues in tool-openssl/pass_util.cc and the Password class:

  • Improved secure memory clearing to cover residual bytes left by string mutations, and ensured all temporary buffers holding passwords are properly cleansed.
  • Fixed fd: parsing to use strtoul instead of atoi, avoiding undefined behavior on overflow.
  • Removed incorrect rejection of colons in pass: passwords, matching OpenSSL behavior.
  • Fixed off-by-one in password length limits for pass: and env: sources.
  • Minor hardening: removed env var names from error messages, fixed same_file initialization.

Call-outs:

  • Behavioral change: pass:a:b was previously rejected and now succeeds with password a:b.
  • Password::secure_clear() now cleanses up to capacity() bytes, which is technically beyond the std::string standard contract but safe across all major implementations.

Testing:

All existing PassUtilTest tests pass. Updated DirectPasswordEdgeCases to cover passwords containing colons.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.51%. Comparing base (0628190) to head (a9dc9a4).

Files with missing lines Patch % Lines
tool-openssl/pass_util.cc 82.35% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3032      +/-   ##
==========================================
+ Coverage   78.35%   78.51%   +0.16%     
==========================================
  Files         689      689              
  Lines      121010   121014       +4     
  Branches    16992    16995       +3     
==========================================
+ Hits        94813    95016     +203     
+ Misses      25302    25099     -203     
- Partials      895      899       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants