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.

Sign up for GitHub

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

Remove SHA1 for certs in prep for Go 1.18 #16455

Merged
merged 4 commits into from
Jul 28, 2022
Merged

Remove SHA1 for certs in prep for Go 1.18 #16455

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e7ba23c
Remove SHA1 for certs in prep for Go 1.18
May 24, 2022
5950b89
Add changelog
Jul 26, 2022
3be7a8e
Update comment about DSA1024
Jul 26, 2022
cd3c89b
Update builtin/credential/aws/pkcs7/sign_test.go
swenson Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion builtin/credential/aws/pkcs7/encrypt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ func TestEncrypt(t *testing.T) {
EncryptionAlgorithmAES256GCM,
}
sigalgs := []x509.SignatureAlgorithm{
x509.SHA1WithRSA,
x509.SHA256WithRSA,
x509.SHA512WithRSA,
}
Expand Down
43 changes: 0 additions & 43 deletions builtin/credential/aws/pkcs7/pkcs7_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,6 @@ func createTestCertificateByIssuer(name string, issuer *certKeyPair, sigAlg x509
issuerKey = *issuer.PrivateKey
}
switch sigAlg {
case x509.SHA1WithRSA:
priv = test1024Key
switch issuerKey.(type) {
case *rsa.PrivateKey:
template.SignatureAlgorithm = x509.SHA1WithRSA
case *ecdsa.PrivateKey:
template.SignatureAlgorithm = x509.ECDSAWithSHA1
case *dsa.PrivateKey:
template.SignatureAlgorithm = x509.DSAWithSHA1
}
case x509.SHA256WithRSA:
priv = test2048Key
switch issuerKey.(type) {
Expand Down Expand Up @@ -165,19 +155,6 @@ func createTestCertificateByIssuer(name string, issuer *certKeyPair, sigAlg x509
case *dsa.PrivateKey:
template.SignatureAlgorithm = x509.DSAWithSHA256
}
case x509.ECDSAWithSHA1:
priv, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
return nil, err
}
switch issuerKey.(type) {
case *rsa.PrivateKey:
template.SignatureAlgorithm = x509.SHA1WithRSA
case *ecdsa.PrivateKey:
template.SignatureAlgorithm = x509.ECDSAWithSHA1
case *dsa.PrivateKey:
template.SignatureAlgorithm = x509.DSAWithSHA1
}
case x509.ECDSAWithSHA256:
priv, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
Expand Down Expand Up @@ -217,26 +194,6 @@ func createTestCertificateByIssuer(name string, issuer *certKeyPair, sigAlg x509
case *dsa.PrivateKey:
template.SignatureAlgorithm = x509.DSAWithSHA256
}
case x509.DSAWithSHA1:
var dsaPriv dsa.PrivateKey
params := &dsaPriv.Parameters
err = dsa.GenerateParameters(params, rand.Reader, dsa.L1024N160)
if err != nil {
return nil, err
}
err = dsa.GenerateKey(&dsaPriv, rand.Reader)
if err != nil {
return nil, err
}
switch issuerKey.(type) {
case *rsa.PrivateKey:
template.SignatureAlgorithm = x509.SHA1WithRSA
case *ecdsa.PrivateKey:
template.SignatureAlgorithm = x509.ECDSAWithSHA1
case *dsa.PrivateKey:
template.SignatureAlgorithm = x509.DSAWithSHA1
}
priv = &dsaPriv
}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there should be a default case here that throws an error if sigAlg isn't one of the known cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly. But not critical I think since this is only in a _test.go file.

