Open pavolloffay opened 3 weeks ago
What would be the replacement for the current crypto module?
I don't know exactly. I am not sure if there is an alternative in the main crypto
package. If no then could we add a build tag to use the implementation from openssl?
Only azidentity.ParseCertificates
calls pkcs12.ToPEM
(see here). This is a helper method for getting a cert and key to pass to NewClientCertificateCredential. That constructor takes the cert and key as types from the standard library's crypto
module, so applications aren't required to call ParseCertificates
. Is the pkcs12.ToPEM
call a problem even if it's never executed?
Hi @pavolloffay. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.
Hi @pavolloffay, we're sending this friendly reminder because we haven't heard back from you in 7 days. We need more information about this issue to help address it. Please be sure to give us your input. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!
@chlowell I got this reply from our FIPS folks:
Not FIPS compliant: pkcs12.ToPEM() performs a MAC check operation, for which it uses getSafeContents(), which calls verifyMac(), which calls pbkdf(), which is implemented in x/crypto, and thus not FIPS validated.
Sure, that makes sense. What I want to understand is whether this is a static analysis or a runtime problem. Our cert authentication implementation doesn't require applications to call pkcs12.ToPEM()
, so we have a straightforward workaround if your application can comply by simply not calling that function. But if the fact that the module contains a line invoking pkcs12.ToPEM()
makes your application noncompliant even if it never executes that line, that's a hard problem 😄
Hi @pavolloffay. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.
Hi @pavolloffay, we're sending this friendly reminder because we haven't heard back from you in 7 days. We need more information about this issue to help address it. Please be sure to give us your input. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!
This is supposed to be a runtime problem.
Are you saying that the app importing azure SDK will never cause invoking pkcs12.ToPEM()
at runtime?
Are you saying that the app importing azure SDK will never cause invoking
pkcs12.ToPEM()
at runtime?
Yes. azidentity
invokes pkcs12.ToPEM()
only when you call azidentity.ParseCertificates with a PKCS#12 (PFX) cert. That function exists to help apps get the []*x509.Certificate
and crypto.PrivateKey
for azidentity.NewClientCertificateCredential but you don't have to use it, the constructor parameters are simple standard library types you can get however you like.
This is supposed to be a runtime problem.
😅 Great! Then we have a couple straightforward workarounds:
azidentity.ParseCertificates
Hi @pavolloffay. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.
Feature Request
Related to https://github.com/Azure/azure-sdk-for-go/issues/21047
This library depends on x/crypto which does not have FIPS validated crypto algorithms:
Not FIPS compliant: pkcs12.ToPEM() performs a MAC check operation, for which it uses getSafeContents(), which calls verifyMac(), which calls pbkdf(), which is implemented in x/crypto, and thus not FIPS validated.
References:
Would you accept a patch to fix this?