Skip to content

CDRIVER-5634: SCRAM-SHA-256 FIPS Compliance #1684

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

Merged
merged 29 commits into from
Aug 9, 2024

Conversation

Julia-Garland
Copy link
Contributor

@Julia-Garland Julia-Garland commented Aug 5, 2024

Summary

(Mostly) ensures the driver implementation of SCRAM-SHA-256 (as well as SCRAM-SHA-1) delegates implementation of key derivation is delegated to a cryptographic provider that already is or can be configured to be FIPS-compliant.

What changed?

Previously the driver implemented the PBKDF2 (Password-based-Key-Derivative-Function) itself in _mongoc_salt_password().

  • When MONGOC_ENABLE_CRYPTO=LIBCRYPTO is enabled, the driver uses PKCS5_PBKDF2_HMAC() with EVP_sha256() (or EVP_sha1()) from the OpenSSL EVP library.
  • When MONGOC_ENABLE_CRYPTO=COMMON_CRYPTO is enabled, the driver uses CCKeyDerivationPBKDF() with kCCPRFHmacAlgSHA256 (or kCCPRFHmacAlgSHA1) from the CommonCrypto KeyDerivation library.
  • When MONGOC_ENABLE_CRYPTO=CNG is enabled, the driver uses BCryptDeriveKeyPBKDF2() with a handle to HMAC-SHA-256 from the Bcrypt library. If the symbol is not supported, the driver reverts to the original key derivation function.

MinGW-w64

BCryptDeriveKeyPBKDF2 was added in MinGW-w64 6.0.0, but some hosts for Evergreen tests use older versions. I filed CDRIVER-5649 to “Require MinGW 6.0.0 or higher”, but until then only users with more modern MinGW versions get FIPS compliance on Windows.

@Julia-Garland Julia-Garland self-assigned this Aug 5, 2024
@Julia-Garland Julia-Garland requested a review from kevinAlbs August 5, 2024 15:05
@Julia-Garland Julia-Garland marked this pull request as ready for review August 5, 2024 18:02
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. Suggested changing returns from int to bool and a possible fix to check for BCryptDeriveKeyPBKDF2.

Nice spot of the duplicate call to BCryptCloseAlgorithmProvider (&_sha256_hmac_algo, 0);.

Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with another check.

@Julia-Garland Julia-Garland merged commit 7b2ed29 into mongodb:master Aug 9, 2024
44 of 46 checks passed
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.

3 participants