Remove client certificate authentication

This will not be used and it didn't work on OpenShift because the CAs
included in the Kubeconfig don't include the CA that signed the
client certificate. This would have required additional code to pull the
correct CA.

Signed-off-by: mprahl <[email protected]>

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,

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)

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

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=
@@ -366,8 +364,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=

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)
 

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())