diff --git a/controllers/complianceeventsapi/auth.go b/controllers/complianceeventsapi/auth.go index 0962cd16..7fe00a30 100644 --- a/controllers/complianceeventsapi/auth.go +++ b/controllers/complianceeventsapi/auth.go @@ -2,10 +2,8 @@ package complianceeventsapi import ( - "crypto/x509" "encoding/base64" "encoding/json" - "errors" "net/http" "slices" "strings" @@ -15,8 +13,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - apiserverx509 "k8s.io/apiserver/pkg/authentication/request/x509" - "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ) @@ -69,69 +65,10 @@ func parseToken(req *http.Request) string { return strings.TrimSpace(strings.TrimPrefix(req.Header.Get("Authorization"), "Bearer")) } -// canRecordComplianceEvent will perform certificate or token authentication and perform a subject access review to +// canRecordComplianceEvent will perform token authentication and perform a self subject access review to // ensure the input user has patch access to patch the policy status in the managed cluster namespace. An error is -// returned if the authorization could not be determined. Note that authenticatedClient and authenticator can be nil -// if certificate authentication isn't used. If both certificate and token authentication is present, certificate takes -// precedence. -func canRecordComplianceEvent( - cfg *rest.Config, - authenticatedClient *kubernetes.Clientset, - authenticator *apiserverx509.Authenticator, - clusterName string, - req *http.Request, -) (bool, error) { - postRules := authzv1.ResourceAttributes{ - Group: "policy.open-cluster-management.io", - Version: "v1", - Resource: "policies", - Verb: "patch", - Namespace: clusterName, - Subresource: "status", - } - - // req.TLS.PeerCertificates will be empty if certificate authentication is not enabled (e.g. endpoint is not HTTPS) - if req.TLS != nil && len(req.TLS.PeerCertificates) > 0 { - resp, ok, err := authenticator.AuthenticateRequest(req) - if err != nil { - if errors.As(err, &x509.UnknownAuthorityError{}) || errors.As(err, &x509.CertificateInvalidError{}) { - return false, ErrUnauthorized - } - - return false, err - } - - if !ok { - return false, ErrUnauthorized - } - - review, err := authenticatedClient.AuthorizationV1().SubjectAccessReviews().Create( - req.Context(), - &authzv1.SubjectAccessReview{ - Spec: authzv1.SubjectAccessReviewSpec{ - ResourceAttributes: &postRules, - User: resp.User.GetName(), - Groups: resp.User.GetGroups(), - UID: resp.User.GetUID(), - }, - }, - metav1.CreateOptions{}, - ) - if err != nil { - return false, err - } - - if !review.Status.Allowed { - log.V(0).Info( - "The user is not authorized to record a compliance event", - "cluster", clusterName, - "user", resp.User.GetName(), - ) - } - - return review.Status.Allowed, nil - } - +// returned if the authorization could not be determined. +func canRecordComplianceEvent(cfg *rest.Config, clusterName string, req *http.Request) (bool, error) { userConfig, err := getUserKubeConfig(cfg, req) if err != nil { return false, err @@ -146,7 +83,14 @@ func canRecordComplianceEvent( req.Context(), &authzv1.SelfSubjectAccessReview{ Spec: authzv1.SelfSubjectAccessReviewSpec{ - ResourceAttributes: &postRules, + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: "policy.open-cluster-management.io", + Version: "v1", + Resource: "policies", + Verb: "patch", + Namespace: clusterName, + Subresource: "status", + }, }, }, metav1.CreateOptions{}, @@ -203,21 +147,6 @@ func getTokenUsername(token string) string { return username } -// getCertAuthenticator returns an Authenticator that can validate that an input certificate is signed by the API -// server represented in the input clientAuthCAs and that the certificate can be used for client authentication -// (e.g. key usage). -func getCertAuthenticator(clientAuthCAs []byte) (*apiserverx509.Authenticator, error) { - p, err := dynamiccertificates.NewStaticCAContent("client-ca", clientAuthCAs) - if err != nil { - return nil, err - } - - // This is the same approach taken by kube-rbac-proxy. - authenticator := apiserverx509.NewDynamic(p.VerifyOptions, apiserverx509.CommonNameUserConversion) - - return authenticator, nil -} - func getUserKubeConfig(config *rest.Config, r *http.Request) (*rest.Config, error) { userConfig := &rest.Config{ Host: config.Host, diff --git a/controllers/complianceeventsapi/server.go b/controllers/complianceeventsapi/server.go index d2e1571b..79072fbe 100644 --- a/controllers/complianceeventsapi/server.go +++ b/controllers/complianceeventsapi/server.go @@ -23,10 +23,7 @@ import ( "time" "github.com/lib/pq" - apiserverx509 "k8s.io/apiserver/pkg/authentication/request/x509" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - certutil "k8s.io/client-go/util/cert" ) // init dynamically parses the database columns of each struct type to create a mapping of user provided sort/filter @@ -135,21 +132,17 @@ var ( ) type ComplianceAPIServer struct { - server *http.Server - addr string - clientAuthCAs []byte - cert *tls.Certificate - cfg *rest.Config + server *http.Server + addr string + cert *tls.Certificate + cfg *rest.Config } -func NewComplianceAPIServer( - listenAddress string, cfg *rest.Config, clientAuthCAs []byte, cert *tls.Certificate, -) *ComplianceAPIServer { +func NewComplianceAPIServer(listenAddress string, cfg *rest.Config, cert *tls.Certificate) *ComplianceAPIServer { return &ComplianceAPIServer{ - addr: listenAddress, - clientAuthCAs: clientAuthCAs, - cert: cert, - cfg: cfg, + addr: listenAddress, + cert: cert, + cfg: cfg, } } @@ -194,34 +187,13 @@ func (s *ComplianceAPIServer) Start(ctx context.Context, serverContext *Complian return err } - var authenticator *apiserverx509.Authenticator - var authenticatedClient *kubernetes.Clientset - if s.cert != nil { - clientAuthCAPool, err := certutil.NewPoolFromBytes(s.clientAuthCAs) - if err != nil { - return err - } - s.server.TLSConfig = &tls.Config{ MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{*s.cert}, - // Let the Kubernetes apiserver package validate it if the certificate is presented - ClientAuth: tls.RequestClientCert, - ClientCAs: clientAuthCAPool, } listener = tls.NewListener(listener, s.server.TLSConfig) - - authenticator, err = getCertAuthenticator(s.clientAuthCAs) - if err != nil { - return err - } - - authenticatedClient, err = kubernetes.NewForConfig(s.cfg) - if err != nil { - return err - } } // register handlers here @@ -250,7 +222,7 @@ func (s *ComplianceAPIServer) Start(ctx context.Context, serverContext *Complian } getComplianceEvents(serverContext.DB, w, r, userConfig) case http.MethodPost: - postComplianceEvent(serverContext.DB, s.cfg, authenticatedClient, authenticator, w, r) + postComplianceEvent(serverContext.DB, s.cfg, w, r) default: writeErrMsgJSON(w, "Method not allowed", http.StatusMethodNotAllowed) } @@ -1003,13 +975,7 @@ LEFT JOIN policies ON compliance_events.policy_id = policies.id` + whereClause / } } -func postComplianceEvent(db *sql.DB, - cfg *rest.Config, - authenticatedClient *kubernetes.Clientset, - authenticator *apiserverx509.Authenticator, - w http.ResponseWriter, - r *http.Request, -) { +func postComplianceEvent(db *sql.DB, cfg *rest.Config, w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) if err != nil { log.Error(err, "error reading request body") @@ -1032,7 +998,7 @@ func postComplianceEvent(db *sql.DB, return } - allowed, err := canRecordComplianceEvent(cfg, authenticatedClient, authenticator, reqEvent.Cluster.Name, r) + allowed, err := canRecordComplianceEvent(cfg, reqEvent.Cluster.Name, r) if err != nil { if errors.Is(err, ErrUnauthorized) { writeErrMsgJSON(w, "Unauthorized", http.StatusUnauthorized) diff --git a/go.mod b/go.mod index 3f439a9c..b694665d 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,6 @@ require ( github.com/stolostron/kubernetes-dependency-watches v0.5.2-0.20231212185913-628ab39622b8 k8s.io/api v0.27.7 k8s.io/apimachinery v0.27.7 - k8s.io/apiserver v0.27.7 k8s.io/client-go v0.27.7 k8s.io/klog/v2 v2.100.1 open-cluster-management.io/api v0.12.0 @@ -31,7 +30,6 @@ require ( github.com/Masterminds/semver/v3 v3.2.1 // indirect github.com/Masterminds/sprig/v3 v3.2.3 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/blang/semver/v4 v4.0.0 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect diff --git a/go.sum b/go.sum index 57b69d09..51198505 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,6 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= -github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= @@ -368,8 +366,6 @@ k8s.io/apiextensions-apiserver v0.27.7 h1:YqIOwZAUokzxJIjunmUd4zS1v3JhK34EPXn+pP k8s.io/apiextensions-apiserver v0.27.7/go.mod h1:x0p+b5a955lfPz9gaDeBy43obM12s+N9dNHK6+dUL+g= k8s.io/apimachinery v0.27.7 h1:Gxgtb7Y/Rsu8ymgmUEaiErkxa6RY4oTd8kNUI6SUR58= k8s.io/apimachinery v0.27.7/go.mod h1:jBGQgTjkw99ef6q5hv1YurDd3BqKDk9YRxmX0Ozo0i8= -k8s.io/apiserver v0.27.7 h1:E8sDHwfUug82YC1++qvE73QxihaXDqT4tr8XYBOEtc4= -k8s.io/apiserver v0.27.7/go.mod h1:OrLG9RwCOerutAlo8QJW5EHzUG9Dad7k6rgcDUNSO/w= k8s.io/client-go v0.27.7 h1:+Xgh9OOKv6A3qdD4Dnl/0VOI5EvAv+0s/OseDxVVTwQ= k8s.io/client-go v0.27.7/go.mod h1:dZ2kqcalYp5YZ2EV12XIMc77G6PxHWOJp/kclZr4+5Q= k8s.io/component-base v0.27.7 h1:kngM58HR9W9Nqpv7e4rpdRyWnKl/ABpUhLAZ+HoliMs= diff --git a/main.go b/main.go index e9dfcf66..6b0adc05 100644 --- a/main.go +++ b/main.go @@ -583,24 +583,8 @@ func startComplianceEventsAPI( reconciler.DynamicWatcher = dbSecretDynamicWatcher var cert *tls.Certificate - var clientAuthCAs []byte if complianceAPICert != "" && complianceAPIKey != "" { - if cfg.CAData == nil && cfg.CAFile == "" { - log.Info("The kubeconfig does not contain a CA") - os.Exit(1) - } - - if cfg.CAData == nil { - clientAuthCAs, err = os.ReadFile(cfg.CAFile) - if err != nil { - log.Error(err, "The kubeconfig does not contain a valid CA") - os.Exit(1) - } - } else { - clientAuthCAs = cfg.CAData - } - certTemp, err := tls.LoadX509KeyPair(complianceAPICert, complianceAPIKey) if err != nil { log.Error( @@ -617,7 +601,7 @@ func startComplianceEventsAPI( log.Info("The compliance events history API will listen on HTTP since no certificate was provided") } - complianceAPI := complianceeventsapi.NewComplianceAPIServer(complianceAPIAddr, cfg, clientAuthCAs, cert) + complianceAPI := complianceeventsapi.NewComplianceAPIServer(complianceAPIAddr, cfg, cert) wg.Add(1) diff --git a/test/e2e/case18_compliance_api_test.go b/test/e2e/case18_compliance_api_test.go index 42e8c55f..9badf9a8 100644 --- a/test/e2e/case18_compliance_api_test.go +++ b/test/e2e/case18_compliance_api_test.go @@ -131,7 +131,7 @@ var _ = Describe("Test the compliance events API", Label("compliance-events-api" err = complianceServerCtx.MigrateDB(ctx, k8sClient, "open-cluster-management") Expect(err).ToNot(HaveOccurred()) - complianceAPI := complianceeventsapi.NewComplianceAPIServer("localhost:8385", k8sConfig, nil, nil) + complianceAPI := complianceeventsapi.NewComplianceAPIServer("localhost:8385", k8sConfig, nil) httpCtx, httpCtxCancel := context.WithCancel(context.Background()) diff --git a/test/e2e/case20_compliance_api_controller_test.go b/test/e2e/case20_compliance_api_controller_test.go index 00721342..80f1bc08 100644 --- a/test/e2e/case20_compliance_api_controller_test.go +++ b/test/e2e/case20_compliance_api_controller_test.go @@ -5,14 +5,8 @@ package e2e import ( "bytes" "context" - cryptorand "crypto/rand" - "crypto/rsa" - "crypto/tls" - "crypto/x509" - "crypto/x509/pkix" "database/sql" "encoding/json" - "encoding/pem" "fmt" "io" "math/rand" @@ -22,13 +16,11 @@ import ( "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - certv1 "k8s.io/api/certificates/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - certutil "k8s.io/client-go/util/cert" "open-cluster-management.io/governance-policy-propagator/controllers/complianceeventsapi" "open-cluster-management.io/governance-policy-propagator/controllers/propagator" @@ -233,7 +225,6 @@ var _ = Describe("Test compliance events API authentication and authorization", eventsEndpoint := fmt.Sprintf("https://localhost:%d/api/v1/compliance-events", complianceAPIPort) const saName string = "compliance-api-user" var token string - var clientCert tls.Certificate getSamplePostRequest := func(clusterName string) *bytes.Buffer { payload := []byte(fmt.Sprintf(`{ @@ -339,63 +330,6 @@ var _ = Describe("Test compliance events API authentication and authorization", token = string(secret.Data["token"]) }, defaultTimeoutSeconds, 1).Should(Succeed()) - - privateKey, err := rsa.GenerateKey(cryptorand.Reader, 2048) - Expect(err).ToNot(HaveOccurred()) - - csr, err := certutil.MakeCSR( - privateKey, - &pkix.Name{CommonName: fmt.Sprintf("system:serviceaccount:%s:%s", testNamespace, saName)}, - nil, - nil, - ) - Expect(err).ToNot(HaveOccurred()) - - csrObj, err := clientHub.CertificatesV1().CertificateSigningRequests().Create( - ctx, - &certv1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - Name: saName, - }, - Spec: certv1.CertificateSigningRequestSpec{ - Request: csr, - SignerName: certv1.KubeAPIServerClientSignerName, - Usages: []certv1.KeyUsage{ - certv1.UsageDigitalSignature, - certv1.UsageKeyEncipherment, - certv1.UsageClientAuth, - }, - }, - }, - metav1.CreateOptions{}, - ) - Expect(err).ToNot(HaveOccurred()) - - csrObj.Status.Conditions = append(csrObj.Status.Conditions, certv1.CertificateSigningRequestCondition{ - Type: certv1.CertificateApproved, - Reason: "test", - Message: "This CSR was approved by the test", - LastUpdateTime: metav1.Now(), - Status: corev1.ConditionTrue, - }) - - _, err = clientHub.CertificatesV1().CertificateSigningRequests().UpdateApproval( - ctx, saName, csrObj, metav1.UpdateOptions{}, - ) - Expect(err).ToNot(HaveOccurred()) - - Eventually(func(g Gomega) { - csrObj, err = clientHub.CertificatesV1().CertificateSigningRequests().Get(ctx, saName, metav1.GetOptions{}) - g.Expect(csrObj.Status.Certificate).ToNot(BeNil()) - }, defaultTimeoutSeconds, 1).Should(Succeed()) - - keyPem := pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(privateKey), - }) - - clientCert, err = tls.X509KeyPair(csrObj.Status.Certificate, keyPem) - Expect(err).ToNot(HaveOccurred()) }) AfterAll(func(ctx context.Context) { @@ -420,11 +354,6 @@ var _ = Describe("Test compliance events API authentication and authorization", Expect(err).ToNot(HaveOccurred()) } - err = clientHub.CertificatesV1().CertificateSigningRequests().Delete(ctx, saName, metav1.DeleteOptions{}) - if !k8serrors.IsNotFound(err) { - Expect(err).ToNot(HaveOccurred()) - } - By("Deleting all database records") connectionURL := "postgresql://grc:grc@localhost:5432/ocm-compliance-history?sslmode=disable" db, err := sql.Open("postgres", connectionURL) @@ -459,7 +388,7 @@ var _ = Describe("Test compliance events API authentication and authorization", Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) }) - It("Rejects recording the compliance event for the wrong namespace (token auth)", func(ctx context.Context) { + It("Rejects recording the compliance event for the wrong namespace", func(ctx context.Context) { payload := getSamplePostRequest("cluster") req, err := http.NewRequestWithContext(ctx, http.MethodPost, eventsEndpoint, payload) @@ -477,61 +406,7 @@ var _ = Describe("Test compliance events API authentication and authorization", Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) }) - It("Rejects recording the compliance event for the wrong namespace (cert auth)", func(ctx context.Context) { - payload := getSamplePostRequest("cluster") - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, eventsEndpoint, payload) - Expect(err).ToNot(HaveOccurred()) - req.Header.Set("Content-Type", "application/json") - - httpClient := http.Client{ - Timeout: 30 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true, Certificates: []tls.Certificate{clientCert}}, - }, - } - - resp, err := httpClient.Do(req) - Expect(err).ToNot(HaveOccurred()) - - if resp != nil { - defer resp.Body.Close() - } - - Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) - }) - - It("Rejects recording the compliance event with cert signed by unknown CA (cert auth)", func(ctx context.Context) { - payload := getSamplePostRequest(testNamespace) - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, eventsEndpoint, payload) - Expect(err).ToNot(HaveOccurred()) - req.Header.Set("Content-Type", "application/json") - - cert, key, err := certutil.GenerateSelfSignedCertKey("test.local", nil, nil) - Expect(err).ToNot(HaveOccurred()) - - badClientCert, err := tls.X509KeyPair(cert, key) - Expect(err).ToNot(HaveOccurred()) - - httpClient := http.Client{ - Timeout: 30 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true, Certificates: []tls.Certificate{badClientCert}}, - }, - } - - resp, err := httpClient.Do(req) - Expect(err).ToNot(HaveOccurred()) - - if resp != nil { - defer resp.Body.Close() - } - - Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) - }) - - It("Allows recording the compliance event (token auth)", func(ctx context.Context) { + It("Allows recording the compliance event", func(ctx context.Context) { payload := getSamplePostRequest(testNamespace) req, err := http.NewRequestWithContext(ctx, http.MethodPost, eventsEndpoint, payload) @@ -548,28 +423,4 @@ var _ = Describe("Test compliance events API authentication and authorization", Expect(resp.StatusCode).To(Equal(http.StatusCreated)) }) - - It("Allows recording the compliance event (cert auth)", func(ctx context.Context) { - payload := getSamplePostRequest(testNamespace) - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, eventsEndpoint, payload) - Expect(err).ToNot(HaveOccurred()) - req.Header.Set("Content-Type", "application/json") - - httpClient := http.Client{ - Timeout: 30 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true, Certificates: []tls.Certificate{clientCert}}, - }, - } - - resp, err := httpClient.Do(req) - Expect(err).ToNot(HaveOccurred()) - - if resp != nil { - defer resp.Body.Close() - } - - Expect(resp.StatusCode).To(Equal(http.StatusCreated)) - }) })