Tolerate differences in RSA private key libraries #383
Conversation
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
2d07bc6 to
c0eb29b
Compare
|
+CC @AlexandreEXFO FYI, in case you find this helpful for your environment |
|
Not sure you need the import of cmp to handle slice order here. You passed Primes[0] into the TPM struct and then used that field as Primes[0] on the way out too. If they came out reversed that would be a surprise indeed! Otherwise LGTM. |
Ha, of course. Too paranoid. I've simplified this, thanks! |
alexmwu
approved these changes
Dec 20, 2024
gopherbot
pushed a commit
to golang/go
that referenced
this pull request
Jan 7, 2025
This has no practical advantage, and requires extra variable time code, but is an explicit FIPS 186-5 requirement. Note that the new behavior is consistent with Go+BoringCrypto, but not with Go 1.23. The resulting keys are essentially interchangeable, but it's not impossible for applications to notice (google/go-tpm#383). gcd_lcm_tests.txt is from BoringSSL. Change-Id: I6a6a4656fd5e92912c87bedc667456d0e8787023 Reviewed-on: https://go-review.googlesource.com/c/go/+/639936 Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org>
FiloSottile
added a commit
to FiloSottile/bigmod
that referenced
this pull request
May 8, 2025
This has no practical advantage, and requires extra variable time code, but is an explicit FIPS 186-5 requirement. Note that the new behavior is consistent with Go+BoringCrypto, but not with Go 1.23. The resulting keys are essentially interchangeable, but it's not impossible for applications to notice (google/go-tpm#383). gcd_lcm_tests.txt is from BoringSSL. Change-Id: I6a6a4656fd5e92912c87bedc667456d0e8787023 Reviewed-on: https://go-review.googlesource.com/c/go/+/639936 Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org> # Conflicts: # nat.go # src/crypto/internal/fips140/rsa/keygen.go # src/crypto/internal/fips140/rsa/keygen_test.go # src/crypto/internal/fips140/rsa/rsa.go
FiloSottile
added a commit
to FiloSottile/bigmod
that referenced
this pull request
May 8, 2025
This has no practical advantage, and requires extra variable time code, but is an explicit FIPS 186-5 requirement. Note that the new behavior is consistent with Go+BoringCrypto, but not with Go 1.23. The resulting keys are essentially interchangeable, but it's not impossible for applications to notice (google/go-tpm#383). gcd_lcm_tests.txt is from BoringSSL. Change-Id: I6a6a4656fd5e92912c87bedc667456d0e8787023 Reviewed-on: https://go-review.googlesource.com/c/go/+/639936 Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org> # Conflicts: # nat.go # src/crypto/internal/fips140/rsa/keygen.go # src/crypto/internal/fips140/rsa/keygen_test.go # src/crypto/internal/fips140/rsa/rsa.go
FiloSottile
added a commit
to FiloSottile/bigmod
that referenced
this pull request
May 8, 2025
This has no practical advantage, and requires extra variable time code, but is an explicit FIPS 186-5 requirement. Note that the new behavior is consistent with Go+BoringCrypto, but not with Go 1.23. The resulting keys are essentially interchangeable, but it's not impossible for applications to notice (google/go-tpm#383). gcd_lcm_tests.txt is from BoringSSL. Change-Id: I6a6a4656fd5e92912c87bedc667456d0e8787023 Reviewed-on: https://go-review.googlesource.com/c/go/+/639936 Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org> # Conflicts: # nat.go # src/crypto/internal/fips140/rsa/keygen.go # src/crypto/internal/fips140/rsa/keygen_test.go # src/crypto/internal/fips140/rsa/rsa.go
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.
GOEXPERIMENT=boringcrypto go test ./... --test.run TestPriv/valid_rsafails occasionally due to the fact that the boringcrypto implementation uses the Carmichael function (lambda(N)) instead of Euler's totient function (phi(N)). Sometimes we get lucky (where phi(N) == lambda(N)) and sometimes we don't.Unfortunately, the TPM RSA private key structure only contains a prime, for compactness. This means that when importing TPM RSA private keys, we have to compute D from the factorization of N ourselves (see crypto.go). This test generates a key in software, converts it into a TPM private key structure, and then checks that the import function returns the same key. Unfortunately, the software key-generation algorithm may disagree with crypto.go's technique (which is currently to use phi(N)).
This change introduces a helper function to the test for comparing RSA private keys by public exponent, modulus, and primes only.