F-6133 enc: fail on unsupported -pass source instead of empty password#256
Open
julek-wolfssl wants to merge 2 commits into
Open
F-6133 enc: fail on unsupported -pass source instead of empty password#256julek-wolfssl wants to merge 2 commits into
julek-wolfssl wants to merge 2 commits into
Conversation
wolfCLU_setup ignored the return value of wolfCLU_GetPassword in the WOLFCLU_PASSWORD_SOURCE (-pass) case. wolfCLU_GetPassword only accepts the "stdin" and "pass:" sources; for any other source it zeroes the password buffer and returns WOLFCLU_FATAL_ERROR. The error was ignored and the crypto dispatch was not gated on ret, so encryption proceeded with an all-zero password, wrote a valid output file, and returned success. A user passing an OpenSSL-style "env:", "file:", or "fd:" source therefore silently encrypted under an empty password. Check the return value and bail out (freeing buffers) on failure. Add EncPassSourceTest regression tests covering unsupported sources and the still-working pass: source.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a security-sensitive behavior in the enc command path: when -pass is given an unsupported password source (e.g., env:, file:, fd:), the tool must fail instead of silently proceeding with an all-zero/empty password. It also adds regression tests to ensure unsupported sources fail and the supported pass: source continues to work.
Changes:
- Gate the
WOLFCLU_PASSWORD_SOURCE(-pass) flow onwolfCLU_GetPassword()’s return value and bail out on failure. - Add regression tests covering unsupported
-passsources and a successfulpass:round-trip.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/crypto/clu_crypto_setup.c |
Adds early-exit error handling for unsupported -pass sources to prevent encrypting with an empty key. |
tests/encrypt/enc-test.py |
Adds EncPassSourceTest regression tests for unsupported password sources and the supported pass: source. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The other early error returns in wolfCLU_setup do not free mode, so freeing it only in this path was inconsistent. Keep the error handling minimal and uniform with the sibling -key/-iv/-inkey error returns.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
wolfCLU_setup ignored the return value of wolfCLU_GetPassword in the WOLFCLU_PASSWORD_SOURCE (-pass) case. wolfCLU_GetPassword only accepts the "stdin" and "pass:" sources; for any other source it zeroes the password buffer and returns WOLFCLU_FATAL_ERROR. The error was ignored and the crypto dispatch was not gated on ret, so encryption proceeded with an all-zero password, wrote a valid output file, and returned success. A user passing an OpenSSL-style "env:", "file:", or "fd:" source therefore silently encrypted under an empty password.
Check the return value and bail out (freeing buffers) on failure.
Add EncPassSourceTest regression tests covering unsupported sources and the still-working pass: source.