-
Notifications
You must be signed in to change notification settings - Fork 139
Centralize password handling tool-openssl #2555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kingstjo
wants to merge
36
commits into
aws:main
Choose a base branch
from
kingstjo:password-handling-clean
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+892
−245
Open
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
be9365c
refactor(tool-openssl): Centralize password handling with HandlePassO…
kingstjo fb32a81
test(tool-openssl): Add comprehensive tests for password handling
kingstjo 1e5f7b0
test(tool-openssl): improve password test assertions
kingstjo c567c79
fix(test): improve cross-platform password test compatibility
kingstjo 2e01934
fix(test): Fix heap-use-after-free in PasswordTest.SensitiveStringDel…
kingstjo d51be8a
Merge branch 'main' into password-handling-clean
kingstjo 0beec94
refactor(test): Improve password_test.cc organization and test coverage
kingstjo 9de6fba
Merge remote-tracking branch 'origin/password-handling-clean' into pa…
kingstjo 873bc7d
Merge remote-tracking branch 'upstream/main' into password-handling-c…
kingstjo 782eac7
refactor(tool-openssl): Rename password.cc to pass_util.cc
kingstjo fd29186
refactor(tool-openssl): Rename password files and fix empty password …
kingstjo 79de4b7
refactor(tool-openssl): Modify ExtractPassword to update source in-place
kingstjo f2db8f6
refactor(tool-openssl): Rename password namespace to pass_util
kingstjo 85fae9d
refactor(tool-openssl): rename test classes to match pass_util naming
kingstjo f2ca6c4
test(tool-openssl): improve file truncation test coverage
kingstjo ad44167
fix: Add constructors to PassUtilSourceParams to fix uninitialized me…
kingstjo 3e50e56
Merge remote-tracking branch 'upstream/main' into password-handling-c…
kingstjo 4865e83
refactor(tool-openssl): Change validate_bio_size to use reference ins…
kingstjo 94f91af
feat(pass_util): Refactor password extraction with consistent API and…
kingstjo e2d63d3
refactor(pass_util): Add ValidateSource helper and remove redundant t…
kingstjo c110c35
(fix) update enum to uint8 for bitflag in order to avoid shadowing
kingstjo bb19693
Merge branch 'main' into password-handling-clean
kingstjo 83bbaa0
refactor(tool-openssl): Replace password source constants with enum c…
kingstjo bced12a
feat(tool-openssl): Include PEM_BUFSIZE limit in password error messages
kingstjo 4ee6c29
style(tool-openssl): Fix truncation detection parentheses for clarity
kingstjo 5d006be
docs(tool-openssl): Remove outdated comment about SensitiveStringDele…
kingstjo 9eff078
removed extra comments
kingstjo 5aec371
Using PEM_read_bio_PrivateKey() directly in pkcs8.cc
kingstjo a900bb8
fix: Allow zero-length passwords in PEM key decryption
kingstjo d047863
refactor(tool-openssl): Simplify pkcs8 key reading with read_private_der
kingstjo dede45e
test: Remove redundant comments from pass_util_test.cc
kingstjo 499285b
fix(pkcs8): Use ExtractPasswords instead of separate ExtractPassword …
kingstjo 7798f1a
test: Improve SensitiveStringDeleter test to use real usage pattern
kingstjo 8ded619
fix: Merge upstream ordered args with centralized password handling
kingstjo 8176847
style: Clean up formatting noise in pkcs8.cc
kingstjo a83ef97
Add Windows carriage return tests for password handling
kingstjo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,255 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 OR ISC | ||
|
||
#include <openssl/base.h> | ||
#include <openssl/err.h> | ||
#include <openssl/evp.h> | ||
#include <openssl/pem.h> | ||
#include <cstring> | ||
#include <string> | ||
#include "internal.h" | ||
|
||
// Use PEM_BUFSIZE (defined in openssl/pem.h) for password buffer size to ensure | ||
// compatibility with PEM functions and password callbacks throughout AWS-LC | ||
|
||
// Detect the type of password source | ||
static pass_util::Source DetectSource( | ||
const bssl::UniquePtr<std::string> &source) { | ||
if (!source || source->empty()) { | ||
return pass_util::Source::kNone; | ||
} | ||
if (source->compare(0, 5, "pass:") == 0) { | ||
return pass_util::Source::kPass; | ||
} | ||
if (source->compare(0, 5, "file:") == 0) { | ||
return pass_util::Source::kFile; | ||
} | ||
if (source->compare(0, 4, "env:") == 0) { | ||
return pass_util::Source::kEnv; | ||
} | ||
return pass_util::Source::kNone; | ||
} | ||
|
||
// Helper function to validate password sources and detect same-file case | ||
static bool ValidateSource(bssl::UniquePtr<std::string> &passin, | ||
bssl::UniquePtr<std::string> *passout = nullptr, | ||
bool *same_file = nullptr) { | ||
// Validate passin | ||
if (!passin) { | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
|
||
// Validate passout if provided | ||
if (passout && !*passout) { | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
|
||
// Validate passin format (if not empty) | ||
if (!passin->empty()) { | ||
pass_util::Source passin_type = DetectSource(passin); | ||
if (passin_type == pass_util::Source::kNone) { | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
} | ||
|
||
// Validate passout format (if provided and not empty) | ||
if (passout && *passout && !(*passout)->empty()) { | ||
pass_util::Source passout_type = DetectSource(*passout); | ||
if (passout_type == pass_util::Source::kNone) { | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
|
||
// Detect same-file case if requested | ||
if (same_file && !passin->empty()) { | ||
pass_util::Source passin_type = DetectSource(passin); | ||
*same_file = | ||
(passin_type == pass_util::Source::kFile && | ||
passout_type == pass_util::Source::kFile && *passin == **passout); | ||
} | ||
} | ||
|
||
// Initialize same_file to false if not detected | ||
if (same_file && (!passout || !*passout)) { | ||
*same_file = false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
static bool ExtractDirectPassword(bssl::UniquePtr<std::string> &source) { | ||
// Check for additional colons in password portion after prefix | ||
if (source->find(':', 5) != std::string::npos) { | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
|
||
// Check length before modification | ||
if (source->length() - 5 > PEM_BUFSIZE) { | ||
fprintf(stderr, "Password exceeds maximum allowed length (%d bytes)\n", | ||
PEM_BUFSIZE); | ||
return false; | ||
} | ||
|
||
// Remove "pass:" prefix by shifting the remaining content to the beginning | ||
source->erase(0, 5); | ||
return true; | ||
} | ||
|
||
static bool ExtractPasswordFromFile(bssl::UniquePtr<std::string> &source, | ||
bool skip_first_line = false) { | ||
// Remove "file:" prefix | ||
source->erase(0, 5); | ||
|
||
bssl::UniquePtr<BIO> bio(BIO_new_file(source->c_str(), "r")); | ||
if (!bio) { | ||
fprintf(stderr, "Cannot open password file\n"); | ||
return false; | ||
} | ||
|
||
char buf[PEM_BUFSIZE] = {}; | ||
|
||
// Skip first line if requested (for passout when using same file) | ||
if (skip_first_line) { | ||
if (BIO_gets(bio.get(), buf, sizeof(buf)) <= 0) { | ||
OPENSSL_cleanse(buf, sizeof(buf)); | ||
fprintf(stderr, "Cannot read password file\n"); | ||
return false; | ||
} | ||
OPENSSL_cleanse(buf, sizeof(buf)); | ||
} | ||
|
||
// Read the password line | ||
int len = BIO_gets(bio.get(), buf, sizeof(buf)); | ||
if (len <= 0) { | ||
OPENSSL_cleanse(buf, sizeof(buf)); | ||
fprintf(stderr, "Cannot read password file\n"); | ||
return false; | ||
} | ||
|
||
const bool possible_truncation = | ||
(static_cast<size_t>(len) == PEM_BUFSIZE - 1) && buf[len - 1] != '\n' && | ||
buf[len - 1] != '\r'; | ||
if (possible_truncation) { | ||
OPENSSL_cleanse(buf, sizeof(buf)); | ||
fprintf(stderr, "Password file content too long (maximum %d bytes)\n", | ||
PEM_BUFSIZE); | ||
return false; | ||
} | ||
|
||
// Trim trailing newlines | ||
size_t buf_len = len; | ||
while (buf_len > 0 && | ||
(buf[buf_len - 1] == '\n' || buf[buf_len - 1] == '\r')) { | ||
buf[--buf_len] = '\0'; | ||
} | ||
kingstjo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Replace source content with password | ||
*source = std::string(buf, buf_len); | ||
OPENSSL_cleanse(buf, sizeof(buf)); | ||
return true; | ||
} | ||
|
||
static bool ExtractPasswordFromEnv(bssl::UniquePtr<std::string> &source) { | ||
// Remove "env:" prefix | ||
source->erase(0, 4); | ||
|
||
if (source->empty()) { | ||
fprintf(stderr, "Empty environment variable name\n"); | ||
return false; | ||
} | ||
|
||
const char *env_val = getenv(source->c_str()); | ||
if (!env_val) { | ||
fprintf(stderr, "Environment variable '%s' not set\n", source->c_str()); | ||
return false; | ||
} | ||
|
||
size_t env_val_len = strlen(env_val); | ||
if (env_val_len == 0) { | ||
fprintf(stderr, "Environment variable '%s' is empty\n", source->c_str()); | ||
return false; | ||
} | ||
if (env_val_len > PEM_BUFSIZE) { | ||
fprintf(stderr, "Environment variable value too long (maximum %d bytes)\n", | ||
PEM_BUFSIZE); | ||
return false; | ||
} | ||
|
||
// Replace source content with environment value | ||
*source = std::string(env_val); | ||
return true; | ||
} | ||
|
||
// Internal helper to extract password based on source type | ||
static bool ExtractPasswordFromSource(bssl::UniquePtr<std::string> &source, | ||
pass_util::Source type, | ||
bool skip_first_line = false) { | ||
switch (type) { | ||
case pass_util::Source::kPass: | ||
return ExtractDirectPassword(source); | ||
case pass_util::Source::kFile: | ||
return ExtractPasswordFromFile(source, skip_first_line); | ||
case pass_util::Source::kEnv: | ||
return ExtractPasswordFromEnv(source); | ||
default: | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
} | ||
|
||
namespace pass_util { | ||
|
||
void SensitiveStringDeleter(std::string *str) { | ||
if (str && !str->empty()) { | ||
OPENSSL_cleanse(&(*str)[0], str->size()); | ||
} | ||
delete str; | ||
} | ||
|
||
bool ExtractPassword(bssl::UniquePtr<std::string> &source) { | ||
if (!ValidateSource(source)) { | ||
return false; | ||
} | ||
|
||
if (source->empty()) { | ||
fprintf(stderr, "Invalid password format (use pass:, file:, or env:)\n"); | ||
return false; | ||
} | ||
|
||
pass_util::Source type = DetectSource(source); | ||
return ExtractPasswordFromSource(source, type); | ||
} | ||
|
||
bool ExtractPasswords(bssl::UniquePtr<std::string> &passin, | ||
bssl::UniquePtr<std::string> &passout) { | ||
// Use ValidateSource for all validation and same-file detection | ||
bool same_file = false; | ||
if (!ValidateSource(passin, &passout, &same_file)) { | ||
return false; | ||
} | ||
|
||
// Extract passin (always from first line) | ||
if (!passin->empty()) { | ||
pass_util::Source passin_type = DetectSource(passin); | ||
if (!ExtractPasswordFromSource(passin, passin_type, false)) { | ||
return false; | ||
} | ||
} | ||
|
||
// Extract passout (from first line if different files, second line if same | ||
// file) | ||
if (!passout->empty()) { | ||
pass_util::Source passout_type = DetectSource(passout); | ||
if (!ExtractPasswordFromSource(passout, passout_type, same_file)) { | ||
return false; | ||
} | ||
} | ||
|
||
return true; | ||
} | ||
|
||
} // namespace pass_util |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.