diff --git a/.github/workflows/reusable-license-checker.yml b/.github/workflows/reusable-license-checker.yml index 941dd754..a09eae70 100644 --- a/.github/workflows/reusable-license-checker.yml +++ b/.github/workflows/reusable-license-checker.yml @@ -23,11 +23,14 @@ jobs: - name: Checkout uses: actions/checkout@v4 - name: Check license header - uses: apache/skywalking-eyes/header@v0.4.0 + uses: apache/skywalking-eyes/header@a790ab8dd23a7f861c18bd6aaa9b012e3a234bce + with: mode: check config: .github/licenserc.yml - name: Check dependencies license - uses: apache/skywalking-eyes/dependency@v0.4.0 + uses: apache/skywalking-eyes/dependency@a790ab8dd23a7f861c18bd6aaa9b012e3a234bce with: - config: .github/licenserc.yml \ No newline at end of file + config: .github/licenserc.yml + flags: + --weak-compatible=true \ No newline at end of file diff --git a/go.mod b/go.mod index 362a3874..0c1bb3c7 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/fxamacker/cbor/v2 v2.5.0 github.com/golang-jwt/jwt/v4 v4.5.0 github.com/veraison/go-cose v1.1.0 - golang.org/x/crypto v0.14.0 + golang.org/x/crypto v0.15.0 ) require github.com/x448/float16 v0.8.4 // indirect diff --git a/go.sum b/go.sum index 02edd738..48be8f87 100644 --- a/go.sum +++ b/go.sum @@ -6,5 +6,5 @@ github.com/veraison/go-cose v1.1.0 h1:AalPS4VGiKavpAzIlBjrn7bhqXiXi4jbMYY/2+UC+4 github.com/veraison/go-cose v1.1.0/go.mod h1:7ziE85vSq4ScFTg6wyoMXjucIGOf4JkFEZi/an96Ct4= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc= -golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= +golang.org/x/crypto v0.15.0 h1:frVn1TEaCEaZcn3Tmd7Y2b5KKPaZ+I32Q2OA3kYp5TA= +golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= diff --git a/x509/cert_validations.go b/x509/cert_validations.go index 84ba24dd..9e87c681 100644 --- a/x509/cert_validations.go +++ b/x509/cert_validations.go @@ -20,20 +20,10 @@ import ( "crypto/x509" "errors" "fmt" + "strings" "time" ) -var kuLeafCertBlocked = x509.KeyUsageContentCommitment | - x509.KeyUsageKeyEncipherment | - x509.KeyUsageDataEncipherment | - x509.KeyUsageKeyAgreement | - x509.KeyUsageCertSign | - x509.KeyUsageCRLSign | - x509.KeyUsageEncipherOnly | - x509.KeyUsageDecipherOnly -var kuLeafCertBlockedString = "ContentCommitment, KeyEncipherment, DataEncipherment, KeyAgreement, " + - "CertSign, CRLSign, EncipherOnly, DecipherOnly" - // ValidateCodeSigningCertChain takes an ordered code-signing certificate chain // and validates issuance from leaf to root // Validates certificates according to this spec: @@ -184,10 +174,36 @@ func validateLeafKeyUsage(cert *x509.Certificate) error { return err } if cert.KeyUsage&x509.KeyUsageDigitalSignature == 0 { - return fmt.Errorf("certificate with subject %q: key usage must have the bit positions for digital signature set", cert.Subject) + return fmt.Errorf("The certificate with subject %q is invalid. The key usage must have the bit positions for \"Digital Signature\" set", cert.Subject) + } + + var invalidKeyUsages []string + if cert.KeyUsage&x509.KeyUsageContentCommitment != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"ContentCommitment"`) + } + if cert.KeyUsage&x509.KeyUsageKeyEncipherment != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"KeyEncipherment"`) + } + if cert.KeyUsage&x509.KeyUsageDataEncipherment != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"DataEncipherment"`) + } + if cert.KeyUsage&x509.KeyUsageKeyAgreement != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"KeyAgreement"`) + } + if cert.KeyUsage&x509.KeyUsageCertSign != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"CertSign"`) + } + if cert.KeyUsage&x509.KeyUsageCRLSign != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"CRLSign"`) + } + if cert.KeyUsage&x509.KeyUsageEncipherOnly != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"EncipherOnly"`) + } + if cert.KeyUsage&x509.KeyUsageDecipherOnly != 0 { + invalidKeyUsages = append(invalidKeyUsages, `"DecipherOnly"`) } - if cert.KeyUsage&kuLeafCertBlocked != 0 { - return fmt.Errorf("certificate with subject %q: key usage must not have the bit positions for %s set", cert.Subject, kuLeafCertBlockedString) + if len(invalidKeyUsages) > 0 { + return fmt.Errorf("The certificate with subject %q is invalid. The key usage must be \"Digital Signature\" only, but found %s", cert.Subject, strings.Join(invalidKeyUsages, ", ")) } return nil } diff --git a/x509/cert_validations_test.go b/x509/cert_validations_test.go index 69446f2e..f0eb563a 100644 --- a/x509/cert_validations_test.go +++ b/x509/cert_validations_test.go @@ -15,7 +15,9 @@ package x509 import ( "crypto/x509" + "crypto/x509/pkix" _ "embed" + "encoding/asn1" "testing" "time" @@ -510,7 +512,7 @@ var kuNoDigitalSignatureLeaf = parseCertificateFromString(kuNoDigitalSignatureLe func TestFailKuNoDigitalSignatureLeaf(t *testing.T) { err := validateLeafCertificate(kuNoDigitalSignatureLeaf, x509.ExtKeyUsageCodeSigning) - assertErrorEqual("certificate with subject \"CN=Hello\": key usage must have the bit positions for digital signature set", err, t) + assertErrorEqual("The certificate with subject \"CN=Hello\" is invalid. The key usage must have the bit positions for \"Digital Signature\" set", err, t) } var kuWrongValuesLeafPem = "-----BEGIN CERTIFICATE-----\n" + @@ -534,7 +536,7 @@ var kuWrongValuesLeaf = parseCertificateFromString(kuWrongValuesLeafPem) func TestFailKuWrongValuesLeaf(t *testing.T) { err := validateLeafCertificate(kuWrongValuesLeaf, x509.ExtKeyUsageCodeSigning) - assertErrorEqual("certificate with subject \"CN=Hello\": key usage must not have the bit positions for ContentCommitment, KeyEncipherment, DataEncipherment, KeyAgreement, CertSign, CRLSign, EncipherOnly, DecipherOnly set", err, t) + assertErrorEqual("The certificate with subject \"CN=Hello\" is invalid. The key usage must be \"Digital Signature\" only, but found \"CertSign\", \"CRLSign\"", err, t) } var rsaKeyTooSmallLeafPem = "-----BEGIN CERTIFICATE-----\n" + @@ -699,3 +701,75 @@ func assertErrorEqual(expected string, err error, t *testing.T) { t.Fatalf("Expected error \"%v\" but was \"%v\"", expected, err) } } + +func TestValidateLeafKeyUsage(t *testing.T) { + extensions := []pkix.Extension{{ + Id: asn1.ObjectIdentifier{2, 5, 29, 15}, // OID for KeyUsage + Critical: true, + }} + + tests := []struct { + name string + cert *x509.Certificate + expectedErrMsg string + }{ + { + name: "Valid DigitalSignature usage", + cert: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test CN"}, + KeyUsage: x509.KeyUsageDigitalSignature, + Extensions: extensions, + }, + expectedErrMsg: "", + }, + { + name: "Valid ContentCommitment usage", + cert: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test CN"}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageContentCommitment, + Extensions: extensions, + }, + expectedErrMsg: "The certificate with subject \"CN=Test CN\" is invalid. The key usage must be \"Digital Signature\" only, but found \"ContentCommitment\"", + }, + { + name: "Missing DigitalSignature usage", + cert: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test CN"}, + KeyUsage: x509.KeyUsageCertSign, + Extensions: extensions, + }, + expectedErrMsg: "The certificate with subject \"CN=Test CN\" is invalid. The key usage must have the bit positions for \"Digital Signature\" set", + }, + { + name: "Invalid KeyEncipherment usage", + cert: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test CN"}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + Extensions: extensions, + }, + expectedErrMsg: "The certificate with subject \"CN=Test CN\" is invalid. The key usage must be \"Digital Signature\" only, but found \"KeyEncipherment\"", + }, + { + name: "Multiple Invalid usages", + cert: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test CN"}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageDataEncipherment | x509.KeyUsageKeyAgreement | x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageEncipherOnly | x509.KeyUsageDecipherOnly | x509.KeyUsageEncipherOnly | x509.KeyUsageDecipherOnly, + Extensions: extensions, + }, + expectedErrMsg: "The certificate with subject \"CN=Test CN\" is invalid. The key usage must be \"Digital Signature\" only, but found \"KeyEncipherment\", \"DataEncipherment\", \"KeyAgreement\", \"CertSign\", \"CRLSign\", \"EncipherOnly\", \"DecipherOnly\"", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateLeafKeyUsage(tt.cert) + if err != nil && tt.expectedErrMsg == "" { + t.Fatalf("expected no error, but got: %s", err) + } else if err == nil && tt.expectedErrMsg != "" { + t.Fatalf("expected error %q, but got none", tt.expectedErrMsg) + } else if err != nil && err.Error() != tt.expectedErrMsg { + t.Fatalf("expected error %q, but got: %s", tt.expectedErrMsg, err) + } + }) + } +}