Skip to content

Commit

Permalink
Fix edge case with isCA
Browse files Browse the repository at this point in the history
Signed-off-by: Maartje Eyskens <maartje@eyskens.me>
  • Loading branch information
meyskens authored and jetstack-bot committed Sep 22, 2020
1 parent 26513a5 commit 772d3cc
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
16 changes: 16 additions & 0 deletions pkg/internal/apis/certmanager/validation/certificaterequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ func ValidateCertificateRequestSpec(crSpec *cmapi.CertificateRequestSpec, fldPat
} else {
// only compare usages if set on CR and in the CSR
if len(crSpec.Usages) > 0 && len(csr.Extensions) > 0 && validateCSRContent && !reflect.DeepEqual(crSpec.Usages, defaultInternalKeyUsages) {
if crSpec.IsCA {
crSpec.Usages = ensureCertSignIsSet(crSpec.Usages)
}
csrUsages, err := getCSRKeyUsage(crSpec, fldPath, csr, el)
if len(err) > 0 {
el = append(el, err...)
Expand Down Expand Up @@ -156,3 +159,16 @@ func isUsageEqual(a, b []cmapi.KeyUsage) bool {

return util.EqualUnsorted(aStrings, bStrings)
}

// ensureCertSignIsSet adds UsageCertSign in case it is not set
// TODO: add a mutating webhook to make sure this is always set
// when isCA is true.
func ensureCertSignIsSet(list []cmapi.KeyUsage) []cmapi.KeyUsage {
for _, usage := range list {
if usage == cmapi.UsageCertSign {
return list
}
}

return append(list, cmapi.UsageCertSign)
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,26 @@ func TestValidateCertificateRequestSpec(t *testing.T) {
},
want: []*field.Error{},
},
{
name: "Test csr that is CA with usages set",
crSpec: &cminternal.CertificateRequestSpec{
Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageCertSign), gen.SetCertificateIsCA(true))),
IssuerRef: validIssuerRef,
IsCA: true,
Usages: []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageCertSign},
},
want: []*field.Error{},
},
{
name: "Test csr that is CA but no cert sign in usages",
crSpec: &cminternal.CertificateRequestSpec{
Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageClientAuth, cmapi.UsageServerAuth), gen.SetCertificateIsCA(true))),
IssuerRef: validIssuerRef,
IsCA: true,
Usages: []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
},
want: []*field.Error{},
},
{
name: "Error on csr not having all usages",
crSpec: &cminternal.CertificateRequestSpec{
Expand Down

0 comments on commit 772d3cc

Please sign in to comment.