diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/kas.go b/control-plane-operator/controllers/hostedcontrolplane/pki/kas.go index ec936f06cd8f..0a2305f92681 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/pki/kas.go +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/kas.go @@ -2,7 +2,6 @@ package pki import ( "fmt" - "net" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/clientcmd" @@ -14,6 +13,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/support/certs" "github.com/openshift/hypershift/support/config" + supportpki "github.com/openshift/hypershift/support/pki" "github.com/openshift/hypershift/support/util" utilsnet "k8s.io/utils/net" ) @@ -25,44 +25,9 @@ const ( ) func ReconcileKASServerCertSecret(secret, ca *corev1.Secret, ownerRef config.OwnerRef, externalAPIAddress, internalAPIAddress string, serviceCIDRs []string, nodeInternalAPIServerIP string) error { - svcAddresses := make([]string, 0) - - for _, serviceCIDR := range serviceCIDRs { - serviceIP, err := util.FirstUsableIP(serviceCIDR) - if err != nil { - return fmt.Errorf("cannot get the first usable IP from CIDR %s: %w", serviceIP, err) - } - svcAddresses = append(svcAddresses, serviceIP) - } - - dnsNames := []string{ - "localhost", - "kubernetes", - "kubernetes.default", - "kubernetes.default.svc", - "kubernetes.default.svc.cluster.local", - // This is needed to configure Openshift Auth Provider that talks to openshift.default.svc - "openshift", - "openshift.default", - "openshift.default.svc", - "openshift.default.svc.cluster.local", - } - apiServerIPs := []string{ - "127.0.0.1", - "0:0:0:0:0:0:0:1", - } - apiServerIPs = append(apiServerIPs, svcAddresses...) - apiServerIPs = append(apiServerIPs, nodeInternalAPIServerIP) - - if isNumericIP(externalAPIAddress) { - apiServerIPs = append(apiServerIPs, externalAPIAddress) - } else { - dnsNames = append(dnsNames, externalAPIAddress) - } - if isNumericIP(internalAPIAddress) { - apiServerIPs = append(apiServerIPs, internalAPIAddress) - } else { - dnsNames = append(dnsNames, internalAPIAddress) + dnsNames, apiServerIPs, err := supportpki.GetKASServerCertificatesSANs(externalAPIAddress, internalAPIAddress, serviceCIDRs, nodeInternalAPIServerIP) + if err != nil { + return fmt.Errorf("failed to get KAS server certificates SANs: %w", err) } return reconcileSignedCertWithAddresses(secret, ca, ownerRef, "kubernetes", []string{"kubernetes"}, X509UsageServerAuth, dnsNames, apiServerIPs) } @@ -114,10 +79,6 @@ func ReconcileServiceAccountKubeconfig(secret, csrSigner *corev1.Secret, ca *cor return ReconcileKubeConfig(secret, secret, ca, svcURL, "", manifests.KubeconfigScopeLocal, config.OwnerRef{}) } -func isNumericIP(s string) bool { - return net.ParseIP(s) != nil -} - func ReconcileKubeConfig(secret, cert *corev1.Secret, ca *corev1.ConfigMap, url string, key string, scope manifests.KubeconfigScope, ownerRef config.OwnerRef) error { ownerRef.ApplyTo(secret) caPEM := ca.Data[certs.CASignerCertMapKey] diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/konnectivity.go b/control-plane-operator/controllers/hostedcontrolplane/pki/konnectivity.go index e6e76196a63f..37a8d091fa2b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/pki/konnectivity.go +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/konnectivity.go @@ -4,6 +4,8 @@ import ( "fmt" "github.com/openshift/hypershift/support/config" + supportpki "github.com/openshift/hypershift/support/pki" + corev1 "k8s.io/api/core/v1" ) @@ -31,7 +33,7 @@ func ReconcileKonnectivityClusterSecret(secret, ca *corev1.Secret, ownerRef conf fmt.Sprintf("konnectivity-server.%s.svc.cluster.local", secret.Namespace), } ips := []string{} - if isNumericIP(externalKconnectivityAddress) { + if supportpki.IsNumericIP(externalKconnectivityAddress) { ips = append(ips, externalKconnectivityAddress) } else { dnsNames = append(dnsNames, externalKconnectivityAddress) diff --git a/go.mod b/go.mod index 71e562a203bc..327be3be9568 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/openshift/hypershift -go 1.22.0 +go 1.22.1 require ( github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 diff --git a/hack/workspace/go.work b/hack/workspace/go.work index 6f0fb7e85abf..8cd08d422554 100644 --- a/hack/workspace/go.work +++ b/hack/workspace/go.work @@ -1,4 +1,4 @@ -go 1.22.0 +go 1.22.1 use ( ./hypershift diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 7818284aed4e..18f45fadcb7a 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -87,6 +87,7 @@ import ( "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform" platformaws "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform/aws" hcmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/metrics" + "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/validations" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/clusterapi" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator" @@ -4168,6 +4169,10 @@ func (r *HostedClusterReconciler) validateConfigAndClusterCapabilities(ctx conte errs = append(errs, err...) } + if err := r.validateOCPConfigurations(ctx, hc, r.Client); err != nil { + errs = append(errs, err) + } + return utilerrors.NewAggregate(errs) } @@ -4478,6 +4483,20 @@ func (r *HostedClusterReconciler) validateNetworks(hc *hyperv1.HostedCluster) er return errs.ToAggregate() } +// validateOCPConfigurations validates OpenShift-specific configurations for a HostedCluster. +// It's worth to abstract this validation to a separate funtion per API to have them organized. +// Currently validates: +// - API Server configuration +// +// TODO: Add validation for other OpenShift components (e.g. OAuth, Ingress, etc.) +// Jira: https://issues.redhat.com/browse/CNTRLPLANE-382 +func (r *HostedClusterReconciler) validateOCPConfigurations(ctx context.Context, hc *hyperv1.HostedCluster, client client.Client) error { + var errs field.ErrorList + errs = append(errs, validations.ValidateOCPAPIServerSANs(ctx, hc, client)...) + + return errs.ToAggregate() +} + // findAdvertiseAddress function returns a string and an error indicating the AdvertiseAddress for the hostedcluster. // if the advertise address is properly set, it will return that value and nil, otherwise will return an error. // if the advertise address is not set, it will return the default one based on the network primary stack. diff --git a/hypershift-operator/controllers/hostedcluster/validations/ocpapiserver.go b/hypershift-operator/controllers/hostedcluster/validations/ocpapiserver.go new file mode 100644 index 000000000000..cbe7f0b133f8 --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/validations/ocpapiserver.go @@ -0,0 +1,155 @@ +package validations + +import ( + "context" + "encoding/pem" + "fmt" + "slices" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" + supportpki "github.com/openshift/hypershift/support/pki" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + KASServerPrivateCertSecretName = "kas-server-private-crt" + KASServerCertSecretName = "kas-server-crt" +) + +func ValidateOCPAPIServerSANs(ctx context.Context, hc *hyperv1.HostedCluster, client client.Client) field.ErrorList { + var ( + errs field.ErrorList + err error + entryCertDNSNames = make([]string, 0) + entryCertIPs = make([]string, 0) + kasNames = make([]string, 0) + kasIPs = make([]string, 0) + ) + + // At this point, maybe the HCP is not there yet + if hc.Spec.Configuration != nil && hc.Spec.Configuration.APIServer != nil && hc.Spec.Configuration.APIServer.ServingCerts.NamedCertificates != nil { + for _, cert := range hc.Spec.Configuration.APIServer.ServingCerts.NamedCertificates { + entryCertDNSNames = append(entryCertDNSNames, cert.Names...) + if len(cert.ServingCertificate.Name) > 0 { + secret := &corev1.Secret{} + err = client.Get(ctx, types.NamespacedName{Namespace: hc.Namespace, Name: cert.ServingCertificate.Name}, secret) + if err != nil { + errs = append(errs, field.Invalid(field.NewPath("NamedCertificates get secret"), cert.ServingCertificate.Name, err.Error())) + return errs + } + entryCertDNSNames, entryCertIPs, err = getSANsFromSecretCert(entryCertDNSNames, entryCertIPs, secret) + if err != nil { + errs = append(errs, field.Invalid(field.NewPath("KAS TLS private cert decrypt"), KASServerPrivateCertSecretName, err.Error())) + return errs + } + } + } + + kasNames, kasIPs, err = supportpki.GetKASServerCertificatesSANs("", fmt.Sprintf("api.%s.hypershift.local", hc.Name), []string{}, "") + if err != nil { + errs = append(errs, field.Invalid(field.NewPath("Hypershift KAS SANs"), entryCertDNSNames, err.Error())) + } + + if err := checkConflictingSANs(entryCertDNSNames, kasNames, "DNS names"); err != nil { + errs = append(errs, field.Invalid(field.NewPath("conflicting entries with KAS SANs"), entryCertDNSNames, err.Error())) + return errs + } + + if err := checkConflictingSANs(entryCertIPs, kasIPs, "IP addresses"); err != nil { + errs = append(errs, field.Invalid(field.NewPath("conflicting entries with KAS SANs"), entryCertIPs, err.Error())) + return errs + } + } + + hcpNamespace := manifests.HostedControlPlaneNamespace(hc.Namespace, hc.Name) + + // Check the KAS TLS private secret + kasServerPrivateSecret := &corev1.Secret{} + err = client.Get(ctx, types.NamespacedName{Namespace: hcpNamespace, Name: KASServerPrivateCertSecretName}, kasServerPrivateSecret) + if err != nil { + if !apierrors.IsNotFound(err) { + errs = append(errs, field.Invalid(field.NewPath("KAS TLS secret"), "error grabbing KAS TLS secret", err.Error())) + } + // return early, we can assume that the KAS is not there yet + return errs + } + kasNames, kasIPs, err = getSANsFromSecretCert(kasNames, kasIPs, kasServerPrivateSecret) + if err != nil { + errs = append(errs, field.Invalid(field.NewPath("KAS TLS cert decrypt"), KASServerPrivateCertSecretName, err.Error())) + } + + // Check the KAS TLS certificate secret + kasServerCertSecret := &corev1.Secret{} + err = client.Get(ctx, types.NamespacedName{Namespace: hcpNamespace, Name: KASServerCertSecretName}, kasServerCertSecret) + if err != nil { + if !apierrors.IsNotFound(err) { + errs = append(errs, field.Invalid(field.NewPath("KAS TLS secret"), "error grabbing KAS TLS secret", err.Error())) + } + // return early, we can assume that the KAS is not there yet + return errs + } + + kasNames, kasIPs, err = getSANsFromSecretCert(kasNames, kasIPs, kasServerCertSecret) + if err != nil { + errs = append(errs, field.Invalid(field.NewPath("KAS TLS cert decrypt"), KASServerCertSecretName, err.Error())) + } + + if err := checkConflictingSANs(entryCertDNSNames, kasNames, "DNS names"); err != nil { + errs = append(errs, field.Invalid(field.NewPath("custom serving cert"), entryCertDNSNames, err.Error())) + return errs + } + + if err := checkConflictingSANs(entryCertIPs, kasIPs, "IP addresses"); err != nil { + errs = append(errs, field.Invalid(field.NewPath("custom serving cert"), entryCertIPs, err.Error())) + return errs + } + + return errs +} + +func getSANsFromSecretCert(entryCertDNSNames []string, entryCertIPs []string, secretCert *corev1.Secret) ([]string, []string, error) { + if secretCert == nil || secretCert.Data == nil || len(secretCert.Data["tls.crt"]) == 0 { + return nil, nil, fmt.Errorf("TLS secret or certificate entries are empty") + } + + // Try to parse the certificate as PEM + block, _ := pem.Decode(secretCert.Data["tls.crt"]) + if block == nil { + return nil, nil, fmt.Errorf("failed to decode PEM block from certificate") + } + + certSANsDNS, certSANsIPs, err := supportpki.GetSANsFromCertificate(block.Bytes) + if err != nil { + return nil, nil, fmt.Errorf("error decrypting TLS certificate: %w", err) + } + + tempEntryCertDNSNames := appendEntriesIfNotExists(entryCertDNSNames, certSANsDNS) + tempEntryCertIPs := appendEntriesIfNotExists(entryCertIPs, certSANsIPs) + + return tempEntryCertDNSNames, tempEntryCertIPs, nil +} + +func appendEntriesIfNotExists(slice []string, entries []string) []string { + for _, entry := range entries { + if !slices.Contains(slice, entry) { + slice = append(slice, entry) + } + } + return slice +} + +func checkConflictingSANs(customEntries []string, kasSANEntries []string, entryType string) error { + for _, customEntry := range customEntries { + if slices.Contains(kasSANEntries, customEntry) { + return fmt.Errorf("conflicting %s found in KAS SANs. Configuration is invalid", entryType) + } + } + return nil +} diff --git a/hypershift-operator/controllers/hostedcluster/validations/ocpapiserver_test.go b/hypershift-operator/controllers/hostedcluster/validations/ocpapiserver_test.go new file mode 100644 index 000000000000..d67052fccd51 --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/validations/ocpapiserver_test.go @@ -0,0 +1,417 @@ +package validations + +import ( + "context" + "fmt" + "testing" + "time" + + . "github.com/onsi/gomega" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/test/util" + + configv1 "github.com/openshift/api/config/v1" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + customServingCertSecretName = "custom-serving-cert" +) + +func TestValidateOCPAPIServerSANs(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + _ = hyperv1.AddToScheme(scheme) + _ = configv1.AddToScheme(scheme) + + kasServerCertSecret := sampleKASServerCertSecret() + kasServerPrivateCertSecret := sampleKASServerPrivateCertSecret() + + // Get a sample HostedCluster + hc := sampleHostedCluster() + + tests := []struct { + name string + customCertSecret *corev1.Secret + secrets []client.Object + namedCertificates []configv1.APIServerNamedServingCert + expectedErrors field.ErrorList + dnsNames []string + ipAddresses []string + }{ + { + name: "custom serving cert, hcp deployed, valid configuration with no conflicts", + customCertSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: customServingCertSecretName, + Namespace: "clusters", + }, + }, + dnsNames: []string{"test.example.com"}, + ipAddresses: []string{"192.168.1.1"}, + secrets: []client.Object{ + kasServerCertSecret, + kasServerPrivateCertSecret, + }, + namedCertificates: []configv1.APIServerNamedServingCert{ + { + Names: []string{"test.example.com"}, + ServingCertificate: configv1.SecretNameReference{Name: customServingCertSecretName}, + }, + }, + expectedErrors: nil, + }, + { + name: "custom serving cert, hcp not deployed, valid configuration with no conflicts", + customCertSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: customServingCertSecretName, + Namespace: "clusters", + }, + }, + dnsNames: []string{"test.example.com"}, + secrets: []client.Object{}, + namedCertificates: []configv1.APIServerNamedServingCert{ + { + Names: []string{"test.example.com"}, + ServingCertificate: configv1.SecretNameReference{Name: customServingCertSecretName}, + }, + }, + expectedErrors: nil, + }, + { + name: "invalid certificate format", + customCertSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: customServingCertSecretName, + Namespace: "clusters", + }, + Data: map[string][]byte{ + "tls.crt": []byte("invalid certificate"), + }, + }, + secrets: []client.Object{}, + namedCertificates: []configv1.APIServerNamedServingCert{ + { + Names: []string{"test.example.com"}, + ServingCertificate: configv1.SecretNameReference{Name: customServingCertSecretName}, + }, + }, + expectedErrors: field.ErrorList{ + field.Invalid(field.NewPath("KAS TLS private cert decrypt"), KASServerPrivateCertSecretName, "failed to decode PEM block from certificate"), + }, + }, + { + name: "missing secret", + namedCertificates: []configv1.APIServerNamedServingCert{ + { + Names: []string{"test.example.com"}, + ServingCertificate: configv1.SecretNameReference{Name: customServingCertSecretName}, + }, + }, + expectedErrors: field.ErrorList{ + field.Invalid(field.NewPath("NamedCertificates get secret"), "custom-serving-cert", "secrets \"custom-serving-cert\" not found"), + }, + }, + { + name: "no custom serving cert, hcp not deployed, valid configuration with no conflicts", + secrets: []client.Object{}, + expectedErrors: nil, + }, + { + name: "conflicting SANs with KAS", + + customCertSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: customServingCertSecretName, + Namespace: "clusters", + }, + }, + dnsNames: []string{"test-conflicting-kas-san.example.com"}, + secrets: []client.Object{ + kasServerCertSecret, + kasServerPrivateCertSecret, + }, + namedCertificates: []configv1.APIServerNamedServingCert{ + { + Names: []string{"test-conflicting-kas-san.example.com"}, + ServingCertificate: configv1.SecretNameReference{Name: customServingCertSecretName}, + }, + }, + expectedErrors: field.ErrorList{ + field.Invalid(field.NewPath("custom serving cert"), []string{"test-conflicting-kas-san.example.com"}, "conflicting DNS names found in KAS SANs. Configuration is invalid"), + }, + }, + { + name: "invalid certificate data", + customCertSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: customServingCertSecretName, + Namespace: "clusters", + }, + Data: map[string][]byte{ + "tls.crt": []byte("invalid certificate data"), + }, + }, + secrets: []client.Object{ + kasServerCertSecret, + kasServerPrivateCertSecret, + }, + namedCertificates: []configv1.APIServerNamedServingCert{ + { + Names: []string{"test.example.com"}, + ServingCertificate: configv1.SecretNameReference{Name: customServingCertSecretName}, + }, + }, + expectedErrors: field.ErrorList{ + field.Invalid(field.NewPath("KAS TLS private cert decrypt"), KASServerPrivateCertSecretName, "failed to decode PEM block from certificate"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var ( + pemCert []byte + pemKey []byte + err error + g = NewWithT(t) + ) + + if len(tt.namedCertificates) > 0 && (tt.customCertSecret != nil && tt.customCertSecret.Data == nil) { + // Generate a test certificate + pemCert, pemKey, err = util.GenerateTestCertificate(context.Background(), tt.dnsNames, tt.ipAddresses, 24*time.Hour) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(pemCert).NotTo(BeNil()) + } + + // Initialize secrets with proper data + objects := make([]client.Object, 0) + if tt.customCertSecret != nil { + if tt.customCertSecret.Data == nil { + tt.customCertSecret.Data = make(map[string][]byte) + tt.customCertSecret.Data["tls.crt"] = pemCert + tt.customCertSecret.Data["tls.key"] = pemKey + } + objects = append(objects, tt.customCertSecret) + } + + // Add KAS secrets if they are in the test case + if len(tt.secrets) > 0 { + objects = append(objects, tt.secrets...) + } + + if len(tt.namedCertificates) > 0 { + hc.Spec.Configuration.APIServer.ServingCerts.NamedCertificates = tt.namedCertificates + objects = append(objects, hc) + } + + // Create a new client with the scheme and objects + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objects...). + Build() + + // Verify that the secrets were added correctly + for _, object := range objects { + switch object.(type) { + case *corev1.Secret: + var foundSecret corev1.Secret + err := fakeClient.Get(context.Background(), types.NamespacedName{ + Name: object.GetName(), + Namespace: object.GetNamespace(), + }, &foundSecret) + g.Expect(err).ToNot(HaveOccurred(), "failed to get secret %s/%s", object.GetNamespace(), object.GetName()) + g.Expect(foundSecret.Data).ToNot(BeNil(), "secret data should not be nil") + g.Expect(foundSecret.Data["tls.crt"]).ToNot(BeEmpty(), "certificate data should not be empty") + } + } + + if hc.Spec.Configuration == nil { + hc.Spec.Configuration = &hyperv1.ClusterConfiguration{} + } + if hc.Spec.Configuration.APIServer == nil { + hc.Spec.Configuration.APIServer = &configv1.APIServerSpec{} + } + if hc.Spec.Configuration.APIServer.ServingCerts.NamedCertificates == nil { + hc.Spec.Configuration.APIServer.ServingCerts.NamedCertificates = []configv1.APIServerNamedServingCert{} + } + hc.Spec.Configuration.APIServer.ServingCerts.NamedCertificates = tt.namedCertificates + errs := ValidateOCPAPIServerSANs(context.Background(), hc, fakeClient) + g.Expect(errs).To(Equal(tt.expectedErrors)) + }) + } +} + +func TestAppendEntriesIfNotExists(t *testing.T) { + tests := []struct { + name string + slice []string + entries []string + expected []string + }{ + { + name: "empty slice and entries", + slice: []string{}, + entries: []string{}, + expected: []string{}, + }, + { + name: "add new entries", + slice: []string{"a", "b"}, + entries: []string{"c", "d"}, + expected: []string{"a", "b", "c", "d"}, + }, + { + name: "add existing and new entries", + slice: []string{"a", "b"}, + entries: []string{"b", "c"}, + expected: []string{"a", "b", "c"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + result := appendEntriesIfNotExists(tt.slice, tt.entries) + g.Expect(result).To(Equal(tt.expected)) + }) + } +} + +func TestCheckConflictingSANs(t *testing.T) { + tests := []struct { + name string + customEntries []string + kasSANEntries []string + entryType string + expectError bool + }{ + { + name: "no conflicts", + customEntries: []string{"a", "b"}, + kasSANEntries: []string{"c", "d"}, + entryType: "DNS names", + expectError: false, + }, + { + name: "has conflicts", + customEntries: []string{"a", "b"}, + kasSANEntries: []string{"b", "c"}, + entryType: "DNS names", + expectError: true, + }, + { + name: "empty entries", + customEntries: []string{}, + kasSANEntries: []string{}, + entryType: "DNS names", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + err := checkConflictingSANs(tt.customEntries, tt.kasSANEntries, tt.entryType) + if tt.expectError { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("conflicting")) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + +func sampleKASServerCertSecret() *corev1.Secret { + pemCert, pemKey, err := util.GenerateTestCertificate( + context.Background(), + []string{ + "kube-apiserver", + "kube-apiserver.clusters-jparrill-hosted.svc", + "kube-apiserver.clusters-jparrill-hosted.svc.cluster.local", + "test-conflicting-kas-san.example.com", + }, + []string{}, + 24*time.Hour, + ) + if err != nil { + panic(fmt.Sprintf("failed to generate KAS server certificate: %v", err)) + } + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: KASServerCertSecretName, + Namespace: "clusters-test", + }, + Data: map[string][]byte{ + "tls.crt": pemCert, + "tls.key": pemKey, + }, + Type: corev1.SecretTypeTLS, + } +} + +func sampleKASServerPrivateCertSecret() *corev1.Secret { + pemCert, pemKey, err := util.GenerateTestCertificate( + context.Background(), + []string{ + "localhost", + "kubernetes", + "kubernetes.default", + "kubernetes.default.svc", + "kubernetes.default.svc.cluster.local", + "openshift", + "openshift.default", + "openshift.default.svc", + "openshift.default.svc.cluster.local", + }, + []string{"127.0.0.1", "::1"}, + 24*time.Hour, + ) + if err != nil { + panic(fmt.Sprintf("failed to generate KAS private certificate: %v", err)) + } + + // Create a PEM block for the certificate + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: KASServerPrivateCertSecretName, + Namespace: "clusters-test", + }, + Data: map[string][]byte{ + "tls.crt": pemCert, + "tls.key": pemKey, + }, + Type: corev1.SecretTypeTLS, + } +} + +func sampleHostedCluster() *hyperv1.HostedCluster { + return &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "clusters", + }, + Spec: hyperv1.HostedClusterSpec{ + Configuration: &hyperv1.ClusterConfiguration{ + APIServer: &configv1.APIServerSpec{ + ServingCerts: configv1.APIServerServingCerts{ + NamedCertificates: []configv1.APIServerNamedServingCert{}, + }, + }, + }, + }, + } +} diff --git a/support/pki/kas.go b/support/pki/kas.go new file mode 100644 index 000000000000..0001bbf4969d --- /dev/null +++ b/support/pki/kas.go @@ -0,0 +1,75 @@ +package pki + +import ( + "crypto/x509" + "fmt" + "net" + + "github.com/openshift/hypershift/support/util" +) + +func GetKASServerCertificatesSANs(externalAPIAddress, internalAPIAddress string, serviceCIDRs []string, nodeInternalAPIServerIP string) ([]string, []string, error) { + svcAddresses := make([]string, 0) + + for _, serviceCIDR := range serviceCIDRs { + serviceIP, err := util.FirstUsableIP(serviceCIDR) + if err != nil { + return nil, nil, fmt.Errorf("cannot get the first usable IP from CIDR %s: %w", serviceIP, err) + } + svcAddresses = append(svcAddresses, serviceIP) + } + + dnsNames := []string{ + "localhost", + "kubernetes", + "kubernetes.default", + "kubernetes.default.svc", + "kubernetes.default.svc.cluster.local", + // This is needed to configure Openshift Auth Provider that talks to openshift.default.svc + "openshift", + "openshift.default", + "openshift.default.svc", + "openshift.default.svc.cluster.local", + } + apiServerIPs := []string{ + "127.0.0.1", + "0:0:0:0:0:0:0:1", + } + apiServerIPs = append(apiServerIPs, svcAddresses...) + apiServerIPs = append(apiServerIPs, nodeInternalAPIServerIP) + + if IsNumericIP(externalAPIAddress) { + apiServerIPs = append(apiServerIPs, externalAPIAddress) + } else { + dnsNames = append(dnsNames, externalAPIAddress) + } + if IsNumericIP(internalAPIAddress) { + apiServerIPs = append(apiServerIPs, internalAPIAddress) + } else { + dnsNames = append(dnsNames, internalAPIAddress) + } + + return dnsNames, apiServerIPs, nil +} + +func IsNumericIP(s string) bool { + return net.ParseIP(s) != nil +} + +// GetSANsFromCertificate returns the SANs from a certificate as separate DNS names and IP addresses +func GetSANsFromCertificate(cert []byte) ([]string, []string, error) { + parsedCert, err := x509.ParseCertificate(cert) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse certificate: %w", err) + } + + dnsNames := make([]string, len(parsedCert.DNSNames)) + copy(dnsNames, parsedCert.DNSNames) + + ipAddresses := make([]string, 0, len(parsedCert.IPAddresses)) + for _, ip := range parsedCert.IPAddresses { + ipAddresses = append(ipAddresses, ip.String()) + } + + return dnsNames, ipAddresses, nil +} diff --git a/support/pki/kas_test.go b/support/pki/kas_test.go new file mode 100644 index 000000000000..163e500e11a2 --- /dev/null +++ b/support/pki/kas_test.go @@ -0,0 +1,88 @@ +package pki + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "net" + "testing" + "time" + + . "github.com/onsi/gomega" +) + +func TestGetSANsFromCertificate(t *testing.T) { + g := NewWithT(t) + // Create a test certificate with known SANs + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + Organization: []string{"Test Org"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour * 24), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + DNSNames: []string{ + "test.example.com", + "*.test.example.com", + }, + IPAddresses: []net.IP{ + net.ParseIP("192.168.1.1"), + net.ParseIP("2001:db8::1"), + }, + } + + // Generate a private key + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + g.Expect(err).ToNot(HaveOccurred()) + + // Create the certificate + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &privateKey.PublicKey, privateKey) + g.Expect(err).ToNot(HaveOccurred()) + + // Test cases + testCases := []struct { + name string + cert []byte + expectedDNS []string + expectedIPs []string + expectedError bool + }{ + { + name: "valid certificate", + cert: certDER, + expectedDNS: []string{"test.example.com", "*.test.example.com"}, + expectedIPs: []string{"192.168.1.1", "2001:db8::1"}, + }, + { + name: "invalid certificate", + cert: []byte("invalid certificate"), + expectedError: true, + }, + { + name: "empty certificate", + cert: []byte{}, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + dnsNames, ipAddresses, err := GetSANsFromCertificate(tc.cert) + + if tc.expectedError { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(dnsNames).To(ConsistOf(tc.expectedDNS), "DNS names don't match") + g.Expect(ipAddresses).To(ConsistOf(tc.expectedIPs), "IP addresses don't match") + }) + } +} diff --git a/test/util/pki.go b/test/util/pki.go new file mode 100644 index 000000000000..ff5a648a8f4c --- /dev/null +++ b/test/util/pki.go @@ -0,0 +1,91 @@ +package util + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "time" +) + +// GenerateTestCertificate creates a test certificate with the given DNS names and IP addresses +func GenerateTestCertificate(ctx context.Context, dnsNames []string, ipAddresses []string, duration time.Duration) ([]byte, []byte, error) { + var cn string + + // Generate a private key + if len(dnsNames) == 0 && len(ipAddresses) == 0 { + return nil, nil, fmt.Errorf("no DNS names or IP addresses provided") + } + + // set the common name to the first DNS name or ip address + if len(dnsNames) > 0 { + cn = dnsNames[0] + } else { + cn = ipAddresses[0] + } + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, nil, err + } + + // Convert IP addresses to net.IP + ips := make([]net.IP, len(ipAddresses)) + for i, ip := range ipAddresses { + ips[i] = net.ParseIP(ip) + } + + // Create certificate template + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + Organization: []string{"HostedControlPlanes TestOrg"}, + CommonName: cn, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(duration), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + DNSNames: dnsNames, + IPAddresses: ips, + IsCA: true, + MaxPathLen: 0, + MaxPathLenZero: true, + } + + // Create the certificate + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &privateKey.PublicKey, privateKey) + if err != nil { + return nil, nil, fmt.Errorf("failed to create certificate: %w", err) + } + + // Validate the certificate by parsing it + _, err = x509.ParseCertificate(certDER) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse generated certificate: %w", err) + } + + // Convert private key to PEM format + privateKeyPEM := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(privateKey), + }) + + // Convert certificate to PEM format + certPEM := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: certDER, + }) + + // Add newline to both PEM blocks + privateKeyPEM = append(privateKeyPEM, '\n') + certPEM = append(certPEM, '\n') + + return certPEM, privateKeyPEM, nil +} diff --git a/test/util/pki_test.go b/test/util/pki_test.go new file mode 100644 index 000000000000..15b0170d360b --- /dev/null +++ b/test/util/pki_test.go @@ -0,0 +1,137 @@ +package util + +import ( + "context" + "crypto/x509" + "encoding/pem" + "testing" + "time" + + . "github.com/onsi/gomega" + + "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/go-logr/logr/testr" +) + +func TestGenerateTestCertificate(t *testing.T) { + ctx := log.IntoContext(context.Background(), testr.New(t)) + + testsCases := []struct { + name string + dnsNames []string + ipAddresses []string + duration time.Duration + wantErr bool + expectedCN string + }{ + { + name: "When generating a certificate with DNS names it should succeed", + dnsNames: []string{"example.com", "test.example.com"}, + ipAddresses: []string{"192.168.1.1"}, + duration: 24 * time.Hour, + wantErr: false, + expectedCN: "example.com", + }, + { + name: "When generating a certificate with IP addresses only it should succeed", + dnsNames: []string{}, + ipAddresses: []string{"192.168.1.1", "10.0.0.1"}, + duration: 24 * time.Hour, + wantErr: false, + expectedCN: "192.168.1.1", + }, + { + name: "When generating a certificate with no DNS names or IP addresses it should fail", + dnsNames: []string{}, + ipAddresses: []string{}, + duration: 24 * time.Hour, + wantErr: true, + }, + { + name: "When generating a certificate with invalid IP address it should fail", + dnsNames: []string{}, + ipAddresses: []string{"invalid.ip.address"}, + duration: 24 * time.Hour, + wantErr: true, + }, + { + name: "When generating a certificate with zero duration it should succeed", + dnsNames: []string{"example.com"}, + ipAddresses: []string{"192.168.1.1"}, + duration: 0, + wantErr: false, + expectedCN: "example.com", + }, + } + + for _, tc := range testsCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + certPEM, keyPEM, err := GenerateTestCertificate(ctx, tc.dnsNames, tc.ipAddresses, tc.duration) + + if tc.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(certPEM).NotTo(BeNil()) + g.Expect(keyPEM).NotTo(BeNil()) + + // Verify PEM blocks have newlines at the end + g.Expect(certPEM[len(certPEM)-1]).To(Equal(byte('\n'))) + g.Expect(keyPEM[len(keyPEM)-1]).To(Equal(byte('\n'))) + + // Parse the certificate to verify its contents + block, _ := pem.Decode(certPEM) + g.Expect(block).NotTo(BeNil()) + g.Expect(block.Type).To(Equal("CERTIFICATE")) + + cert, err := x509.ParseCertificate(block.Bytes) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify CommonName + g.Expect(cert.Subject.CommonName).To(Equal(tc.expectedCN)) + + // Verify DNS names + if len(tc.dnsNames) == 0 { + g.Expect(cert.DNSNames).To(BeEmpty()) + } else { + g.Expect(cert.DNSNames).To(Equal(tc.dnsNames)) + } + + // Verify IP addresses + if len(tc.ipAddresses) > 0 { + g.Expect(cert.IPAddresses).To(HaveLen(len(tc.ipAddresses))) + } + + // Verify certificate is self-signed + g.Expect(cert.Subject.CommonName).To(Equal(cert.Issuer.CommonName)) + g.Expect(cert.Subject.Organization).To(Equal(cert.Issuer.Organization)) + + // Verify certificate is a CA + g.Expect(cert.IsCA).To(BeTrue()) + g.Expect(cert.MaxPathLen).To(Equal(0)) + g.Expect(cert.MaxPathLenZero).To(BeTrue()) + + // Verify key usage + expectedKeyUsage := x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature + g.Expect(cert.KeyUsage).To(Equal(expectedKeyUsage)) + + // Verify extended key usage + g.Expect(cert.ExtKeyUsage).To(Equal([]x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth})) + + // Verify validity period + if tc.duration > 0 { + g.Expect(time.Now()).To(BeTemporally("<", cert.NotAfter)) + g.Expect(time.Now()).To(BeTemporally(">", cert.NotBefore)) + } + + // Verify private key PEM block + keyBlock, _ := pem.Decode(keyPEM) + g.Expect(keyBlock).NotTo(BeNil()) + g.Expect(keyBlock.Type).To(Equal("RSA PRIVATE KEY")) + }) + } +}