-
Notifications
You must be signed in to change notification settings - Fork 448
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
Core: SecurityUtils.createMtlsKeyStore
should use a fixed algorithm for creating the PrivateKey
#1675
Labels
priority: p3
Desirable enhancement or fix. May not be included in next release.
Comments
Capstan
referenced
this issue
Jun 19, 2022
* feat: support keystore in transport for mtls * fix format * update code * add tests * update test and doc * update names * create keystore from cert and key string * change certAndKey from string to inputstream * add mtls file * Update google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java Co-authored-by: Jeff Ching <chingor@google.com> * Update google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java Co-authored-by: Jeff Ching <chingor@google.com> * Update google-http-client/src/main/java/com/google/api/client/util/SslUtils.java Co-authored-by: Jeff Ching <chingor@google.com> * Update google-http-client/src/main/java/com/google/api/client/util/SslUtils.java Co-authored-by: Jeff Ching <chingor@google.com> * Update google-http-client/src/test/java/com/google/api/client/util/SecurityUtilsTest.java Co-authored-by: Jeff Ching <chingor@google.com> * Update google-http-client/src/main/java/com/google/api/client/util/SslUtils.java Co-authored-by: Jeff Ching <chingor@google.com> * update the code * fix name Co-authored-by: Jeff Ching <chingor@google.com>
@TimurSadykov Is this something you can look at? Thanks! |
meltsufin
added
priority: p3
Desirable enhancement or fix. May not be included in next release.
and removed
triage me
I really want to be triaged.
labels
Jun 22, 2022
ack, will take a look and update |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Callers of
SecurityUtils.createMtlsKeyStore
can direct the use of insecure algorithms in the construction of the mTLS connection. In general, we attempt to affix the algorithm used to inhibit use of algorithms with known vulnerabilities.Personally, I would be fine with denying provided certs whose
cert.getPublicKey().getAlgorithm()
wasDH
orDSA
, but in lieu of blocklisting known bad algorithms, perhaps it'd be more security affirmative to allowlist currently-OK algorithms, so we only ever use good versions, and suffer toil later if we have to retrofit additional entries.My suggestion is to create a case statement just below where the method currently throws
IllegalArgumentException
for various cases, and add the set of known good algorithms, and throw anInvalidAlgorithmParameterException
if it is not one of those. In addition, special caseDH
andDSA
to give references to their vulnerabilities as described inInsecureCypherMode.identifyDiffieHellmanAndDsaVulnerabilities
./cc @sophieschmieg, as her guidance/requirements will be canonical for importing whatever changes are made here into the monorepo.
Environment details
Steps to reproduce
SecurityUtils.createMtlsKeyStore
will generate anInsecureCipherMode
warning.Code example
The text was updated successfully, but these errors were encountered: