From a3d0b2cbc99013423df800adb1f5c3002d66fce3 Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Tue, 18 Mar 2025 09:34:19 +0100 Subject: [PATCH 1/2] certificates: remove milliseconds from current time in GetHumanCertDetail --- .../src/k8s.io/apiserver/pkg/server/dynamiccertificates/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/util.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/util.go index 4e55a057833a2..2fb0a9ef303ac 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/util.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/util.go @@ -62,5 +62,5 @@ func GetHumanCertDetail(certificate *x509.Certificate) string { } return fmt.Sprintf("%q [%s]%s%s issuer=%q (%v to %v (now=%v))", humanName, strings.Join(usages, ","), groupString, servingString, signerHumanName, certificate.NotBefore.UTC(), certificate.NotAfter.UTC(), - time.Now().UTC()) + time.Now().Truncate(time.Second).UTC()) } From b4713f36fff537842d4132272f6881f29635fa1c Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Mon, 3 Mar 2025 18:15:34 +0100 Subject: [PATCH 2/2] authentication: when cert verification fails store details in audit log In some cases finding the invalid cert by its serial number is tedious. However we can't expose more PII in kube-apiserver logs. This commit will add an audit log annotations with additional details: common name, issuer common name and serial number --- .../pkg/authentication/request/x509/x509.go | 22 +++ test/integration/auth/certs_test.go | 150 ++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 test/integration/auth/certs_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go b/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go index fc827208bd625..5e17214eee915 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go @@ -30,6 +30,7 @@ import ( asn1util "k8s.io/apimachinery/pkg/apis/asn1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/features" @@ -38,6 +39,10 @@ import ( "k8s.io/component-base/metrics/legacyregistry" ) +const ( + CertificateErrorAuditAnnotation = "authentication.k8s.io/certificate-error" +) + /* * By default, the following metric is defined as falling under * ALPHA stability level https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/1209-metrics-stability/kubernetes-control-plane-metrics-stability.md#stability-classes) @@ -134,6 +139,15 @@ func NewDynamic(verifyOptionsFn VerifyOptionFunc, user UserConversion) *Authenti return &Authenticator{verifyOptionsFn, user} } +func certificateErrorIdentifier(c *x509.Certificate) string { + return fmt.Sprintf( + "CN=%s, Issuer=%s, SN=%d", + c.Subject.CommonName, + c.Issuer.CommonName, + c.SerialNumber, + )[:512] +} + // AuthenticateRequest authenticates the request using presented client certificates func (a *Authenticator) AuthenticateRequest(req *http.Request) (*authenticator.Response, bool, error) { if req.TLS == nil || len(req.TLS.PeerCertificates) == 0 { @@ -184,6 +198,14 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (*authenticator.R clientCertificateExpirationHistogram.WithContext(req.Context()).Observe(remaining.Seconds()) chains, err := req.TLS.PeerCertificates[0].Verify(optsCopy) if err != nil { + ctx := req.Context() + audit.AddAuditAnnotation(ctx, + CertificateErrorAuditAnnotation, + fmt.Sprintf("certificate %s failed: %v", certificateErrorIdentifier(req.TLS.PeerCertificates[0]), + err), + ) + req = req.WithContext(ctx) + return nil, false, fmt.Errorf( "verifying certificate %s failed: %w", certificateIdentifier(req.TLS.PeerCertificates[0]), diff --git a/test/integration/auth/certs_test.go b/test/integration/auth/certs_test.go new file mode 100644 index 0000000000000..f3e4806bb52a1 --- /dev/null +++ b/test/integration/auth/certs_test.go @@ -0,0 +1,150 @@ +package auth + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "os" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + auditinternal "k8s.io/apiserver/pkg/apis/audit" + auditv1 "k8s.io/apiserver/pkg/apis/audit/v1" + kubex509 "k8s.io/apiserver/pkg/authentication/request/x509" + + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + utiltesting "k8s.io/client-go/util/testing" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/test/integration/framework" + "k8s.io/kubernetes/test/utils" +) + +var ( + auditPolicy = ` +apiVersion: audit.k8s.io/v1 +kind: Policy +rules: + - level: Request + resources: + - group: "" + resources: ["pods"] + verbs: ["get"] +` +) + +func TestCerts(t *testing.T) { + logFile, err := os.CreateTemp("", "audit.log") + if err != nil { + t.Fatalf("Failed to create audit log file: %v", err) + } + defer utiltesting.CloseAndRemove(t, logFile) + + policyFile, err := os.CreateTemp("", "audit-policy.yaml") + if err != nil { + t.Fatalf("Failed to create audit policy file: %v", err) + } + if _, err := policyFile.Write([]byte(auditPolicy)); err != nil { + t.Fatalf("Failed to write audit policy file: %v", err) + } + + s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{ + "--audit-policy-file", policyFile.Name(), + "--audit-log-version", "audit.k8s.io/v1", + "--audit-log-mode", "blocking", + "--audit-log-path", logFile.Name(), + }, framework.SharedEtcd()) + defer s.TearDownFn() + + // Generate self-signed certificate + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + keyRaw, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatal(err) + } + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + t.Fatal(err) + } + commonName := "test-cn" + notBefore := time.Now().Truncate(time.Second) + notAfter := notBefore.Add(time.Hour) + cert := &x509.Certificate{ + SerialNumber: serialNumber, + Subject: pkix.Name{CommonName: commonName}, + NotBefore: notBefore, + NotAfter: notAfter, + + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + BasicConstraintsValid: true, + } + certRaw, err := x509.CreateCertificate(rand.Reader, cert, cert, key.Public(), key) + if err != nil { + t.Fatal(err) + } + + // Use self-signed certificate in client config + clientConfig := rest.CopyConfig(s.ClientConfig) + clientConfig.BearerToken = "" + clientConfig.CertData = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certRaw}) + clientConfig.KeyData = pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) + client, err := clientset.NewForConfig(clientConfig) + if err != nil { + t.Fatalf("error occurred: %v", err) + } + + // Make a request using the client + podName := "foobar" + currentTime := time.Now().Truncate(time.Second) + _, err = client.CoreV1().Pods("default").Get(context.TODO(), podName, metav1.GetOptions{}) + if err == nil { + t.Fatal("expected error, but it was nil") + } + + // Verify that audit log has the expected entry + stream, err := os.OpenFile(logFile.Name(), os.O_RDWR, 0600) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + defer stream.Close() + + annotationDetails := fmt.Sprintf(`certificate "%s" [client] issuer="" (%s to %s (now=%s)) failed: x509: certificate signed by unknown authority`, commonName, notBefore.UTC(), notAfter.UTC(), currentTime.UTC()) + expectedEvents := []utils.AuditEvent{ + { + Level: auditinternal.LevelRequest, + Stage: auditinternal.StageResponseStarted, + RequestURI: fmt.Sprintf("/api/v1/namespaces/default/pods/%s", podName), + Verb: "get", + Resource: "pods", + Namespace: "default", + Code: 401, + CustomAuditAnnotations: map[string]string{ + kubex509.CertificateErrorAuditAnnotation: annotationDetails, + }, + }, + } + + auditAnnotationFilter := func(key, val string) bool { + return key == kubex509.CertificateErrorAuditAnnotation + } + + missing, err := utils.CheckAuditLinesFiltered(stream, expectedEvents, auditv1.SchemeGroupVersion, auditAnnotationFilter) + if err != nil { + t.Errorf("unexpected error checking audit lines: %v", err) + } + if len(missing.MissingEvents) > 0 { + t.Errorf("failed to get expected events -- missing: %s", missing) + } +}