From fdb640b7a1324c2a4fc579389c4bc287ea90f1db Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 22 Jan 2021 10:16:24 -0800 Subject: [PATCH] crypto/x509: disable signing with MD5WithRSA MD5 is hopelessly broken, we already don't allow verification of MD5 signatures, we shouldn't support generating them. Fixes #42125 Change-Id: Ib25d750e6fc72a03198a505ac71e6d2c99eff2ed Reviewed-on: https://go-review.googlesource.com/c/go/+/285872 Run-TryBot: Roland Shoemaker Reviewed-by: Katie Hockman TryBot-Result: Gopher Robot Reviewed-by: Filippo Valsorda Reviewed-by: David Chase --- src/crypto/x509/x509.go | 15 ++++++--------- src/crypto/x509/x509_test.go | 4 ++-- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 582e1b15195..e17df0dd943 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -1397,6 +1397,10 @@ func signingParamsForPublicKey(pub any, requestedSigAlgo SignatureAlgorithm) (ha err = errors.New("x509: cannot sign with hash function requested") return } + if hashFunc == crypto.MD5 { + err = errors.New("x509: signing with MD5 is not supported") + return + } if requestedSigAlgo.isRSAPSS() { sigAlgo.Parameters = hashToPSSParameters[hashFunc] } @@ -1591,15 +1595,8 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv } // Check the signature to ensure the crypto.Signer behaved correctly. - sigAlg := getSignatureAlgorithmFromAI(signatureAlgorithm) - switch sigAlg { - case MD5WithRSA: - // We skip the check if the signature algorithm is only supported for - // signing, not verification. - default: - if err := checkSignature(sigAlg, c.Raw, signature, key.Public(), true); err != nil { - return nil, fmt.Errorf("x509: signature over certificate returned by signer is invalid: %w", err) - } + if err := checkSignature(getSignatureAlgorithmFromAI(signatureAlgorithm), c.Raw, signature, key.Public(), true); err != nil { + return nil, fmt.Errorf("x509: signature over certificate returned by signer is invalid: %w", err) } return signedCert, nil diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index f68dd0299a6..4469a42ce29 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -2929,8 +2929,8 @@ func TestCreateCertificateLegacy(t *testing.T) { SignatureAlgorithm: sigAlg, } _, err := CreateCertificate(rand.Reader, template, template, testPrivateKey.Public(), &brokenSigner{testPrivateKey.Public()}) - if err != nil { - t.Fatalf("CreateCertificate failed when SignatureAlgorithm = %v: %s", sigAlg, err) + if err == nil { + t.Fatal("CreateCertificate didn't fail when SignatureAlgorithm = MD5WithRSA") } }