Skip to content

Commit

Permalink
add SecretPublicKeysDiffersFromCurrentCertificateRequest check
Browse files Browse the repository at this point in the history
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
  • Loading branch information
inteon committed Jun 20, 2023
1 parent 2d01fbe commit fd72561
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 0 deletions.
25 changes: 25 additions & 0 deletions internal/controller/certificates/policies/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,31 @@ func SecretIssuerAnnotationsNotUpToDate(input Input) (string, string, bool) {
return "", "", false
}

func SecretPublicKeysDiffersFromCurrentCertificateRequest(input Input) (string, string, bool) {
if input.CurrentRevisionRequest == nil {
return "", "", false
}
pk, err := pki.DecodePrivateKeyBytes(input.Secret.Data[corev1.TLSPrivateKeyKey])
if err != nil {
return InvalidKeyPair, fmt.Sprintf("Issuing certificate as Secret contains invalid private key data: %v", err), true
}

csr, err := pki.DecodeX509CertificateRequestBytes(input.CurrentRevisionRequest.Spec.Request)
if err != nil {
return InvalidCertificateRequest, fmt.Sprintf("Failed to decode current CertificateRequest: %v", err), true
}

equal, err := pki.PublicKeysEqual(csr.PublicKey, pk.Public())
if err != nil {
return InvalidCertificateRequest, fmt.Sprintf("CertificateRequest's public key is invalid: %v", err), true
}
if !equal {
return InvalidCertificateRequest, "Secret contains a private key that does not match the current CertificateRequest", true
}

return "", "", false
}

func CurrentCertificateRequestNotValidForSpec(input Input) (string, string, bool) {
if input.CurrentRevisionRequest == nil {
// Fallback to comparing the Certificate spec with the issued certificate.
Expand Down
25 changes: 25 additions & 0 deletions internal/controller/certificates/policies/checks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,32 @@ func Test_NewTriggerPolicyChain(t *testing.T) {
},
},
reason: IncorrectIssuer,
reissue: true,
message: "Issuing certificate as Secret was previously issued by IssuerKind.new.example.com/testissuer",
},
"trigger issuance as current CertificateRequest is not signed with private key": {
certificate: &cmapi.Certificate{Spec: cmapi.CertificateSpec{SecretName: "something"}},
secret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "something"},
Data: map[string][]byte{
corev1.TLSPrivateKeyKey: staticFixedPrivateKey,
corev1.TLSCertKey: testcrypto.MustCreateCert(
t, staticFixedPrivateKey,
&cmapi.Certificate{Spec: cmapi.CertificateSpec{CommonName: "example.com"}},
),
},
},
request: &cmapi.CertificateRequest{Spec: cmapi.CertificateRequestSpec{
IssuerRef: cmmeta.ObjectReference{
Name: "testissuer",
Kind: "IssuerKind",
Group: "group.example.com",
},
Request: testcrypto.MustGenerateCSRImpl(t, testcrypto.MustCreatePEMPrivateKey(t), &cmapi.Certificate{Spec: cmapi.CertificateSpec{
CommonName: "example.com",
}}),
}},
reason: InvalidCertificateRequest,
message: "Secret contains a private key that does not match the current CertificateRequest",
reissue: true,
},
// we only have a basic test here for this as unit tests for the
Expand Down
4 changes: 4 additions & 0 deletions internal/controller/certificates/policies/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ const (
// InvalidCertificate is a policy violation whereby the signed certificate in
// the Input Secret could not be parsed or decoded.
InvalidCertificate string = "InvalidCertificate"
// InvalidCertificateRequest is a policy violation whereby the CSR in
// the Input CertificateRequest could not be parsed or decoded.
InvalidCertificateRequest string = "InvalidCertificateRequest"

// SecretMismatch is a policy violation reason for a scenario where Secret's
// private key does not match spec.
SecretMismatch string = "SecretMismatch"
Expand Down
2 changes: 2 additions & 0 deletions internal/controller/certificates/policies/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func NewTriggerPolicyChain(c clock.Clock) Chain {
SecretPublicKeysDiffer,
SecretPrivateKeyMatchesSpec,
SecretIssuerAnnotationsNotUpToDate,
SecretPublicKeysDiffersFromCurrentCertificateRequest,
CurrentCertificateRequestNotValidForSpec,
CurrentCertificateNearingExpiry(c),
}
Expand All @@ -84,6 +85,7 @@ func NewReadinessPolicyChain(c clock.Clock) Chain {
SecretDoesNotExist,
SecretIsMissingData,
SecretPublicKeysDiffer,
SecretPublicKeysDiffersFromCurrentCertificateRequest,
CurrentCertificateRequestNotValidForSpec,
CurrentCertificateHasExpired(c),
}
Expand Down

0 comments on commit fd72561

Please sign in to comment.