From 3d195a4254d223efa92dbea6c21401c6364f422e Mon Sep 17 00:00:00 2001 From: Haseeb Tariq Date: Wed, 3 Jun 2026 11:26:04 -0700 Subject: [PATCH 1/2] pki: add PKI configuration types, validation, and CR manifest generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the configurable PKI API surface to InstallConfig behind the ConfigurablePKI feature gate. When the gate is active, the installer generates a config.openshift.io/v1alpha1 PKI custom resource manifest that day-2 operators use for certificate rotation parameters. The default PKI profile uses RSA-4096 until all day-2 operators (CKAO, CKMO, etc.) support ECDSA rotation. When pki is not specified in install-config the PKI CR uses mode: Default. When pki is specified the PKI CR uses mode: Custom with DefaultPKIProfile as the base and user signerCertificates overrides layered on top. No certificate generation changes are included — all certs remain RSA-2048. Non-TechPreview clusters are completely unaffected. Assisted-by: Claude Code (Opus 4.6) --- .../install.openshift.io_installconfigs.yaml | 76 +++++ pkg/asset/manifests/operators.go | 5 +- pkg/asset/manifests/pki.go | 102 +++++++ pkg/asset/manifests/pki_test.go | 164 +++++++++++ pkg/explain/printer_test.go | 6 + pkg/types/defaults/installconfig.go | 1 + pkg/types/installconfig.go | 107 +++++++ pkg/types/pki/conversion.go | 18 ++ pkg/types/pki/conversion_test.go | 58 ++++ pkg/types/pki/defaults.go | 27 ++ pkg/types/pki/defaults_test.go | 18 ++ pkg/types/pki/validation.go | 116 ++++++++ pkg/types/pki/validation_test.go | 263 ++++++++++++++++++ pkg/types/validation/featuregate_test.go | 35 +++ pkg/types/validation/featuregates.go | 5 + pkg/types/validation/installconfig.go | 6 + pkg/types/validation/installconfig_test.go | 49 ++++ pkg/types/zz_generated.deepcopy.go | 89 ++++++ 18 files changed, 1144 insertions(+), 1 deletion(-) create mode 100644 pkg/asset/manifests/pki.go create mode 100644 pkg/asset/manifests/pki_test.go create mode 100644 pkg/types/pki/conversion.go create mode 100644 pkg/types/pki/conversion_test.go create mode 100644 pkg/types/pki/defaults.go create mode 100644 pkg/types/pki/defaults_test.go create mode 100644 pkg/types/pki/validation.go create mode 100644 pkg/types/pki/validation_test.go diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 37c2e9c4021..9ffb030ec83 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -5084,6 +5084,82 @@ spec: - rhel-9 - rhel-10 type: string + pki: + description: |- + PKI configures cryptographic parameters for installer-generated + signer certificates. When specified, all signer certificates use the + algorithm and parameters from signerCertificates. + Feature gated by ConfigurablePKI. + properties: + signerCertificates: + description: |- + signerCertificates specifies key parameters for all installer-generated + certificate authority (CA) certificates. + When set, all signer certificates use the specified algorithm and parameters. + minProperties: 1 + properties: + key: + description: key specifies the cryptographic parameters for the + certificate's key pair. + properties: + algorithm: + description: |- + algorithm specifies the key generation algorithm. + Valid values are "RSA" and "ECDSA". + enum: + - RSA + - ECDSA + type: string + ecdsa: + description: |- + ecdsa specifies ECDSA key parameters. + Required when algorithm is ECDSA, and forbidden otherwise. + properties: + curve: + description: |- + curve specifies the NIST elliptic curve for ECDSA keys. + Valid values are "P256", "P384", and "P521". + enum: + - P256 + - P384 + - P521 + type: string + required: + - curve + type: object + rsa: + description: |- + rsa specifies RSA key parameters. + Required when algorithm is RSA, and forbidden otherwise. + properties: + keySize: + description: |- + keySize specifies the size of RSA keys in bits. + Valid values are multiples of 1024 from 2048 to 8192. + format: int32 + maximum: 8192 + minimum: 2048 + multipleOf: 1024 + type: integer + required: + - keySize + type: object + required: + - algorithm + type: object + x-kubernetes-validations: + - message: rsa is required when algorithm is RSA, and forbidden + otherwise + rule: 'has(self.algorithm) && self.algorithm == ''RSA'' ? has(self.rsa) + : !has(self.rsa)' + - message: ecdsa is required when algorithm is ECDSA, and forbidden + otherwise + rule: 'has(self.algorithm) && self.algorithm == ''ECDSA'' ? has(self.ecdsa) + : !has(self.ecdsa)' + type: object + required: + - signerCertificates + type: object platform: description: |- Platform is the configuration for the specific platform upon which to diff --git a/pkg/asset/manifests/operators.go b/pkg/asset/manifests/operators.go index d57be7cab8a..0685d7d5693 100644 --- a/pkg/asset/manifests/operators.go +++ b/pkg/asset/manifests/operators.go @@ -90,6 +90,7 @@ func (m *Manifests) Dependencies() []asset.Asset { &bootkube.InternalReleaseImageTLSSecret{}, &bootkube.InternalReleaseImageRegistryAuthSecret{}, &BMCVerifyCAConfigMap{}, + &PKIConfiguration{}, } } @@ -107,8 +108,9 @@ func (m *Manifests) Generate(_ context.Context, dependencies asset.Parents) erro imageDigestMirrorSet := &ImageDigestMirrorSet{} mcoCfgTemplate := &manifests.MCO{} bmcVerifyCAConfigMap := &BMCVerifyCAConfigMap{} + pkiConfig := &PKIConfiguration{} - dependencies.Get(installConfig, ingress, dns, network, infra, proxy, scheduler, imageContentSourcePolicy, imageDigestMirrorSet, clusterCSIDriverConfig, mcoCfgTemplate, bmcVerifyCAConfigMap) + dependencies.Get(installConfig, ingress, dns, network, infra, proxy, scheduler, imageContentSourcePolicy, imageDigestMirrorSet, clusterCSIDriverConfig, mcoCfgTemplate, bmcVerifyCAConfigMap, pkiConfig) redactedConfig, err := redactedInstallConfig(*installConfig.Config) if err != nil { @@ -147,6 +149,7 @@ func (m *Manifests) Generate(_ context.Context, dependencies asset.Parents) erro m.FileList = append(m.FileList, clusterCSIDriverConfig.Files()...) m.FileList = append(m.FileList, imageDigestMirrorSet.Files()...) m.FileList = append(m.FileList, bmcVerifyCAConfigMap.Files()...) + m.FileList = append(m.FileList, pkiConfig.Files()...) asset.SortFiles(m.FileList) diff --git a/pkg/asset/manifests/pki.go b/pkg/asset/manifests/pki.go new file mode 100644 index 00000000000..b35c8a64262 --- /dev/null +++ b/pkg/asset/manifests/pki.go @@ -0,0 +1,102 @@ +package manifests + +import ( + "context" + "path" + + "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + features "github.com/openshift/api/features" + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/installconfig" + pkidefaults "github.com/openshift/installer/pkg/types/pki" +) + +var pkiCfgFilename = path.Join(manifestDir, "cluster-pki-02-config.yaml") + +// PKIConfiguration generates the PKI custom resource manifest. +type PKIConfiguration struct { + FileList []*asset.File +} + +var _ asset.WritableAsset = (*PKIConfiguration)(nil) + +// Name returns a human friendly name for the asset. +func (*PKIConfiguration) Name() string { + return "PKI Config" +} + +// Dependencies returns all of the dependencies directly needed to generate +// the asset. +func (*PKIConfiguration) Dependencies() []asset.Asset { + return []asset.Asset{ + &installconfig.InstallConfig{}, + } +} + +// Generate generates the PKI custom resource manifest. +// The manifest is only generated when the ConfigurablePKI feature gate is enabled. +func (p *PKIConfiguration) Generate(_ context.Context, dependencies asset.Parents) error { + installConfig := &installconfig.InstallConfig{} + dependencies.Get(installConfig) + + if !installConfig.Config.Enabled(features.FeatureGateConfigurablePKI) { + return nil + } + + certMgmt := configv1alpha1.PKICertificateManagement{ + Mode: configv1alpha1.PKICertificateManagementModeDefault, + } + + if installConfig.Config.PKI != nil { + profile := pkidefaults.DefaultPKIProfile() + profile.SignerCertificates = pkidefaults.CertificateConfigToUpstream(installConfig.Config.PKI.SignerCertificates) + + certMgmt = configv1alpha1.PKICertificateManagement{ + Mode: configv1alpha1.PKICertificateManagementModeCustom, + Custom: configv1alpha1.CustomPKIPolicy{ + PKIProfile: profile, + }, + } + } + + config := &configv1alpha1.PKI{ + TypeMeta: metav1.TypeMeta{ + APIVersion: configv1alpha1.SchemeGroupVersion.String(), + Kind: "PKI", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: configv1alpha1.PKISpec{ + CertificateManagement: certMgmt, + }, + } + + configData, err := yaml.Marshal(config) + if err != nil { + return errors.Wrapf(err, "failed to marshal PKI config") + } + + p.FileList = []*asset.File{ + { + Filename: pkiCfgFilename, + Data: configData, + }, + } + + return nil +} + +// Files returns the files generated by the asset. +func (p *PKIConfiguration) Files() []*asset.File { + return p.FileList +} + +// Load returns false since this asset is not written to disk by the installer. +func (p *PKIConfiguration) Load(f asset.FileFetcher) (bool, error) { + return false, nil +} diff --git a/pkg/asset/manifests/pki_test.go b/pkg/asset/manifests/pki_test.go new file mode 100644 index 00000000000..c807aa496dd --- /dev/null +++ b/pkg/asset/manifests/pki_test.go @@ -0,0 +1,164 @@ +package manifests + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" + + configv1 "github.com/openshift/api/config/v1" + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/installconfig" + "github.com/openshift/installer/pkg/types" +) + +func TestPKIConfigurationGenerate(t *testing.T) { + cases := []struct { + name string + installConfig *types.InstallConfig + expectEmpty bool + expectMode configv1alpha1.PKICertificateManagementMode + expectSignerAlgo configv1alpha1.KeyAlgorithm + expectSignerRSA int32 + expectSignerCurve configv1alpha1.ECDSACurve + expectDefaultsAlgo configv1alpha1.KeyAlgorithm + expectDefaultsRSA int32 + expectDefaultsCurve configv1alpha1.ECDSACurve + }{ + { + name: "feature gate disabled - no manifest generated", + installConfig: &types.InstallConfig{ + FeatureSet: configv1.Default, + }, + expectEmpty: true, + }, + { + name: "feature gate enabled, pki nil - mode Default", + installConfig: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + }, + expectEmpty: false, + expectMode: configv1alpha1.PKICertificateManagementModeDefault, + }, + { + name: "feature gate enabled, pki RSA-4096", + installConfig: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + PKI: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + }, + }, + }, + expectEmpty: false, + expectMode: configv1alpha1.PKICertificateManagementModeCustom, + expectSignerAlgo: configv1alpha1.KeyAlgorithmRSA, + expectSignerRSA: 4096, + expectDefaultsAlgo: configv1alpha1.KeyAlgorithmRSA, + expectDefaultsRSA: 4096, + }, + { + name: "feature gate enabled, pki ECDSA P-384", + installConfig: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + PKI: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + }, + }, + expectEmpty: false, + expectMode: configv1alpha1.PKICertificateManagementModeCustom, + expectSignerAlgo: configv1alpha1.KeyAlgorithmECDSA, + expectSignerCurve: configv1alpha1.ECDSACurveP384, + expectDefaultsAlgo: configv1alpha1.KeyAlgorithmRSA, + expectDefaultsRSA: 4096, + }, + { + name: "feature gate enabled, pki RSA-2048 explicit", + installConfig: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + PKI: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 2048}, + }, + }, + }, + }, + expectEmpty: false, + expectMode: configv1alpha1.PKICertificateManagementModeCustom, + expectSignerAlgo: configv1alpha1.KeyAlgorithmRSA, + expectSignerRSA: 2048, + expectDefaultsAlgo: configv1alpha1.KeyAlgorithmRSA, + expectDefaultsRSA: 4096, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + parents := asset.Parents{} + parents.Add(installconfig.MakeAsset(tc.installConfig)) + + pkiAsset := &PKIConfiguration{} + err := pkiAsset.Generate(context.Background(), parents) + if !assert.NoError(t, err) { + return + } + + if tc.expectEmpty { + assert.Empty(t, pkiAsset.Files()) + return + } + + if !assert.Len(t, pkiAsset.Files(), 1) { + return + } + assert.Equal(t, "manifests/cluster-pki-02-config.yaml", pkiAsset.Files()[0].Filename) + + // Unmarshal and verify the CR structure + var pkiCR configv1alpha1.PKI + err = yaml.Unmarshal(pkiAsset.Files()[0].Data, &pkiCR) + if !assert.NoError(t, err) { + return + } + + assert.Equal(t, "config.openshift.io/v1alpha1", pkiCR.APIVersion) + assert.Equal(t, "PKI", pkiCR.Kind) + assert.Equal(t, "cluster", pkiCR.Name) + assert.Equal(t, tc.expectMode, pkiCR.Spec.CertificateManagement.Mode) + + if tc.expectMode != configv1alpha1.PKICertificateManagementModeCustom { + return + } + + profile := pkiCR.Spec.CertificateManagement.Custom.PKIProfile + + // Verify defaults + assert.Equal(t, tc.expectDefaultsAlgo, profile.Defaults.Key.Algorithm) + if tc.expectDefaultsAlgo == configv1alpha1.KeyAlgorithmECDSA { + assert.Equal(t, tc.expectDefaultsCurve, profile.Defaults.Key.ECDSA.Curve) + } else { + assert.Equal(t, tc.expectDefaultsRSA, profile.Defaults.Key.RSA.KeySize) + } + + // Verify signerCertificates + assert.Equal(t, tc.expectSignerAlgo, profile.SignerCertificates.Key.Algorithm) + if tc.expectSignerAlgo == configv1alpha1.KeyAlgorithmRSA { + assert.Equal(t, tc.expectSignerRSA, profile.SignerCertificates.Key.RSA.KeySize) + } + if tc.expectSignerAlgo == configv1alpha1.KeyAlgorithmECDSA { + assert.Equal(t, tc.expectSignerCurve, profile.SignerCertificates.Key.ECDSA.Curve) + } + }) + } +} diff --git a/pkg/explain/printer_test.go b/pkg/explain/printer_test.go index 6333877a5ed..cc07374bf5c 100644 --- a/pkg/explain/printer_test.go +++ b/pkg/explain/printer_test.go @@ -139,6 +139,12 @@ the cluster. Valid Values: "rhel-9","rhel-10" OSImageStream is the global OS Image Stream to be used for all machines in the cluster. + pki + PKI configures cryptographic parameters for installer-generated +signer certificates. When specified, all signer certificates use the +algorithm and parameters from signerCertificates. +Feature gated by ConfigurablePKI. + platform -required- Platform is the configuration for the specific platform upon which to perform the installation. diff --git a/pkg/types/defaults/installconfig.go b/pkg/types/defaults/installconfig.go index 6e88ba5aba7..afef381e64a 100644 --- a/pkg/types/defaults/installconfig.go +++ b/pkg/types/defaults/installconfig.go @@ -142,4 +142,5 @@ func SetInstallConfigDefaults(c *types.InstallConfig) { if c.AdditionalTrustBundlePolicy == "" { c.AdditionalTrustBundlePolicy = types.PolicyProxyOnly } + } diff --git a/pkg/types/installconfig.go b/pkg/types/installconfig.go index fcfda23cc9e..9666d0ca172 100644 --- a/pkg/types/installconfig.go +++ b/pkg/types/installconfig.go @@ -231,8 +231,115 @@ type InstallConfig struct { // OSImageStream is the global OS Image Stream to be used for all machines in the cluster. // +optional OSImageStream OSImageStream `json:"osImageStream,omitempty"` + + // PKI configures cryptographic parameters for installer-generated + // signer certificates. When specified, all signer certificates use the + // algorithm and parameters from signerCertificates. + // Feature gated by ConfigurablePKI. + // +openshift:enable:FeatureGate=ConfigurablePKI + // +optional + PKI *PKIConfig `json:"pki,omitempty"` +} + +// PKIConfig configures cryptographic parameters for installer-generated +// signer certificates. When pki is present in the install config, +// signerCertificates must be fully specified with algorithm and key parameters. +type PKIConfig struct { + // signerCertificates specifies key parameters for all installer-generated + // certificate authority (CA) certificates. + // When set, all signer certificates use the specified algorithm and parameters. + // +required + SignerCertificates CertificateConfig `json:"signerCertificates"` } +// The types below (CertificateConfig, KeyConfig, RSAKeyConfig, ECDSAKeyConfig, +// KeyAlgorithm, ECDSACurve) mirror configv1alpha1 types from openshift/api. +// We maintain local copies so that new fields added upstream do not silently +// appear in the install-config YAML API without explicit opt-in and validation +// by the installer. Conversion to the upstream type happens at the manifest +// boundary (see pkg/types/pki/conversion.go). + +// CertificateConfig specifies configuration parameters for certificates. +// +kubebuilder:validation:MinProperties=1 +type CertificateConfig struct { + // key specifies the cryptographic parameters for the certificate's key pair. + // +optional + Key KeyConfig `json:"key,omitzero"` +} + +// KeyConfig specifies cryptographic parameters for key generation. +// +// +kubebuilder:validation:XValidation:rule="has(self.algorithm) && self.algorithm == 'RSA' ? has(self.rsa) : !has(self.rsa)",message="rsa is required when algorithm is RSA, and forbidden otherwise" +// +kubebuilder:validation:XValidation:rule="has(self.algorithm) && self.algorithm == 'ECDSA' ? has(self.ecdsa) : !has(self.ecdsa)",message="ecdsa is required when algorithm is ECDSA, and forbidden otherwise" +// +union +// +//nolint:godot +type KeyConfig struct { + // algorithm specifies the key generation algorithm. + // Valid values are "RSA" and "ECDSA". + // +required + // +unionDiscriminator + Algorithm KeyAlgorithm `json:"algorithm,omitempty"` + + // rsa specifies RSA key parameters. + // Required when algorithm is RSA, and forbidden otherwise. + // +optional + // +unionMember + RSA RSAKeyConfig `json:"rsa,omitzero"` + + // ecdsa specifies ECDSA key parameters. + // Required when algorithm is ECDSA, and forbidden otherwise. + // +optional + // +unionMember + ECDSA ECDSAKeyConfig `json:"ecdsa,omitzero"` +} + +// RSAKeyConfig specifies parameters for RSA key generation. +type RSAKeyConfig struct { + // keySize specifies the size of RSA keys in bits. + // Valid values are multiples of 1024 from 2048 to 8192. + // +required + // +kubebuilder:validation:Minimum=2048 + // +kubebuilder:validation:Maximum=8192 + // +kubebuilder:validation:MultipleOf=1024 + KeySize int32 `json:"keySize,omitempty"` +} + +// ECDSAKeyConfig specifies parameters for ECDSA key generation. +type ECDSAKeyConfig struct { + // curve specifies the NIST elliptic curve for ECDSA keys. + // Valid values are "P256", "P384", and "P521". + // +required + Curve ECDSACurve `json:"curve,omitempty"` +} + +// KeyAlgorithm specifies the cryptographic algorithm used for key generation. +// +kubebuilder:validation:Enum=RSA;ECDSA +type KeyAlgorithm string + +const ( + // KeyAlgorithmRSA specifies the RSA algorithm for key generation. + KeyAlgorithmRSA KeyAlgorithm = "RSA" + + // KeyAlgorithmECDSA specifies the ECDSA algorithm for key generation. + KeyAlgorithmECDSA KeyAlgorithm = "ECDSA" +) + +// ECDSACurve specifies the elliptic curve used for ECDSA key generation. +// +kubebuilder:validation:Enum=P256;P384;P521 +type ECDSACurve string + +const ( + // ECDSACurveP256 specifies the NIST P-256 curve. + ECDSACurveP256 ECDSACurve = "P256" + + // ECDSACurveP384 specifies the NIST P-384 curve. + ECDSACurveP384 ECDSACurve = "P384" + + // ECDSACurveP521 specifies the NIST P-521 curve. + ECDSACurveP521 ECDSACurve = "P521" +) + // ClusterDomain returns the DNS domain that all records for a cluster must belong to. func (c *InstallConfig) ClusterDomain() string { return fmt.Sprintf("%s.%s", c.ObjectMeta.Name, strings.TrimSuffix(c.BaseDomain, ".")) diff --git a/pkg/types/pki/conversion.go b/pkg/types/pki/conversion.go new file mode 100644 index 00000000000..8a262a433c6 --- /dev/null +++ b/pkg/types/pki/conversion.go @@ -0,0 +1,18 @@ +package pki + +import ( + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/installer/pkg/types" +) + +// CertificateConfigToUpstream converts the installer-local CertificateConfig +// to the upstream configv1alpha1.CertificateConfig for use in the PKI CR manifest. +func CertificateConfigToUpstream(local types.CertificateConfig) configv1alpha1.CertificateConfig { + return configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithm(local.Key.Algorithm), + RSA: configv1alpha1.RSAKeyConfig{KeySize: local.Key.RSA.KeySize}, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurve(local.Key.ECDSA.Curve)}, + }, + } +} diff --git a/pkg/types/pki/conversion_test.go b/pkg/types/pki/conversion_test.go new file mode 100644 index 00000000000..51b489dfa3d --- /dev/null +++ b/pkg/types/pki/conversion_test.go @@ -0,0 +1,58 @@ +package pki + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" + + "github.com/openshift/installer/pkg/types" +) + +func TestCertificateConfigToUpstreamMarshal(t *testing.T) { + cases := []struct { + name string + input types.CertificateConfig + wantKey string + absentKey string + }{ + { + name: "RSA config omits ECDSA in marshaled output", + input: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + }, + wantKey: "rsa:", + absentKey: "ecdsa:", + }, + { + name: "ECDSA config omits RSA in marshaled output", + input: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + wantKey: "ecdsa:", + absentKey: "rsa:", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + upstream := CertificateConfigToUpstream(tc.input) + data, err := yaml.Marshal(upstream) + if !assert.NoError(t, err) { + return + } + + output := string(data) + assert.Contains(t, output, tc.wantKey) + assert.False(t, strings.Contains(output, tc.absentKey), + "marshaled output should not contain %q:\n%s", tc.absentKey, output) + }) + } +} diff --git a/pkg/types/pki/defaults.go b/pkg/types/pki/defaults.go new file mode 100644 index 00000000000..1cec7ff58be --- /dev/null +++ b/pkg/types/pki/defaults.go @@ -0,0 +1,27 @@ +package pki + +import ( + configv1alpha1 "github.com/openshift/api/config/v1alpha1" +) + +// DefaultPKIProfile returns the default PKI profile for OpenShift clusters. +// Currently uses RSA-4096 until all day-2 operators (CKAO, CKMO, etc.) support +// ECDSA certificate rotation. Once operator support lands, switch to ECDSA P-384 +// signers and ECDSA P-256 defaults to match the upstream library-go profile: +// https://github.com/openshift/library-go/blob/12d8376369b7c5b76f688d01089882ca28e351c3/pkg/pki/profile.go#L11-L26 +func DefaultPKIProfile() configv1alpha1.PKIProfile { + return configv1alpha1.PKIProfile{ + Defaults: configv1alpha1.DefaultCertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 4096}, + }, + }, + SignerCertificates: configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 4096}, + }, + }, + } +} \ No newline at end of file diff --git a/pkg/types/pki/defaults_test.go b/pkg/types/pki/defaults_test.go new file mode 100644 index 00000000000..a5fb0810ff7 --- /dev/null +++ b/pkg/types/pki/defaults_test.go @@ -0,0 +1,18 @@ +package pki + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" +) + +func TestDefaultPKIProfile(t *testing.T) { + profile := DefaultPKIProfile() + + assert.Equal(t, configv1alpha1.KeyAlgorithmRSA, profile.Defaults.Key.Algorithm) + assert.Equal(t, int32(4096), profile.Defaults.Key.RSA.KeySize) + assert.Equal(t, configv1alpha1.KeyAlgorithmRSA, profile.SignerCertificates.Key.Algorithm) + assert.Equal(t, int32(4096), profile.SignerCertificates.Key.RSA.KeySize) +} diff --git a/pkg/types/pki/validation.go b/pkg/types/pki/validation.go new file mode 100644 index 00000000000..c27acd5365e --- /dev/null +++ b/pkg/types/pki/validation.go @@ -0,0 +1,116 @@ +package pki + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types" +) + +// ValidatePKIConfig validates the PKI configuration. +// When pkiConfig is non-nil, signerCertificates must be fully specified. +// NOTE: All fields are value types (not pointers). Use zero-value checks. +func ValidatePKIConfig(pkiConfig *types.PKIConfig, fldPath *field.Path, fips bool) field.ErrorList { + allErrs := field.ErrorList{} + + if pkiConfig == nil { + return allErrs + } + + // signerCertificates.key must be fully specified when pki is present + if pkiConfig.SignerCertificates.Key.Algorithm == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("signerCertificates", "key"), + "signerCertificates.key is required when pki is specified")) + return allErrs + } + + allErrs = append(allErrs, ValidateKeyConfig(pkiConfig.SignerCertificates.Key, + fldPath.Child("signerCertificates", "key"), fips)...) + + return allErrs +} + +// ValidateKeyConfig validates the KeyConfig structure. +// KeyConfig fields are value types: RSA is RSAKeyConfig, ECDSA is ECDSAKeyConfig. +// Use zero-value checks (KeySize == 0, Curve == "") instead of nil checks. +func ValidateKeyConfig(config types.KeyConfig, fldPath *field.Path, fips bool) field.ErrorList { + allErrs := field.ErrorList{} + + if config.Algorithm == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("algorithm"), + "algorithm must be specified")) + return allErrs + } + + if config.Algorithm != types.KeyAlgorithmRSA && config.Algorithm != types.KeyAlgorithmECDSA { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("algorithm"), + config.Algorithm, []string{string(types.KeyAlgorithmRSA), string(types.KeyAlgorithmECDSA)})) + return allErrs + } + + if config.Algorithm == types.KeyAlgorithmRSA { + if config.RSA.KeySize == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("rsa", "keySize"), + "keySize must be specified when algorithm is RSA")) + } else { + allErrs = append(allErrs, validateRSAKeyConfig(config.RSA, fldPath.Child("rsa"), fips)...) + } + + if config.ECDSA.Curve != "" { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("ecdsa"), + "ecdsa must not be set when algorithm is RSA")) + } + } + + if config.Algorithm == types.KeyAlgorithmECDSA { + if config.ECDSA.Curve == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("ecdsa", "curve"), + "curve must be specified when algorithm is ECDSA")) + } else { + allErrs = append(allErrs, validateECDSAKeyConfig(config.ECDSA, fldPath.Child("ecdsa"), fips)...) + } + + if config.RSA.KeySize != 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("rsa"), + "rsa must not be set when algorithm is ECDSA")) + } + } + + return allErrs +} + +func validateRSAKeyConfig(config types.RSAKeyConfig, fldPath *field.Path, fips bool) field.ErrorList { + allErrs := field.ErrorList{} + + // Validate key size — aligned with API kubebuilder validation: + // multiples of 1024 from 2048 to 8192 + if config.KeySize < 2048 || config.KeySize > 8192 || config.KeySize%1024 != 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("keySize"), config.KeySize, + "must be a multiple of 1024 from 2048 to 8192")) + } + + return allErrs +} + +func validateECDSAKeyConfig(config types.ECDSAKeyConfig, fldPath *field.Path, fips bool) field.ErrorList { + allErrs := field.ErrorList{} + + validCurves := []types.ECDSACurve{ + types.ECDSACurveP256, + types.ECDSACurveP384, + types.ECDSACurveP521, + } + valid := false + for _, curve := range validCurves { + if config.Curve == curve { + valid = true + break + } + } + + if !valid { + allErrs = append(allErrs, field.Invalid(fldPath.Child("curve"), config.Curve, + "must be P256, P384, or P521")) + } + + return allErrs +} diff --git a/pkg/types/pki/validation_test.go b/pkg/types/pki/validation_test.go new file mode 100644 index 00000000000..f687259a570 --- /dev/null +++ b/pkg/types/pki/validation_test.go @@ -0,0 +1,263 @@ +package pki + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types" +) + +func TestValidatePKIConfig(t *testing.T) { + fldPath := field.NewPath("pki") + + cases := []struct { + name string + pkiConfig *types.PKIConfig + fips bool + expectError bool + errorCount int + }{ + { + name: "nil config is valid", + pkiConfig: nil, + expectError: false, + }, + { + name: "valid RSA signer config", + pkiConfig: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + }, + }, + expectError: false, + }, + { + name: "valid ECDSA signer config", + pkiConfig: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + }, + expectError: false, + }, + { + name: "empty PKI config - signerCertificates required", + pkiConfig: &types.PKIConfig{}, + expectError: true, + errorCount: 1, + }, + { + name: "invalid RSA key size", + pkiConfig: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 1024}, + }, + }, + }, + expectError: true, + errorCount: 1, + }, + { + name: "invalid ECDSA curve", + pkiConfig: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: "P224"}, + }, + }, + }, + expectError: true, + errorCount: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := ValidatePKIConfig(tc.pkiConfig, fldPath, tc.fips) + if tc.expectError { + assert.Len(t, errs, tc.errorCount) + } else { + assert.Empty(t, errs) + } + }) + } +} + +func TestValidateKeyConfig(t *testing.T) { + fldPath := field.NewPath("pki", "signerCertificates", "key") + + cases := []struct { + name string + config types.KeyConfig + fips bool + expectError bool + errorCount int + }{ + // Valid RSA configs + { + name: "valid RSA 2048", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 2048}, + }, + }, + { + name: "valid RSA 3072", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 3072}, + }, + }, + { + name: "valid RSA 4096", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + }, + { + name: "valid RSA 8192", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 8192}, + }, + }, + // Valid ECDSA configs + { + name: "valid ECDSA P256", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP256}, + }, + }, + { + name: "valid ECDSA P384", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + { + name: "valid ECDSA P521", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP521}, + }, + }, + // Invalid: missing algorithm + { + name: "missing algorithm", + config: types.KeyConfig{ + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + expectError: true, + errorCount: 1, + }, + // Invalid: unsupported algorithm + { + name: "unsupported algorithm", + config: types.KeyConfig{ + Algorithm: "Ed25519", + }, + expectError: true, + errorCount: 1, + }, + // Invalid RSA key sizes + { + name: "RSA key size too small", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 1024}, + }, + expectError: true, + errorCount: 1, + }, + { + name: "RSA key size too large", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 9216}, + }, + expectError: true, + errorCount: 1, + }, + { + name: "RSA key size not multiple of 1024", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 5000}, + }, + expectError: true, + errorCount: 1, + }, + { + name: "RSA missing key size", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + }, + expectError: true, + errorCount: 1, + }, + // Invalid ECDSA curves + { + name: "ECDSA invalid curve", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: "P224"}, + }, + expectError: true, + errorCount: 1, + }, + { + name: "ECDSA missing curve", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + }, + expectError: true, + errorCount: 1, + }, + // Mismatched algorithm/params + { + name: "RSA with ECDSA config", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP256}, + }, + expectError: true, + errorCount: 1, + }, + { + name: "ECDSA with RSA config", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + expectError: true, + errorCount: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := ValidateKeyConfig(tc.config, fldPath, tc.fips) + if tc.expectError { + assert.Len(t, errs, tc.errorCount, "expected %d errors, got: %v", tc.errorCount, errs) + } else { + assert.Empty(t, errs, "expected no errors, got: %v", errs) + } + }) + } +} diff --git a/pkg/types/validation/featuregate_test.go b/pkg/types/validation/featuregate_test.go index efffd9a3e4d..46d574dbc02 100644 --- a/pkg/types/validation/featuregate_test.go +++ b/pkg/types/validation/featuregate_test.go @@ -225,6 +225,41 @@ func TestFeatureGates(t *testing.T) { return c }(), }, + { + name: "PKI config present without feature gate - error", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.PKI = &types.PKIConfig{} + return c + }(), + expected: `^pki: Forbidden: this field is protected by the ConfigurablePKI feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`, + }, + { + name: "PKI config present with TechPreviewNoUpgrade - passes", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.FeatureSet = v1.TechPreviewNoUpgrade + c.PKI = &types.PKIConfig{} + return c + }(), + }, + { + name: "PKI config present with CustomNoUpgrade and ConfigurablePKI - passes", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.FeatureSet = v1.CustomNoUpgrade + c.FeatureGates = []string{"ConfigurablePKI=true"} + c.PKI = &types.PKIConfig{} + return c + }(), + }, + { + name: "PKI nil without feature gate - no error", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + return c + }(), + }, { name: "OKD featureset requires SCOS-compiled installer", installConfig: func() *types.InstallConfig { diff --git a/pkg/types/validation/featuregates.go b/pkg/types/validation/featuregates.go index a46b5bbff50..9ff53a1ff64 100644 --- a/pkg/types/validation/featuregates.go +++ b/pkg/types/validation/featuregates.go @@ -51,5 +51,10 @@ func validateMachinePoolFeatureGates(c *types.InstallConfig) []featuregates.Gate Condition: len(c.Compute) > 0 && c.Compute[0].Management == types.ClusterAPI, Field: field.NewPath("compute", "management"), }, + { + FeatureGateName: features.FeatureGateConfigurablePKI, + Condition: c.PKI != nil, + Field: field.NewPath("pki"), + }, } } diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index d6c088ee596..eb2dc6195f2 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -22,6 +22,7 @@ import ( utilsnet "k8s.io/utils/net" configv1 "github.com/openshift/api/config/v1" + features "github.com/openshift/api/features" operv1 "github.com/openshift/api/operator/v1" "github.com/openshift/installer/pkg/hostcrypt" "github.com/openshift/installer/pkg/ipnet" @@ -48,6 +49,7 @@ import ( openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" "github.com/openshift/installer/pkg/types/ovirt" ovirtvalidation "github.com/openshift/installer/pkg/types/ovirt/validation" + pkivalidation "github.com/openshift/installer/pkg/types/pki" "github.com/openshift/installer/pkg/types/powervc" powervcvalidation "github.com/openshift/installer/pkg/types/powervc/validation" "github.com/openshift/installer/pkg/types/powervs" @@ -284,6 +286,10 @@ func ValidateInstallConfig(c *types.InstallConfig, usingAgentMethod bool) field. } } + if c.PKI != nil && c.Enabled(features.FeatureGateConfigurablePKI) { + allErrs = append(allErrs, pkivalidation.ValidatePKIConfig(c.PKI, field.NewPath("pki"), c.FIPS)...) + } + allErrs = append(allErrs, ValidateFeatureSet(c)...) allErrs = append(allErrs, validateOSImageStream(c)...) diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index ef7e2372c13..a085288d201 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -3101,6 +3101,55 @@ func TestValidateInstallConfig(t *testing.T) { }(), expectedError: `^bootstrapInPlace: Invalid value: "": Single Node OpenShift is not supported on the baremetal platform$`, }, + { + name: "valid PKI with signer certificates", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.FeatureSet = configv1.TechPreviewNoUpgrade + c.PKI = &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + } + return c + }(), + }, + { + name: "invalid PKI signer with unsupported algorithm", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.FeatureSet = configv1.TechPreviewNoUpgrade + c.PKI = &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: "EdDSA", + }, + }, + } + return c + }(), + expectedError: `pki\.signerCertificates\.key\.algorithm: Unsupported value: "EdDSA"`, + }, + { + name: "invalid PKI signer with bad RSA key size", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.FeatureSet = configv1.TechPreviewNoUpgrade + c.PKI = &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 1024}, + }, + }, + } + return c + }(), + expectedError: `pki\.signerCertificates\.key\.rsa\.keySize: Invalid value: 1024`, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/types/zz_generated.deepcopy.go b/pkg/types/zz_generated.deepcopy.go index b32e9eded99..199ac92b9e6 100644 --- a/pkg/types/zz_generated.deepcopy.go +++ b/pkg/types/zz_generated.deepcopy.go @@ -60,6 +60,23 @@ func (in *Capabilities) DeepCopy() *Capabilities { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CertificateConfig) DeepCopyInto(out *CertificateConfig) { + *out = *in + out.Key = in.Key + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CertificateConfig. +func (in *CertificateConfig) DeepCopy() *CertificateConfig { + if in == nil { + return nil + } + out := new(CertificateConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterMetadata) DeepCopyInto(out *ClusterMetadata) { *out = *in @@ -292,6 +309,22 @@ func (in *DiskUserDefined) DeepCopy() *DiskUserDefined { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ECDSAKeyConfig) DeepCopyInto(out *ECDSAKeyConfig) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ECDSAKeyConfig. +func (in *ECDSAKeyConfig) DeepCopy() *ECDSAKeyConfig { + if in == nil { + return nil + } + out := new(ECDSAKeyConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Fencing) DeepCopyInto(out *Fencing) { *out = *in @@ -448,6 +481,11 @@ func (in *InstallConfig) DeepCopyInto(out *InstallConfig) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.PKI != nil { + in, out := &in.PKI, &out.PKI + *out = new(PKIConfig) + **out = **in + } return } @@ -461,6 +499,24 @@ func (in *InstallConfig) DeepCopy() *InstallConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *KeyConfig) DeepCopyInto(out *KeyConfig) { + *out = *in + out.RSA = in.RSA + out.ECDSA = in.ECDSA + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KeyConfig. +func (in *KeyConfig) DeepCopy() *KeyConfig { + if in == nil { + return nil + } + out := new(KeyConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineNetworkEntry) DeepCopyInto(out *MachineNetworkEntry) { *out = *in @@ -720,6 +776,23 @@ func (in *OperatorPublishingStrategy) DeepCopy() *OperatorPublishingStrategy { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PKIConfig) DeepCopyInto(out *PKIConfig) { + *out = *in + out.SignerCertificates = in.SignerCertificates + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PKIConfig. +func (in *PKIConfig) DeepCopy() *PKIConfig { + if in == nil { + return nil + } + out := new(PKIConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Platform) DeepCopyInto(out *Platform) { *out = *in @@ -816,3 +889,19 @@ func (in *Proxy) DeepCopy() *Proxy { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RSAKeyConfig) DeepCopyInto(out *RSAKeyConfig) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RSAKeyConfig. +func (in *RSAKeyConfig) DeepCopy() *RSAKeyConfig { + if in == nil { + return nil + } + out := new(RSAKeyConfig) + in.DeepCopyInto(out) + return out +} From 96bfc2d7c49bb52c4c7f5f3936883aa8734eac67 Mon Sep 17 00:00:00 2001 From: Haseeb Tariq Date: Wed, 3 Jun 2026 12:16:38 -0700 Subject: [PATCH 2/2] tls: refactor TLS cert generation to support configurable key algorithms Refactor pkg/asset/tls/ to support generating signer certificates with configurable key algorithms (RSA or ECDSA). PrivateKeyToPem now returns ([]byte, error) instead of calling logrus.Fatalf, and GenerateSelfSignedCertificate accepts PrivateKeyParams to control key generation. KeyUsage flags are set based on the algorithm since ECDSA keys cannot perform key encipherment. All signer certs pass nil for pkiConfig in this commit, preserving the existing RSA-2048 behavior. Wiring signers to read PKI config is deferred to a follow-up to avoid breaking codepaths that generate signer certs without an install-config on disk (e.g. agent create certificates, node-joiner add-nodes). Assisted-by: Claude Code (Opus 4.6) --- .../configimage/ingressoperatorsigner.go | 5 +- pkg/asset/tls/adminkubeconfig.go | 18 +- pkg/asset/tls/aggregator.go | 20 +- pkg/asset/tls/apiserver.go | 64 ++++-- pkg/asset/tls/boundsasigningkey.go | 11 +- pkg/asset/tls/certkey.go | 51 ++--- pkg/asset/tls/certkey_test.go | 131 +++++++++++- pkg/asset/tls/ironictls.go | 2 +- pkg/asset/tls/keypair.go | 5 +- pkg/asset/tls/kubecontrolplane.go | 10 +- pkg/asset/tls/kubelet.go | 28 ++- pkg/asset/tls/root.go | 19 +- pkg/asset/tls/tls.go | 131 ++++++++++-- pkg/asset/tls/tls_test.go | 201 ++++++++++++++++++ pkg/asset/tls/utils.go | 60 ++++-- pkg/asset/tls/utils_test.go | 92 ++++++++ 16 files changed, 729 insertions(+), 119 deletions(-) create mode 100644 pkg/asset/tls/utils_test.go diff --git a/pkg/asset/imagebased/configimage/ingressoperatorsigner.go b/pkg/asset/imagebased/configimage/ingressoperatorsigner.go index 3cca119b3a4..48184aa37fa 100644 --- a/pkg/asset/imagebased/configimage/ingressoperatorsigner.go +++ b/pkg/asset/imagebased/configimage/ingressoperatorsigner.go @@ -59,7 +59,10 @@ func (a *IngressOperatorSignerCertKey) Generate(ctx context.Context, dependencie return err } - a.KeyRaw = tls.PrivateKeyToPem(key) + a.KeyRaw, err = tls.PrivateKeyToPem(key) + if err != nil { + return fmt.Errorf("failed to encode private key to PEM: %w", err) + } a.CertRaw = tls.CertToPem(crt) return nil diff --git a/pkg/asset/tls/adminkubeconfig.go b/pkg/asset/tls/adminkubeconfig.go index f4ec8a7bf65..8b2e33f9d11 100644 --- a/pkg/asset/tls/adminkubeconfig.go +++ b/pkg/asset/tls/adminkubeconfig.go @@ -15,7 +15,13 @@ type AdminKubeConfigSignerCertKey struct { var _ asset.WritableAsset = (*AdminKubeConfigSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// Dependencies returns no dependencies. Configurable PKI requires +// reading the PKI config from InstallConfig, but adding InstallConfig +// as a dependency here would break codepaths that generate signer certs +// without an install-config on disk (e.g. agent create certificates, +// node-joiner). A follow-up commit introduces an intermediate asset +// that reads the config directly from disk without triggering +// InstallConfig validation. func (c *AdminKubeConfigSignerCertKey) Dependencies() []asset.Asset { return []asset.Asset{} } @@ -23,13 +29,13 @@ func (c *AdminKubeConfigSignerCertKey) Dependencies() []asset.Asset { // Generate generates the root-ca key and cert pair. func (c *AdminKubeConfigSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "admin-kubeconfig-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityTenYears(), - IsCA: true, + Subject: pkix.Name{CommonName: "admin-kubeconfig-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityTenYears(), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "admin-kubeconfig-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "admin-kubeconfig-signer", nil) } // Load reads the asset files from disk. diff --git a/pkg/asset/tls/aggregator.go b/pkg/asset/tls/aggregator.go index 6088b6785b1..082f23d7266 100644 --- a/pkg/asset/tls/aggregator.go +++ b/pkg/asset/tls/aggregator.go @@ -32,13 +32,13 @@ func (a *AggregatorCA) Generate(ctx context.Context, dependencies asset.Parents) dependencies.Get(installConfig) cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "aggregator", OrganizationalUnit: []string{"bootkube"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityOneDay(installConfig), - IsCA: true, + Subject: pkix.Name{CommonName: "aggregator", OrganizationalUnit: []string{"bootkube"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityOneDay(installConfig), + IsCA: true, } - return a.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-ca") + return a.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-ca", nil) } // Name returns the human-friendly name of the asset. @@ -102,13 +102,13 @@ func (c *AggregatorSignerCertKey) Generate(ctx context.Context, parents asset.Pa installConfig := &installconfig.InstallConfig{} parents.Get(installConfig) cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "aggregator-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityOneDay(installConfig), - IsCA: true, + Subject: pkix.Name{CommonName: "aggregator-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityOneDay(installConfig), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-signer", nil) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/apiserver.go b/pkg/asset/tls/apiserver.go index 8eb94e9b871..34659e7c663 100644 --- a/pkg/asset/tls/apiserver.go +++ b/pkg/asset/tls/apiserver.go @@ -29,13 +29,13 @@ func (c *KubeAPIServerToKubeletSignerCertKey) Generate(ctx context.Context, pare installConfig := &installconfig.InstallConfig{} parents.Get(installConfig) cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kube-apiserver-to-kubelet-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityOneYear(installConfig), - IsCA: true, + Subject: pkix.Name{CommonName: "kube-apiserver-to-kubelet-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityOneYear(installConfig), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-to-kubelet-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-to-kubelet-signer", nil) } // Name returns the human-friendly name of the asset. @@ -116,7 +116,13 @@ type KubeAPIServerLocalhostSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerLocalhostSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// Dependencies returns no dependencies. Configurable PKI requires +// reading the PKI config from InstallConfig, but adding InstallConfig +// as a dependency here would break codepaths that generate signer certs +// without an install-config on disk (e.g. agent create certificates, +// node-joiner). A follow-up commit introduces an intermediate asset +// that reads the config directly from disk without triggering +// InstallConfig validation. func (c *KubeAPIServerLocalhostSignerCertKey) Dependencies() []asset.Asset { return []asset.Asset{} } @@ -124,13 +130,13 @@ func (c *KubeAPIServerLocalhostSignerCertKey) Dependencies() []asset.Asset { // Generate generates the root-ca key and cert pair. func (c *KubeAPIServerLocalhostSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kube-apiserver-localhost-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityTenYears(), - IsCA: true, + Subject: pkix.Name{CommonName: "kube-apiserver-localhost-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityTenYears(), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-localhost-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-localhost-signer", nil) } // Load reads the asset files from disk. @@ -220,7 +226,13 @@ type KubeAPIServerServiceNetworkSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerServiceNetworkSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// Dependencies returns no dependencies. Configurable PKI requires +// reading the PKI config from InstallConfig, but adding InstallConfig +// as a dependency here would break codepaths that generate signer certs +// without an install-config on disk (e.g. agent create certificates, +// node-joiner). A follow-up commit introduces an intermediate asset +// that reads the config directly from disk without triggering +// InstallConfig validation. func (c *KubeAPIServerServiceNetworkSignerCertKey) Dependencies() []asset.Asset { return []asset.Asset{} } @@ -228,13 +240,13 @@ func (c *KubeAPIServerServiceNetworkSignerCertKey) Dependencies() []asset.Asset // Generate generates the root-ca key and cert pair. func (c *KubeAPIServerServiceNetworkSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kube-apiserver-service-network-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityTenYears(), - IsCA: true, + Subject: pkix.Name{CommonName: "kube-apiserver-service-network-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityTenYears(), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-service-network-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-service-network-signer", nil) } // Load reads the asset files from disk. @@ -333,7 +345,13 @@ type KubeAPIServerLBSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerLBSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// Dependencies returns no dependencies. Configurable PKI requires +// reading the PKI config from InstallConfig, but adding InstallConfig +// as a dependency here would break codepaths that generate signer certs +// without an install-config on disk (e.g. agent create certificates, +// node-joiner). A follow-up commit introduces an intermediate asset +// that reads the config directly from disk without triggering +// InstallConfig validation. func (c *KubeAPIServerLBSignerCertKey) Dependencies() []asset.Asset { return []asset.Asset{} } @@ -341,13 +359,13 @@ func (c *KubeAPIServerLBSignerCertKey) Dependencies() []asset.Asset { // Generate generates the root-ca key and cert pair. func (c *KubeAPIServerLBSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kube-apiserver-lb-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityTenYears(), - IsCA: true, + Subject: pkix.Name{CommonName: "kube-apiserver-lb-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityTenYears(), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-lb-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-lb-signer", nil) } // Load reads the asset files from disk. diff --git a/pkg/asset/tls/boundsasigningkey.go b/pkg/asset/tls/boundsasigningkey.go index 1f88f2f295b..d70ed00b476 100644 --- a/pkg/asset/tls/boundsasigningkey.go +++ b/pkg/asset/tls/boundsasigningkey.go @@ -2,6 +2,7 @@ package tls import ( "context" + "crypto/rsa" "os" "github.com/pkg/errors" @@ -50,10 +51,14 @@ func (sk *BoundSASigningKey) Load(f asset.FileFetcher) (bool, error) { return false, err } - rsaKey, err := PemToPrivateKey(keyFile.Data) + key, err := PemToPrivateKey(keyFile.Data) if err != nil { - logrus.Debugf("Failed to load rsa.PrivateKey from file: %s", err) - return false, errors.Wrap(err, "failed to load rsa.PrivateKey from the file") + logrus.Debugf("Failed to load private key from file: %s", err) + return false, errors.Wrap(err, "failed to load private key from the file") + } + rsaKey, ok := key.(*rsa.PrivateKey) + if !ok { + return false, errors.New("bound service account signing key must be RSA") } pubData, err := PublicKeyToPem(&rsaKey.PublicKey) if err != nil { diff --git a/pkg/asset/tls/certkey.go b/pkg/asset/tls/certkey.go index 489ee158a05..b6513c74035 100644 --- a/pkg/asset/tls/certkey.go +++ b/pkg/asset/tls/certkey.go @@ -3,14 +3,13 @@ package tls import ( "bytes" "context" - "crypto/rsa" - "crypto/x509" "os" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/types" ) // CertInterface contains cert. @@ -128,14 +127,10 @@ func (c *SignedCertKey) Generate(_ context.Context, filenameBase string, appendParent AppendParentChoice, ) error { - var key *rsa.PrivateKey - var crt *x509.Certificate - var err error - caKey, err := PemToPrivateKey(parentCA.Key()) if err != nil { - logrus.Debugf("Failed to parse RSA private key: %s", err) - return errors.Wrap(err, "failed to parse rsa private key") + logrus.Debugf("Failed to parse private key: %s", err) + return errors.Wrap(err, "failed to parse private key") } caCert, err := PemToCertificate(parentCA.Cert()) @@ -144,13 +139,16 @@ func (c *SignedCertKey) Generate(_ context.Context, return errors.Wrap(err, "failed to parse x509 certificate") } - key, crt, err = GenerateSignedCertificate(caKey, caCert, cfg) + key, crt, err := GenerateSignedCertificate(caKey, caCert, cfg) if err != nil { logrus.Debugf("Failed to generate signed cert/key pair: %s", err) return errors.Wrap(err, "failed to generate signed cert/key pair") } - c.KeyRaw = PrivateKeyToPem(key) + c.KeyRaw, err = PrivateKeyToPem(key) + if err != nil { + return errors.Wrap(err, "failed to encode private key to PEM") + } c.CertRaw = CertToPem(crt) if appendParent { @@ -167,17 +165,23 @@ type SelfSignedCertKey struct { CertKey } -// Generate generates a cert/key pair signed by the specified parent CA. +// Generate generates a self-signed cert/key pair using the specified PKI profile. func (c *SelfSignedCertKey) Generate(_ context.Context, cfg *CertCfg, filenameBase string, + pkiConfig *types.PKIConfig, ) error { - key, crt, err := GenerateSelfSignedCertificate(cfg) + params := PKIConfigToKeyParams(pkiConfig) + + key, crt, err := GenerateSelfSignedCertificate(cfg, params) if err != nil { return errors.Wrap(err, "failed to generate self-signed cert/key pair") } - c.KeyRaw = PrivateKeyToPem(key) + c.KeyRaw, err = PrivateKeyToPem(key) + if err != nil { + return errors.Wrap(err, "failed to encode private key to PEM") + } c.CertRaw = CertToPem(crt) c.generateFiles(filenameBase) @@ -192,14 +196,10 @@ func RegenerateSignedCertKey( parentCA CertKeyInterface, appendParent AppendParentChoice, ) ([]byte, []byte, error) { - var key *rsa.PrivateKey - var crt *x509.Certificate - var err error - caKey, err := PemToPrivateKey(parentCA.Key()) if err != nil { - logrus.Debugf("Failed to parse RSA private key: %s", err) - return nil, nil, errors.Wrap(err, "failed to parse rsa private key") + logrus.Debugf("Failed to parse private key: %s", err) + return nil, nil, errors.Wrap(err, "failed to parse private key") } caCert, err := PemToCertificate(parentCA.Cert()) @@ -208,13 +208,16 @@ func RegenerateSignedCertKey( return nil, nil, errors.Wrap(err, "failed to parse x509 certificate") } - key, crt, err = GenerateSignedCertificate(caKey, caCert, cfg) - if err != nil { - logrus.Debugf("Failed to generate signed cert/key pair: %s", err) - return nil, nil, errors.Wrap(err, "failed to generate signed cert/key pair") + key, crt, generateErr := GenerateSignedCertificate(caKey, caCert, cfg) + if generateErr != nil { + logrus.Debugf("Failed to generate signed cert/key pair: %s", generateErr) + return nil, nil, errors.Wrap(generateErr, "failed to generate signed cert/key pair") } - keyRaw := PrivateKeyToPem(key) + keyRaw, err := PrivateKeyToPem(key) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to encode private key to PEM") + } certRaw := CertToPem(crt) if appendParent { diff --git a/pkg/asset/tls/certkey_test.go b/pkg/asset/tls/certkey_test.go index c8801c7e3d9..48e8112c164 100644 --- a/pkg/asset/tls/certkey_test.go +++ b/pkg/asset/tls/certkey_test.go @@ -2,12 +2,16 @@ package tls import ( "context" + "crypto/ecdsa" + "crypto/rsa" "crypto/x509" "crypto/x509/pkix" "net" "testing" "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/types" ) func TestSignedCertKeyGenerate(t *testing.T) { @@ -46,8 +50,14 @@ func TestSignedCertKeyGenerate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rootCA := &RootCA{} - err := rootCA.Generate(context.Background(), nil) + rootCA := &SelfSignedCertKey{} + rootCACfg := &CertCfg{ + Subject: pkix.Name{CommonName: "test-root-ca", OrganizationalUnit: []string{"openshift"}}, + KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + Validity: ValidityTenYears(), + IsCA: true, + } + err := rootCA.Generate(context.Background(), rootCACfg, "test-root-ca", nil) assert.NoError(t, err, "failed to generate root CA") certKey := &SignedCertKey{} @@ -90,3 +100,120 @@ func TestSignedCertKeyGenerate(t *testing.T) { }) } } + +func TestSelfSignedCertKeyGenerateWithPKIConfig(t *testing.T) { + cases := []struct { + name string + pkiConfig *types.PKIConfig + expectKeyType interface{} + expectPubKeyAlg x509.PublicKeyAlgorithm + }{ + { + name: "RSA 4096", + pkiConfig: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + }, + }, + expectKeyType: &rsa.PrivateKey{}, + expectPubKeyAlg: x509.RSA, + }, + { + name: "ECDSA P384", + pkiConfig: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + }, + expectKeyType: &ecdsa.PrivateKey{}, + expectPubKeyAlg: x509.ECDSA, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := &CertCfg{ + Subject: pkix.Name{CommonName: "test-pki-ca", OrganizationalUnit: []string{"openshift"}}, + Validity: ValidityTenYears(), + IsCA: true, + } + + ca := &SelfSignedCertKey{} + err := ca.Generate(context.Background(), cfg, "test-pki-ca", tc.pkiConfig) + assert.NoError(t, err) + + key, err := PemToPrivateKey(ca.Key()) + assert.NoError(t, err) + assert.IsType(t, tc.expectKeyType, key) + + cert, err := PemToCertificate(ca.Cert()) + assert.NoError(t, err) + assert.Equal(t, tc.expectPubKeyAlg, cert.PublicKeyAlgorithm) + assert.True(t, cert.IsCA) + }) + } +} + +func TestCrossAlgorithmCertificateSigning(t *testing.T) { + // Generate ECDSA P384 CA + ecdsaPKI := &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + } + rootCA := &SelfSignedCertKey{} + rootCACfg := &CertCfg{ + Subject: pkix.Name{CommonName: "ecdsa-ca", OrganizationalUnit: []string{"openshift"}}, + Validity: ValidityTenYears(), + IsCA: true, + } + err := rootCA.Generate(context.Background(), rootCACfg, "ecdsa-ca", ecdsaPKI) + assert.NoError(t, err) + + // Verify CA key is ECDSA + caKey, err := PemToPrivateKey(rootCA.Key()) + assert.NoError(t, err) + assert.IsType(t, &ecdsa.PrivateKey{}, caKey) + + // Generate RSA leaf signed by ECDSA CA + leafCfg := &CertCfg{ + Subject: pkix.Name{CommonName: "leaf-cert", OrganizationalUnit: []string{"openshift"}}, + KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + Validity: ValidityTenYears(), + DNSNames: []string{"test.openshift.io"}, + } + certKey := &SignedCertKey{} + err = certKey.Generate(context.Background(), leafCfg, rootCA, "cross-algo-leaf", DoNotAppendParent) + assert.NoError(t, err) + + // Verify leaf key is RSA (SignedCertKey always generates RSA leaf keys) + leafKey, err := PemToPrivateKey(certKey.Key()) + assert.NoError(t, err) + assert.IsType(t, &rsa.PrivateKey{}, leafKey) + + // Verify the leaf cert was signed by the ECDSA CA + leafCert, err := PemToCertificate(certKey.Cert()) + assert.NoError(t, err) + assert.Equal(t, x509.ECDSAWithSHA384, leafCert.SignatureAlgorithm) + + // Verify cert chain: leaf validates against CA + caCert, err := PemToCertificate(rootCA.Cert()) + assert.NoError(t, err) + certPool := x509.NewCertPool() + certPool.AddCert(caCert) + _, err = leafCert.Verify(x509.VerifyOptions{ + Roots: certPool, + DNSName: "test.openshift.io", + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + }) + assert.NoError(t, err, "leaf cert should validate against ECDSA CA") +} diff --git a/pkg/asset/tls/ironictls.go b/pkg/asset/tls/ironictls.go index 7ac8d6094d9..eef4697df92 100644 --- a/pkg/asset/tls/ironictls.go +++ b/pkg/asset/tls/ironictls.go @@ -56,7 +56,7 @@ func (a *IronicTLSCert) Generate(ctx context.Context, dependencies asset.Parents cfg.DNSNames = []string{hostname} logrus.Debugf("Generating TLS certificate for ironic (virtual media)") - return a.SelfSignedCertKey.Generate(ctx, cfg, "ironic/tls") + return a.SelfSignedCertKey.Generate(ctx, cfg, "ironic/tls", nil) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/keypair.go b/pkg/asset/tls/keypair.go index b8348b1d765..f122ff1af43 100644 --- a/pkg/asset/tls/keypair.go +++ b/pkg/asset/tls/keypair.go @@ -35,7 +35,10 @@ func (k *KeyPair) Generate(_ context.Context, filenameBase string) error { return errors.Wrap(err, "failed to get public key data from private key") } - k.Pvt = PrivateKeyToPem(key) + k.Pvt, err = PrivateKeyToPem(key) + if err != nil { + return errors.Wrap(err, "failed to encode private key to PEM") + } k.Pub = pubkeyData k.FileList = []*asset.File{ diff --git a/pkg/asset/tls/kubecontrolplane.go b/pkg/asset/tls/kubecontrolplane.go index 59735d3d6a0..5834ffdb83d 100644 --- a/pkg/asset/tls/kubecontrolplane.go +++ b/pkg/asset/tls/kubecontrolplane.go @@ -26,13 +26,13 @@ func (c *KubeControlPlaneSignerCertKey) Generate(ctx context.Context, parents as installConfig := &installconfig.InstallConfig{} parents.Get(installConfig) cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kube-control-plane-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityOneYear(installConfig), - IsCA: true, + Subject: pkix.Name{CommonName: "kube-control-plane-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityOneYear(installConfig), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-control-plane-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-control-plane-signer", nil) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/kubelet.go b/pkg/asset/tls/kubelet.go index bfec9000020..2d585d4a1b8 100644 --- a/pkg/asset/tls/kubelet.go +++ b/pkg/asset/tls/kubelet.go @@ -26,13 +26,13 @@ func (c *KubeletCSRSignerCertKey) Generate(ctx context.Context, parents asset.Pa installConfig := &installconfig.InstallConfig{} parents.Get(installConfig) cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kubelet-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityOneDay(installConfig), - IsCA: true, + Subject: pkix.Name{CommonName: "kubelet-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityOneDay(installConfig), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-signer", nil) } // Name returns the human-friendly name of the asset. @@ -108,7 +108,13 @@ type KubeletBootstrapCertSigner struct { var _ asset.WritableAsset = (*KubeletBootstrapCertSigner)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// Dependencies returns no dependencies. Configurable PKI requires +// reading the PKI config from InstallConfig, but adding InstallConfig +// as a dependency here would break codepaths that generate signer certs +// without an install-config on disk (e.g. agent create certificates, +// node-joiner). A follow-up commit introduces an intermediate asset +// that reads the config directly from disk without triggering +// InstallConfig validation. func (c *KubeletBootstrapCertSigner) Dependencies() []asset.Asset { return []asset.Asset{} } @@ -116,13 +122,13 @@ func (c *KubeletBootstrapCertSigner) Dependencies() []asset.Asset { // Generate generates the root-ca key and cert pair. func (c *KubeletBootstrapCertSigner) Generate(ctx context.Context, parents asset.Parents) error { cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "kubelet-bootstrap-kubeconfig-signer", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityTenYears(), - IsCA: true, + Subject: pkix.Name{CommonName: "kubelet-bootstrap-kubeconfig-signer", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityTenYears(), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-bootstrap-kubeconfig-signer") + return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-bootstrap-kubeconfig-signer", nil) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/root.go b/pkg/asset/tls/root.go index 6529cdc7890..9ab310f68f8 100644 --- a/pkg/asset/tls/root.go +++ b/pkg/asset/tls/root.go @@ -2,7 +2,6 @@ package tls import ( "context" - "crypto/x509" "crypto/x509/pkix" "github.com/openshift/installer/pkg/asset" @@ -22,7 +21,13 @@ type RootCA struct { var _ asset.WritableAsset = (*RootCA)(nil) -// Dependencies returns nothing. +// Dependencies returns no dependencies. Configurable PKI requires +// reading the PKI config from InstallConfig, but adding InstallConfig +// as a dependency here would break codepaths that generate signer certs +// without an install-config on disk (e.g. agent create certificates, +// node-joiner). A follow-up commit introduces an intermediate asset +// that reads the config directly from disk without triggering +// InstallConfig validation. func (c *RootCA) Dependencies() []asset.Asset { return []asset.Asset{} } @@ -30,13 +35,13 @@ func (c *RootCA) Dependencies() []asset.Asset { // Generate generates the MCS/Ignition CA. func (c *RootCA) Generate(ctx context.Context, parents asset.Parents) error { cfg := &CertCfg{ - Subject: pkix.Name{CommonName: "root-ca", OrganizationalUnit: []string{"openshift"}}, - KeyUsages: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - Validity: ValidityTenYears(), - IsCA: true, + Subject: pkix.Name{CommonName: "root-ca", OrganizationalUnit: []string{"openshift"}}, + // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. + Validity: ValidityTenYears(), + IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "root-ca") + return c.SelfSignedCertKey.Generate(ctx, cfg, "root-ca", nil) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/tls.go b/pkg/asset/tls/tls.go index f027bcd7bbe..f90c6a3d21b 100644 --- a/pkg/asset/tls/tls.go +++ b/pkg/asset/tls/tls.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "fmt" "math" "math/big" "net" @@ -20,12 +21,103 @@ import ( features "github.com/openshift/api/features" "github.com/openshift/installer/pkg/asset/installconfig" + "github.com/openshift/installer/pkg/types" ) const ( - keySize = 2048 + // DefaultRSAKeySize is the default RSA key size used when PKI config is not specified. + DefaultRSAKeySize int32 = 2048 + // DefaultKeyAlgorithm is the default key algorithm used when PKI config is not specified. + DefaultKeyAlgorithm = types.KeyAlgorithmRSA ) +// PrivateKeyParams specifies parameters for private key generation. +type PrivateKeyParams struct { + Algorithm types.KeyAlgorithm + RSAKeySize int32 + ECDSACurve types.ECDSACurve +} + +// PKIConfigToKeyParams converts PKI config to key generation parameters. +// If pkiConfig is nil, returns default RSA 2048 parameters. +func PKIConfigToKeyParams(pkiConfig *types.PKIConfig) PrivateKeyParams { + if pkiConfig == nil { + return PrivateKeyParams{ + Algorithm: DefaultKeyAlgorithm, + RSAKeySize: DefaultRSAKeySize, + } + } + + keyConfig := pkiConfig.SignerCertificates.Key + params := PrivateKeyParams{ + Algorithm: keyConfig.Algorithm, + } + + switch keyConfig.Algorithm { + case types.KeyAlgorithmRSA: + params.RSAKeySize = keyConfig.RSA.KeySize + case types.KeyAlgorithmECDSA: + params.ECDSACurve = keyConfig.ECDSA.Curve + } + + return params +} + +// GeneratePrivateKeyWithParams generates a private key with the specified parameters. +func GeneratePrivateKeyWithParams(params PrivateKeyParams) (crypto.PrivateKey, error) { + switch params.Algorithm { + case types.KeyAlgorithmRSA: + return GenerateRSAPrivateKey(params.RSAKeySize) + case types.KeyAlgorithmECDSA: + return GenerateECDSAPrivateKey(params.ECDSACurve) + default: + return nil, fmt.Errorf("unsupported algorithm: %s", params.Algorithm) + } +} + +// GenerateRSAPrivateKey generates an RSA private key with the specified size. +func GenerateRSAPrivateKey(keySize int32) (*rsa.PrivateKey, error) { + rsaKey, err := rsa.GenerateKey(rand.Reader, int(keySize)) + if err != nil { + return nil, errors.Wrap(err, "error generating RSA private key") + } + return rsaKey, nil +} + +// GenerateECDSAPrivateKey generates an ECDSA private key with the specified curve. +func GenerateECDSAPrivateKey(curve types.ECDSACurve) (*ecdsa.PrivateKey, error) { + var c elliptic.Curve + + switch curve { + case types.ECDSACurveP256: + c = elliptic.P256() + case types.ECDSACurveP384: + c = elliptic.P384() + case types.ECDSACurveP521: + c = elliptic.P521() + default: + return nil, fmt.Errorf("unsupported ECDSA curve: %s", curve) + } + + ecdsaKey, err := ecdsa.GenerateKey(c, rand.Reader) + if err != nil { + return nil, errors.Wrap(err, "error generating ECDSA private key") + } + return ecdsaKey, nil +} + +// keyUsageForAlgorithm returns appropriate x509.KeyUsage flags for the given algorithm. +// ECDSA keys can only perform digital signatures — they cannot perform key encipherment. +// RSA keys support both digital signatures and key encipherment. +func keyUsageForAlgorithm(algorithm types.KeyAlgorithm) x509.KeyUsage { + switch algorithm { + case types.KeyAlgorithmECDSA: + return x509.KeyUsageDigitalSignature + default: // RSA + return x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature + } +} + // CertCfg contains all needed fields to configure a new certificate type CertCfg struct { DNSNames []string @@ -43,18 +135,17 @@ type rsaPublicKey struct { E int } -// PrivateKey generates an RSA Private key and returns the value +// PrivateKey generates an RSA private key with default parameters (for leaf certs). func PrivateKey() (*rsa.PrivateKey, error) { - rsaKey, err := rsa.GenerateKey(rand.Reader, keySize) + rsaKey, err := rsa.GenerateKey(rand.Reader, int(DefaultRSAKeySize)) if err != nil { return nil, errors.Wrap(err, "error generating RSA private key") } - return rsaKey, nil } // SelfSignedCertificate creates a self signed certificate -func SelfSignedCertificate(cfg *CertCfg, key *rsa.PrivateKey) (*x509.Certificate, error) { +func SelfSignedCertificate(cfg *CertCfg, key crypto.Signer) (*x509.Certificate, error) { serial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64)) if err != nil { return nil, err @@ -90,7 +181,7 @@ func SignedCertificate( csr *x509.CertificateRequest, key *rsa.PrivateKey, caCert *x509.Certificate, - caKey *rsa.PrivateKey, + caKey crypto.PrivateKey, ) (*x509.Certificate, error) { serial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64)) if err != nil { @@ -144,7 +235,7 @@ func generateSubjectKeyID(pub crypto.PublicKey) ([]byte, error) { } // GenerateSignedCertificate generate a key and cert defined by CertCfg and signed by CA. -func GenerateSignedCertificate(caKey *rsa.PrivateKey, caCert *x509.Certificate, +func GenerateSignedCertificate(caKey crypto.PrivateKey, caCert *x509.Certificate, cfg *CertCfg) (*rsa.PrivateKey, *x509.Certificate, error) { // create a private key @@ -175,17 +266,31 @@ func GenerateSignedCertificate(caKey *rsa.PrivateKey, caCert *x509.Certificate, return key, cert, nil } -// GenerateSelfSignedCertificate generates a key/cert pair defined by CertCfg. -func GenerateSelfSignedCertificate(cfg *CertCfg) (*rsa.PrivateKey, *x509.Certificate, error) { - key, err := PrivateKey() +// GenerateSelfSignedCertificate generates a key/cert pair defined by CertCfg +// using the specified key parameters. KeyUsage is automatically adjusted +// based on the algorithm (ECDSA cannot have KeyEncipherment). +func GenerateSelfSignedCertificate(cfg *CertCfg, params PrivateKeyParams) (crypto.PrivateKey, *x509.Certificate, error) { + key, err := GeneratePrivateKeyWithParams(params) if err != nil { - logrus.Debugf("Failed to generate a private key: %s", err) return nil, nil, errors.Wrap(err, "failed to generate private key") } - crt, err := SelfSignedCertificate(cfg, key) + // Set KeyUsage based on algorithm — ECDSA keys cannot perform key encipherment + adjustedCfg := *cfg + baseUsage := keyUsageForAlgorithm(params.Algorithm) + if cfg.IsCA { + adjustedCfg.KeyUsages = baseUsage | x509.KeyUsageCertSign + } else { + adjustedCfg.KeyUsages = baseUsage + } + + signer, ok := key.(crypto.Signer) + if !ok { + return nil, nil, fmt.Errorf("generated key does not implement crypto.Signer") + } + + crt, err := SelfSignedCertificate(&adjustedCfg, signer) if err != nil { - logrus.Debugf("Failed to create self-signed certificate: %s", err) return nil, nil, errors.Wrap(err, "failed to create self-signed certificate") } return key, crt, nil diff --git a/pkg/asset/tls/tls_test.go b/pkg/asset/tls/tls_test.go index 116cf046fb3..feac611ddda 100644 --- a/pkg/asset/tls/tls_test.go +++ b/pkg/asset/tls/tls_test.go @@ -1,11 +1,18 @@ package tls import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" + "crypto/rsa" "crypto/x509" "crypto/x509/pkix" "testing" "time" + + "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/types" ) func TestSelfSignedCertificate(t *testing.T) { @@ -105,3 +112,197 @@ func TestSignedCertificate(t *testing.T) { } } } + +func TestGenerateRSAPrivateKey(t *testing.T) { + cases := []struct { + name string + keySize int32 + }{ + {"RSA 2048", 2048}, + {"RSA 4096", 4096}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + key, err := GenerateRSAPrivateKey(tc.keySize) + assert.NoError(t, err) + assert.IsType(t, &rsa.PrivateKey{}, key) + assert.Equal(t, int(tc.keySize), key.N.BitLen()) + }) + } +} + +func TestGenerateECDSAPrivateKey(t *testing.T) { + cases := []struct { + name string + curve types.ECDSACurve + expected elliptic.Curve + expectErr bool + }{ + {"P256", types.ECDSACurveP256, elliptic.P256(), false}, + {"P384", types.ECDSACurveP384, elliptic.P384(), false}, + {"P521", types.ECDSACurveP521, elliptic.P521(), false}, + {"invalid", "P224", nil, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + key, err := GenerateECDSAPrivateKey(tc.curve) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.IsType(t, &ecdsa.PrivateKey{}, key) + assert.Equal(t, tc.expected, key.Curve) + }) + } +} + +func TestGenerateSelfSignedCertificateWithParams(t *testing.T) { + cases := []struct { + name string + params PrivateKeyParams + expectKeyType interface{} + expectPubKeyAlg x509.PublicKeyAlgorithm + }{ + { + name: "RSA 4096 CA", + params: PrivateKeyParams{ + Algorithm: types.KeyAlgorithmRSA, + RSAKeySize: 4096, + }, + expectKeyType: &rsa.PrivateKey{}, + expectPubKeyAlg: x509.RSA, + }, + { + name: "ECDSA P384 CA", + params: PrivateKeyParams{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSACurve: types.ECDSACurveP384, + }, + expectKeyType: &ecdsa.PrivateKey{}, + expectPubKeyAlg: x509.ECDSA, + }, + { + name: "RSA 2048 CA (default)", + params: PrivateKeyParams{ + Algorithm: types.KeyAlgorithmRSA, + RSAKeySize: 2048, + }, + expectKeyType: &rsa.PrivateKey{}, + expectPubKeyAlg: x509.RSA, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := &CertCfg{ + Subject: pkix.Name{CommonName: "test-ca", OrganizationalUnit: []string{"openshift"}}, + Validity: time.Hour, + IsCA: true, + } + key, cert, err := GenerateSelfSignedCertificate(cfg, tc.params) + assert.NoError(t, err) + assert.IsType(t, tc.expectKeyType, key) + assert.Equal(t, tc.expectPubKeyAlg, cert.PublicKeyAlgorithm) + assert.True(t, cert.IsCA) + }) + } +} + +func TestKeyUsageForAlgorithm(t *testing.T) { + cases := []struct { + name string + params PrivateKeyParams + isCA bool + wantUsage x509.KeyUsage + notUsage x509.KeyUsage + }{ + { + name: "RSA signer", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmRSA, RSAKeySize: 2048}, + isCA: true, + wantUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign, + }, + { + name: "ECDSA signer", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmECDSA, ECDSACurve: types.ECDSACurveP256}, + isCA: true, + wantUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + notUsage: x509.KeyUsageKeyEncipherment, + }, + { + name: "RSA non-CA", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmRSA, RSAKeySize: 2048}, + isCA: false, + wantUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + notUsage: x509.KeyUsageCertSign, + }, + { + name: "ECDSA non-CA", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmECDSA, ECDSACurve: types.ECDSACurveP384}, + isCA: false, + wantUsage: x509.KeyUsageDigitalSignature, + notUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := &CertCfg{ + Subject: pkix.Name{CommonName: "test", OrganizationalUnit: []string{"openshift"}}, + Validity: time.Hour, + IsCA: tc.isCA, + } + _, cert, err := GenerateSelfSignedCertificate(cfg, tc.params) + assert.NoError(t, err) + assert.Equal(t, tc.wantUsage, cert.KeyUsage, "KeyUsage mismatch") + if tc.notUsage != 0 { + assert.Zero(t, cert.KeyUsage&tc.notUsage, "unexpected KeyUsage bits set") + } + }) + } +} + +func TestSignatureAlgorithmAutoDetection(t *testing.T) { + cases := []struct { + name string + params PrivateKeyParams + expected x509.SignatureAlgorithm + }{ + { + name: "RSA", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmRSA, RSAKeySize: 2048}, + expected: x509.SHA256WithRSA, + }, + { + name: "ECDSA P256", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmECDSA, ECDSACurve: types.ECDSACurveP256}, + expected: x509.ECDSAWithSHA256, + }, + { + name: "ECDSA P384", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmECDSA, ECDSACurve: types.ECDSACurveP384}, + expected: x509.ECDSAWithSHA384, + }, + { + name: "ECDSA P521", + params: PrivateKeyParams{Algorithm: types.KeyAlgorithmECDSA, ECDSACurve: types.ECDSACurveP521}, + expected: x509.ECDSAWithSHA512, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := &CertCfg{ + Subject: pkix.Name{CommonName: "test-sig", OrganizationalUnit: []string{"openshift"}}, + Validity: time.Hour, + IsCA: true, + } + _, cert, err := GenerateSelfSignedCertificate(cfg, tc.params) + assert.NoError(t, err) + assert.Equal(t, tc.expected, cert.SignatureAlgorithm) + }) + } +} diff --git a/pkg/asset/tls/utils.go b/pkg/asset/tls/utils.go index dd60f187b97..e619b12f36e 100644 --- a/pkg/asset/tls/utils.go +++ b/pkg/asset/tls/utils.go @@ -1,24 +1,41 @@ package tls import ( + "crypto" + "crypto/ecdsa" "crypto/rsa" "crypto/x509" "encoding/pem" + "fmt" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -// PrivateKeyToPem converts an rsa.PrivateKey object to pem string -func PrivateKeyToPem(key *rsa.PrivateKey) []byte { - keyInBytes := x509.MarshalPKCS1PrivateKey(key) - keyinPem := pem.EncodeToMemory( - &pem.Block{ +// PrivateKeyToPem converts a private key (RSA or ECDSA) to PEM format. +func PrivateKeyToPem(key crypto.PrivateKey) ([]byte, error) { + var block *pem.Block + + switch k := key.(type) { + case *rsa.PrivateKey: + block = &pem.Block{ Type: "RSA PRIVATE KEY", - Bytes: keyInBytes, - }, - ) - return keyinPem + Bytes: x509.MarshalPKCS1PrivateKey(k), + } + case *ecdsa.PrivateKey: + bytes, err := x509.MarshalECPrivateKey(k) + if err != nil { + return nil, fmt.Errorf("failed to marshal ECDSA private key: %w", err) + } + block = &pem.Block{ + Type: "EC PRIVATE KEY", + Bytes: bytes, + } + default: + return nil, fmt.Errorf("unsupported private key type: %T", key) + } + + return pem.EncodeToMemory(block), nil } // CertToPem converts an x509.Certificate object to a pem string @@ -59,13 +76,32 @@ func PublicKeyToPem(key *rsa.PublicKey) ([]byte, error) { return keyinPem, nil } -// PemToPrivateKey converts a data block to rsa.PrivateKey. -func PemToPrivateKey(data []byte) (*rsa.PrivateKey, error) { +// PemToPrivateKey converts a PEM data block to a private key (RSA or ECDSA). +func PemToPrivateKey(data []byte) (crypto.PrivateKey, error) { block, _ := pem.Decode(data) if block == nil { return nil, errors.Errorf("could not find a PEM block in the private key") } - return x509.ParsePKCS1PrivateKey(block.Bytes) + + switch block.Type { + case "RSA PRIVATE KEY": + return x509.ParsePKCS1PrivateKey(block.Bytes) + case "EC PRIVATE KEY": + return x509.ParseECPrivateKey(block.Bytes) + case "PRIVATE KEY": + key, err := x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, err + } + switch key.(type) { + case *rsa.PrivateKey, *ecdsa.PrivateKey: + return key, nil + default: + return nil, fmt.Errorf("unsupported PKCS#8 key type: %T", key) + } + default: + return nil, fmt.Errorf("unsupported PEM block type: %s", block.Type) + } } // PemToPublicKey converts a data block to rsa.PublicKey. diff --git a/pkg/asset/tls/utils_test.go b/pkg/asset/tls/utils_test.go new file mode 100644 index 00000000000..91e8a20d83e --- /dev/null +++ b/pkg/asset/tls/utils_test.go @@ -0,0 +1,92 @@ +package tls + +import ( + "crypto/ecdsa" + "crypto/rsa" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/types" +) + +func TestPrivateKeyToPemRoundtrip(t *testing.T) { + cases := []struct { + name string + genFunc func() (interface{}, error) + expectType interface{} + }{ + { + name: "RSA key", + genFunc: func() (interface{}, error) { + return GenerateRSAPrivateKey(2048) + }, + expectType: &rsa.PrivateKey{}, + }, + { + name: "ECDSA P256 key", + genFunc: func() (interface{}, error) { + return GenerateECDSAPrivateKey(types.ECDSACurveP256) + }, + expectType: &ecdsa.PrivateKey{}, + }, + { + name: "ECDSA P384 key", + genFunc: func() (interface{}, error) { + return GenerateECDSAPrivateKey(types.ECDSACurveP384) + }, + expectType: &ecdsa.PrivateKey{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + key, err := tc.genFunc() + assert.NoError(t, err) + + pemBytes, err := PrivateKeyToPem(key) + assert.NoError(t, err) + assert.NotEmpty(t, pemBytes) + + decoded, err := PemToPrivateKey(pemBytes) + assert.NoError(t, err) + assert.IsType(t, tc.expectType, decoded) + }) + } +} + +func TestPemToPrivateKeyFormats(t *testing.T) { + t.Run("invalid PEM", func(t *testing.T) { + _, err := PemToPrivateKey([]byte("not a PEM")) + assert.Error(t, err) + }) + + t.Run("empty data", func(t *testing.T) { + _, err := PemToPrivateKey([]byte{}) + assert.Error(t, err) + }) + + t.Run("RSA PEM block", func(t *testing.T) { + key, err := GenerateRSAPrivateKey(2048) + assert.NoError(t, err) + pemBytes, pemErr := PrivateKeyToPem(key) + assert.NoError(t, pemErr) + + decoded, err := PemToPrivateKey(pemBytes) + assert.NoError(t, err) + _, ok := decoded.(*rsa.PrivateKey) + assert.True(t, ok, "expected *rsa.PrivateKey") + }) + + t.Run("EC PEM block", func(t *testing.T) { + key, err := GenerateECDSAPrivateKey(types.ECDSACurveP256) + assert.NoError(t, err) + pemBytes, pemErr := PrivateKeyToPem(key) + assert.NoError(t, pemErr) + + decoded, err := PemToPrivateKey(pemBytes) + assert.NoError(t, err) + _, ok := decoded.(*ecdsa.PrivateKey) + assert.True(t, ok, "expected *ecdsa.PrivateKey") + }) +}