Skip to content

bcrypt: Validate imported ECC public keys - #11

Merged
jungwuk-ryu merged 1 commit into
mainfrom
codex/fix-public-key-validation-bypass-in-ecc-imports
Aug 9, 2026
Merged

jungwuk-ryu merged 1 commit into
mainfrom
codex/fix-public-key-validation-bypass-in-ecc-imports

Conversation

@jungwuk-ryu

Copy link
Copy Markdown
Owner

Motivation

  • The ECC import helper unconditionally enabled SYMCRYPT_FLAG_KEY_MINIMAL_VALIDATION, which causes SymCrypt to skip required public-key checks for imported ECDH/ECDSA blobs and allows a validation bypass for caller-controlled key material.

Description

  • Remove SYMCRYPT_FLAG_KEY_MINIMAL_VALIDATION from get_ecc_import_flags() in dlls/bcrypt/bcrypt_main.c so imported ECC public keys are passed to SymCrypt without the minimal-validation opt-out and therefore receive full point and subgroup validation during SymCryptEckeySetValue().

Testing

  • Ran git diff --check with no issues reported.
  • Ran git status --short --branch to inspect repository state and it completed successfully.
  • Ran git show --stat --oneline HEAD to confirm the change is present and it completed successfully.

Codex Task

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Stop requesting minimal SymCrypt validation so malformed public points and
invalid private scalars are rejected. Derive generic blob coordinate sizes
from the configured curve before forming pointers, and translate expected
SymCrypt failures to BCrypt status codes.

Add ECDSA and ECDH tests for invalid points and mismatched generic
coordinate lengths.
@jungwuk-ryu
jungwuk-ryu force-pushed the codex/fix-public-key-validation-bypass-in-ecc-imports branch from 23fd04f to db88e8b Compare August 9, 2026 10:11

Copy link
Copy Markdown
Owner Author

Review completed

Root cause and fix

  • ECC imports requested SymCrypt minimal validation, so malformed public points were not validated.
  • Generic ECC blobs also trusted the caller-provided coordinate width when checking the declared input length.
  • The updated implementation validates the blob against the configured curve before forming coordinate pointers, enables normal SymCrypt key validation, maps expected validation/allocation errors to BCrypt status codes, and publishes the key only after success.

Verification

  • Optimized x86_64 and i386 bcrypt DLL builds succeeded.
  • Focused x86_64/i386 probes rejected invalid P-256 points and malformed generic ECC lengths with STATUS_INVALID_PARAMETER.
  • x86_64 bcrypt suite: 23,332 tests, 8 todos, 0 failures, 0 skipped.
  • ./switchyard/verify_source.sh, diff checks, and linear-history checks passed.

Final review

  • Reviewed caller-controlled lengths, coordinate-size arithmetic, pointer formation, partial-allocation cleanup, output-handle publication, error mapping, ABI behavior, logging, and thread interactions.
  • No new shared state, secret logging, or ABI/layout change was introduced.
  • A valid P-256 import/destroy A/B/A/B run on Apple M5 Pro through Rosetta showed no separable regression at the observed noise level (aggregate medians: 33.224 µs before, 33.206 µs after; block variation was larger than the delta).
  • SYMCRYPT_FLAG_KEY_NO_FIPS remains intentionally unchanged; FIPS key-pair regeneration is outside this PR and would require separate Windows-compatibility evidence.

@jungwuk-ryu
jungwuk-ryu merged commit b9563f0 into main Aug 9, 2026
1 check passed
@jungwuk-ryu
jungwuk-ryu deleted the codex/fix-public-key-validation-bypass-in-ecc-imports branch August 9, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant