Validate unknown and duplicate usages in CSR v1

This commit is contained in:
Jordan Liggitt 2020-06-04 13:13:26 -04:00
parent 14cb9217df
commit 595adc402a
2 changed files with 167 additions and 1 deletions

View File

@ -74,6 +74,10 @@ type certificateValidationOptions struct {
allowEmptyConditionType bool
// allow arbitrary content in status.certificate
allowArbitraryCertificate bool
// allow usages values outside the known set
allowUnknownUsages bool
// allow duplicate usages values
allowDuplicateUsages bool
}
// validateCSR validates the signature and formatting of a base64-wrapped,
@ -140,6 +144,34 @@ func ValidateCertificateSigningRequestCreate(csr *certificates.CertificateSignin
return validateCertificateSigningRequest(csr, opts)
}
var (
allValidUsages = sets.NewString(
string(certificates.UsageSigning),
string(certificates.UsageDigitalSignature),
string(certificates.UsageContentCommitment),
string(certificates.UsageKeyEncipherment),
string(certificates.UsageKeyAgreement),
string(certificates.UsageDataEncipherment),
string(certificates.UsageCertSign),
string(certificates.UsageCRLSign),
string(certificates.UsageEncipherOnly),
string(certificates.UsageDecipherOnly),
string(certificates.UsageAny),
string(certificates.UsageServerAuth),
string(certificates.UsageClientAuth),
string(certificates.UsageCodeSigning),
string(certificates.UsageEmailProtection),
string(certificates.UsageSMIME),
string(certificates.UsageIPsecEndSystem),
string(certificates.UsageIPsecTunnel),
string(certificates.UsageIPsecUser),
string(certificates.UsageTimestamping),
string(certificates.UsageOCSPSigning),
string(certificates.UsageMicrosoftSGC),
string(certificates.UsageNetscapeSGC),
)
)
func validateCertificateSigningRequest(csr *certificates.CertificateSigningRequest, opts certificateValidationOptions) field.ErrorList {
isNamespaced := false
allErrs := apivalidation.ValidateObjectMeta(&csr.ObjectMeta, isNamespaced, ValidateCertificateRequestName, field.NewPath("metadata"))
@ -152,6 +184,22 @@ func validateCertificateSigningRequest(csr *certificates.CertificateSigningReque
if len(csr.Spec.Usages) == 0 {
allErrs = append(allErrs, field.Required(specPath.Child("usages"), "usages must be provided"))
}
if !opts.allowUnknownUsages {
for i, usage := range csr.Spec.Usages {
if !allValidUsages.Has(string(usage)) {
allErrs = append(allErrs, field.NotSupported(specPath.Child("usages").Index(i), usage, allValidUsages.List()))
}
}
}
if !opts.allowDuplicateUsages {
seen := make(map[certificates.KeyUsage]bool, len(csr.Spec.Usages))
for i, usage := range csr.Spec.Usages {
if seen[usage] {
allErrs = append(allErrs, field.Duplicate(specPath.Child("usages").Index(i), usage))
}
seen[usage] = true
}
}
if !opts.allowLegacySignerName && csr.Spec.SignerName == certificates.LegacyUnknownSignerName {
allErrs = append(allErrs, field.Invalid(specPath.Child("signerName"), csr.Spec.SignerName, "the legacy signerName is not allowed via this API version"))
} else {
@ -377,6 +425,8 @@ func getValidationOptions(version schema.GroupVersion, newCSR, oldCSR *certifica
allowDuplicateConditionTypes: allowDuplicateConditionTypes(version, oldCSR),
allowEmptyConditionType: allowEmptyConditionType(version, oldCSR),
allowArbitraryCertificate: allowArbitraryCertificate(version, newCSR, oldCSR),
allowDuplicateUsages: allowDuplicateUsages(version, oldCSR),
allowUnknownUsages: allowUnknownUsages(version, oldCSR),
}
}
@ -465,3 +515,45 @@ func allowArbitraryCertificate(version schema.GroupVersion, newCSR, oldCSR *cert
return false
}
}
func allowUnknownUsages(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool {
switch {
case version == certificatesv1beta1.SchemeGroupVersion:
return true // compatibility with v1beta1
case oldCSR != nil && hasUnknownUsage(oldCSR.Spec.Usages):
return true // compatibility with existing data
default:
return false
}
}
func hasUnknownUsage(usages []certificates.KeyUsage) bool {
for _, usage := range usages {
if !allValidUsages.Has(string(usage)) {
return true
}
}
return false
}
func allowDuplicateUsages(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool {
switch {
case version == certificatesv1beta1.SchemeGroupVersion:
return true // compatibility with v1beta1
case oldCSR != nil && hasDuplicateUsage(oldCSR.Spec.Usages):
return true // compatibility with existing data
default:
return false
}
}
func hasDuplicateUsage(usages []certificates.KeyUsage) bool {
seen := make(map[certificates.KeyUsage]bool, len(usages))
for _, usage := range usages {
if seen[usage] {
return true
}
seen[usage] = true
}
return false
}

View File

@ -263,12 +263,53 @@ func TestValidateCertificateSigningRequestCreate(t *testing.T) {
},
errs: field.ErrorList{},
},
"missing usages": {
csr: capi.CertificateSigningRequest{
ObjectMeta: validObjectMeta,
Spec: capi.CertificateSigningRequestSpec{
Usages: []capi.KeyUsage{},
Request: newCSRPEM(t),
SignerName: validSignerName,
},
},
errs: field.ErrorList{
field.Required(specPath.Child("usages"), "usages must be provided"),
},
},
"unknown and duplicate usages - v1beta1": {
gv: schema.GroupVersion{Group: capi.SchemeGroupVersion.Group, Version: "v1beta1"},
csr: capi.CertificateSigningRequest{
ObjectMeta: validObjectMeta,
Spec: capi.CertificateSigningRequestSpec{
Usages: []capi.KeyUsage{"unknown", "unknown"},
Request: newCSRPEM(t),
SignerName: validSignerName,
},
},
errs: field.ErrorList{},
},
"unknown and duplicate usages - v1": {
gv: schema.GroupVersion{Group: capi.SchemeGroupVersion.Group, Version: "v1"},
csr: capi.CertificateSigningRequest{
ObjectMeta: validObjectMeta,
Spec: capi.CertificateSigningRequestSpec{
Usages: []capi.KeyUsage{"unknown", "unknown"},
Request: newCSRPEM(t),
SignerName: validSignerName,
},
},
errs: field.ErrorList{
field.NotSupported(specPath.Child("usages").Index(0), capi.KeyUsage("unknown"), allValidUsages.List()),
field.NotSupported(specPath.Child("usages").Index(1), capi.KeyUsage("unknown"), allValidUsages.List()),
field.Duplicate(specPath.Child("usages").Index(1), capi.KeyUsage("unknown")),
},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
el := ValidateCertificateSigningRequestCreate(&test.csr, test.gv)
if !reflect.DeepEqual(el, test.errs) {
t.Errorf("returned and expected errors did not match - expected %v but got %v", test.errs.ToAggregate(), el.ToAggregate())
t.Errorf("returned and expected errors did not match - expected\n%v\nbut got\n%v", test.errs.ToAggregate(), el.ToAggregate())
}
})
}
@ -331,6 +372,8 @@ func Test_getValidationOptions(t *testing.T) {
allowDuplicateConditionTypes: true,
allowEmptyConditionType: true,
allowArbitraryCertificate: true,
allowUnknownUsages: true,
allowDuplicateUsages: true,
},
},
{
@ -352,6 +395,8 @@ func Test_getValidationOptions(t *testing.T) {
allowDuplicateConditionTypes: true,
allowEmptyConditionType: true,
allowArbitraryCertificate: true,
allowUnknownUsages: true,
allowDuplicateUsages: true,
},
},
{
@ -424,6 +469,22 @@ func Test_getValidationOptions(t *testing.T) {
allowArbitraryCertificate: true,
},
},
{
name: "v1 compatible update, existing unknown usages",
version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{Usages: []capi.KeyUsage{"unknown"}}},
want: certificateValidationOptions{
allowUnknownUsages: true,
},
},
{
name: "v1 compatible update, existing duplicate usages",
version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{Usages: []capi.KeyUsage{"any", "any"}}},
want: certificateValidationOptions{
allowDuplicateUsages: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -587,6 +648,19 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) {
newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
},
{
name: "finalizer change with duplicate and unknown usages",
newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: capi.CertificateSigningRequestSpec{
Usages: []capi.KeyUsage{"unknown", "unknown"},
Request: newCSRPEM(t),
SignerName: validSignerName,
}},
oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: capi.CertificateSigningRequestSpec{
Usages: []capi.KeyUsage{"unknown", "unknown"},
Request: newCSRPEM(t),
SignerName: validSignerName,
}},
},
{
name: "add Approved condition",
newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{