if isCA {
template.IsCA = true
Expand Down
4 changes: 2 additions & 2 deletions builtin/credential/aws/pkcs7/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type SignedData struct {
}

// NewSignedData takes data and initializes a PKCS7 SignedData struct that is
// ready to be signed via AddSigner. The digest algorithm is set to SHA1 by default
// ready to be signed via AddSigner. The digest algorithm is set to SHA-256 by default
// and can be changed by calling SetDigestAlgorithm.
func NewSignedData(data []byte) (*SignedData, error) {
content, err := asn1.Marshal(data)
Expand All @@ -39,7 +39,7 @@ func NewSignedData(data []byte) (*SignedData, error) {
ContentInfo: ci,
Version: 1,
}
return &SignedData{sd: sd, data: data, digestOid: OIDDigestAlgorithmSHA1}, nil
return &SignedData{sd: sd, data: data, digestOid: OIDDigestAlgorithmSHA256}, nil
}

// SignerInfoConfig are optional values to include when adding a signer
Expand Down
9 changes: 5 additions & 4 deletions builtin/credential/aws/pkcs7/sign_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ import (
func TestSign(t *testing.T) {
content := []byte("Hello World")
sigalgs := []x509.SignatureAlgorithm{
x509.SHA1WithRSA,
x509.SHA256WithRSA,
x509.SHA512WithRSA,
x509.ECDSAWithSHA1,
x509.ECDSAWithSHA256,
x509.ECDSAWithSHA384,
x509.ECDSAWithSHA512,
Expand Down Expand Up @@ -99,7 +97,7 @@ func TestDSASignAndVerifyWithOpenSSL(t *testing.T) {
}
ioutil.WriteFile(tmpContentFile.Name(), content, 0o755)

block, _ := pem.Decode([]byte(dsaPublicCert))
block, _ := pem.Decode(dsaPublicCert)
if block == nil {
t.Fatal("failed to parse certificate PEM")
}
Expand Down Expand Up @@ -129,6 +127,8 @@ func TestDSASignAndVerifyWithOpenSSL(t *testing.T) {
if err != nil {
t.Fatalf("test case: cannot initialize signed data: %s", err)
}
// openssl DSA only supports SHA1 for our 1024-bit DSA key, since that is all the standard officially supports
toBeSigned.digestOid = OIDDigestAlgorithmSHA1
if err := toBeSigned.SignWithoutAttr(signerCert, &priv, SignerInfoConfig{}); err != nil {
t.Fatalf("Cannot add signer: %s", err)
}
Expand All @@ -151,6 +151,7 @@ func TestDSASignAndVerifyWithOpenSSL(t *testing.T) {
"-content", tmpContentFile.Name())
out, err := opensslCMD.CombinedOutput()
if err != nil {
t.Errorf("Command: openssl smime -verify -noverify -in %s -inform PEM -content %s", tmpSignatureFile.Name(), tmpContentFile.Name())
swenson marked this conversation as resolved.
Show resolved Hide resolved
t.Fatalf("test case: openssl command failed with %s: %s", err, out)
}
os.Remove(tmpSignatureFile.Name()) // clean up
Expand Down Expand Up @@ -224,7 +225,7 @@ func TestUnmarshalSignedAttribute(t *testing.T) {
}

func TestDegenerateCertificate(t *testing.T) {
cert, err := createTestCertificate(x509.SHA1WithRSA)
cert, err := createTestCertificate(x509.SHA256WithRSA)
if err != nil {
t.Fatal(err)
}
Expand Down
0 ...n/credential/aws/pkcs7/verify_test_dsa.go → ...n/credential/aws/pkcs7/verify_dsa_test.go
View file
Open in desktop
File renamed without changes.
3 changes: 3 additions & 0 deletions changelog/16455.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
auth/aws: PKCS7 signatures will now use SHA256 by default in prep for Go 1.18
```
2 changes: 1 addition & 1 deletion website/content/partials/x509-sha1-deprecation.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
~> **Note**: This engine can use external X.509 certificates as part of TLS or signature validation.
Verifying signatures against X.509 certificates that use SHA-1 is deprecated and will no longer be
Verifying signatures against X.509 certificates that use SHA-1 is deprecated and is no longer
usable without a workaround starting in Vault 1.12. See the
[deprecation FAQ](/docs/deprecation/faq#q-what-is-the-impact-of-removing-support-for-x-509-certificates-with-signatures-that-use-sha-1)
for more information.