Skip to content
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

use local random generator in GCP #1047

Merged
merged 1 commit into from
Oct 25, 2024
Merged

use local random generator in GCP #1047

merged 1 commit into from
Oct 25, 2024

Conversation

jmhodges
Copy link
Contributor

We tried making the GCP KMS GenerateRandom one work (that's called by
libkmsp11 through the crypto11.PKCS11RandReader interface) but we hit
rate limits quickly (esp because of Go's use of randutil.MaybeReadByte)
and it required a lot of code to fit all reads between 8 and 1024 bytes.
See https://cloud.google.com/kms/docs/generate-random#known_limitations

Modern Linux systems have a great /dev/urandom and Go uses that. It's
security properties are as good or better than HSM implementations.

We hadn't even finished the code to use at least 8 bytes, so this avoids
even more code churn.

We tried making the GCP KMS GenerateRandom one work (that's called by
libkmsp11 through the crypto11.PKCS11RandReader interface) but we hit
rate limits quickly (esp because of Go's use of randutil.MaybeReadByte)
and it required a lot of code to fit all reads between 8 and 1024 bytes.
See https://cloud.google.com/kms/docs/generate-random#known_limitations

Modern Linux systems have a great /dev/urandom and Go uses that. It's
security properties are as good or better than HSM implementations.

We hadn't even finished the code to use at least 8 bytes, so this avoids
even more code churn.
@jmhodges jmhodges marked this pull request as ready for review October 25, 2024 19:09
@jmhodges jmhodges requested review from a team as code owners October 25, 2024 19:09
@jmhodges jmhodges requested review from alexcottner and removed request for a team October 25, 2024 19:09
@@ -664,262 +662,3 @@ func TestMakeKeyGCPRSA(t *testing.T) {
t.Fatalf("MakeKey failed: %v", err)
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

My beautiful tests! 😭

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lmao the life of the engineer 💀

@jmhodges jmhodges enabled auto-merge (squash) October 25, 2024 19:16
@jmhodges jmhodges merged commit f9c7ec9 into main Oct 25, 2024
14 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.

2 participants