From 85252ceb69f5d277c0170f0bde251af1e8950a28 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 | 120 ++++++++ pkg/asset/manifests/pki_test.go | 165 +++++++++++ pkg/explain/printer_test.go | 6 + pkg/types/installconfig.go | 105 +++++++ pkg/types/pki/defaults.go | 27 ++ pkg/types/pki/defaults_test.go | 18 ++ pkg/types/pki/validation.go | 94 ++++++ pkg/types/pki/validation_test.go | 280 ++++++++++++++++++ pkg/types/validation/featuregate_test.go | 35 +++ pkg/types/validation/featuregates.go | 5 + pkg/types/validation/installconfig.go | 5 + pkg/types/validation/installconfig_test.go | 49 +++ pkg/types/zz_generated.deepcopy.go | 97 ++++++ 15 files changed, 1086 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/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..f0d6433cd2d --- /dev/null +++ b/pkg/asset/manifests/pki.go @@ -0,0 +1,120 @@ +package manifests + +import ( + "context" + "fmt" + "path" + + 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" + "github.com/openshift/installer/pkg/types" + 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 = convertToAPICertConfig(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 fmt.Errorf("failed to marshal PKI config: %w", err) + } + + 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 +} + +// convertToAPICertConfig converts the installer CertificateConfig +// to the openshift/api configv1alpha1.CertificateConfig for use in the PKI CR manifest. +func convertToAPICertConfig(certConf types.CertificateConfig) configv1alpha1.CertificateConfig { + out := configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithm(certConf.Key.Algorithm), + }, + } + if certConf.Key.RSA != nil { + out.Key.RSA = configv1alpha1.RSAKeyConfig{KeySize: certConf.Key.RSA.KeySize} + } + if certConf.Key.ECDSA != nil { + out.Key.ECDSA = configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurve(certConf.Key.ECDSA.Curve)} + } + return out +} diff --git a/pkg/asset/manifests/pki_test.go b/pkg/asset/manifests/pki_test.go new file mode 100644 index 00000000000..4136acd9ac7 --- /dev/null +++ b/pkg/asset/manifests/pki_test.go @@ -0,0 +1,165 @@ +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.KeyAlgorithmRSA { + assert.Equal(t, tc.expectDefaultsRSA, profile.Defaults.Key.RSA.KeySize) + } + if tc.expectDefaultsAlgo == configv1alpha1.KeyAlgorithmECDSA { + assert.Equal(t, tc.expectDefaultsCurve, profile.Defaults.Key.ECDSA.Curve) + } + + // 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/installconfig.go b/pkg/types/installconfig.go index fcfda23cc9e..5aacad468c9 100644 --- a/pkg/types/installconfig.go +++ b/pkg/types/installconfig.go @@ -231,8 +231,113 @@ 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 in openshift/api do not +// silently appear in the install-config YAML API without explicit opt-in and +// validation by the installer. Conversion to the openshift/api 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. +// +// +union +// +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" +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/defaults.go b/pkg/types/pki/defaults.go new file mode 100644 index 00000000000..951c876dccd --- /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}, + }, + }, + } +} 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..542d40096a0 --- /dev/null +++ b/pkg/types/pki/validation.go @@ -0,0 +1,94 @@ +package pki + +import ( + "k8s.io/apimachinery/pkg/util/sets" + "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. +func ValidatePKIConfig(pkiConfig *types.PKIConfig, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if pkiConfig == nil { + return allErrs + } + + allErrs = append(allErrs, ValidateKeyConfig(pkiConfig.SignerCertificates.Key, + fldPath.Child("signerCertificates", "key"))...) + + return allErrs +} + +// ValidateKeyConfig validates the KeyConfig structure. +func ValidateKeyConfig(config types.KeyConfig, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if config.Algorithm == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("algorithm"), + "algorithm must be specified")) + return allErrs + } + + validAlgorithms := []types.KeyAlgorithm{types.KeyAlgorithmRSA, types.KeyAlgorithmECDSA} + if !sets.New(validAlgorithms...).Has(config.Algorithm) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("algorithm"), + config.Algorithm, validAlgorithms)) + return allErrs + } + + if config.Algorithm == types.KeyAlgorithmRSA { + if config.RSA == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("rsa"), + "rsa is required when algorithm is RSA")) + } else { + allErrs = append(allErrs, validateRSAKeyConfig(*config.RSA, fldPath.Child("rsa"))...) + } + + if config.ECDSA != nil { + 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 == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("ecdsa"), + "ecdsa is required when algorithm is ECDSA")) + } else { + allErrs = append(allErrs, validateECDSAKeyConfig(*config.ECDSA, fldPath.Child("ecdsa"))...) + } + + if config.RSA != nil { + 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) field.ErrorList { + allErrs := field.ErrorList{} + + 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) field.ErrorList { + allErrs := field.ErrorList{} + + validCurves := []types.ECDSACurve{types.ECDSACurveP256, types.ECDSACurveP384, types.ECDSACurveP521} + if !sets.New(validCurves...).Has(config.Curve) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("curve"), + config.Curve, validCurves)) + } + + return allErrs +} diff --git a/pkg/types/pki/validation_test.go b/pkg/types/pki/validation_test.go new file mode 100644 index 00000000000..9cb91035636 --- /dev/null +++ b/pkg/types/pki/validation_test.go @@ -0,0 +1,280 @@ +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 + expectError bool + errorCount int + errorMsg string + }{ + { + 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 - algorithm required", + pkiConfig: &types.PKIConfig{}, + expectError: true, + errorCount: 1, + errorMsg: "algorithm must be specified", + }, + { + 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, + errorMsg: "must be a multiple of 1024 from 2048 to 8192", + }, + { + 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, + errorMsg: "supported values", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := ValidatePKIConfig(tc.pkiConfig, fldPath) + if tc.expectError { + if assert.Len(t, errs, tc.errorCount) { + assert.Regexp(t, tc.errorMsg, errs.ToAggregate()) + } + } else { + assert.Empty(t, errs) + } + }) + } +} + +func TestValidateKeyConfig(t *testing.T) { + fldPath := field.NewPath("pki", "signerCertificates", "key") + + cases := []struct { + name string + config types.KeyConfig + expectError bool + errorCount int + errorMsg string + }{ + // 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, + errorMsg: "algorithm must be specified", + }, + // Invalid: unsupported algorithm + { + name: "unsupported algorithm", + config: types.KeyConfig{ + Algorithm: "Ed25519", + }, + expectError: true, + errorCount: 1, + errorMsg: "supported values", + }, + // 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, + errorMsg: "must be a multiple of 1024 from 2048 to 8192", + }, + { + name: "RSA key size too large", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: &types.RSAKeyConfig{KeySize: 9216}, + }, + expectError: true, + errorCount: 1, + errorMsg: "must be a multiple of 1024 from 2048 to 8192", + }, + { + name: "RSA key size not multiple of 1024", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: &types.RSAKeyConfig{KeySize: 5000}, + }, + expectError: true, + errorCount: 1, + errorMsg: "must be a multiple of 1024 from 2048 to 8192", + }, + { + name: "RSA missing rsa config", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + }, + expectError: true, + errorCount: 1, + errorMsg: "rsa is required when algorithm is RSA", + }, + // Invalid ECDSA curves + { + name: "ECDSA invalid curve", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: &types.ECDSAKeyConfig{Curve: "P224"}, + }, + expectError: true, + errorCount: 1, + errorMsg: "supported values", + }, + { + name: "ECDSA missing ecdsa config", + config: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + }, + expectError: true, + errorCount: 1, + errorMsg: "ecdsa is required when algorithm is ECDSA", + }, + // 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, + errorMsg: "ecdsa must not be set when algorithm is RSA", + }, + { + 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, + errorMsg: "rsa must not be set when algorithm is ECDSA", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := ValidateKeyConfig(tc.config, fldPath) + if tc.expectError { + if assert.Len(t, errs, tc.errorCount, "expected %d errors, got: %v", tc.errorCount, errs) { + assert.Regexp(t, tc.errorMsg, errs.ToAggregate()) + } + } 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..8bdea836979 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -48,6 +48,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 +285,10 @@ func ValidateInstallConfig(c *types.InstallConfig, usingAgentMethod bool) field. } } + if c.PKI != nil { + allErrs = append(allErrs, pkivalidation.ValidatePKIConfig(c.PKI, field.NewPath("pki"))...) + } + 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..2a85fe0f63e 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..0cf5571f75a 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 + in.Key.DeepCopyInto(&out.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) + (*in).DeepCopyInto(*out) + } return } @@ -461,6 +499,32 @@ 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 + if in.RSA != nil { + in, out := &in.RSA, &out.RSA + *out = new(RSAKeyConfig) + **out = **in + } + if in.ECDSA != nil { + in, out := &in.ECDSA, &out.ECDSA + *out = new(ECDSAKeyConfig) + **out = **in + } + 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 +784,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 + in.SignerCertificates.DeepCopyInto(&out.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 +897,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 7e5b690f2d4b046a0d68a327167f5d1044bbebb2 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 | 12 +- pkg/asset/tls/certkey.go | 61 +++--- pkg/asset/tls/certkey_test.go | 131 +++++++++++- pkg/asset/tls/ironictls.go | 2 +- pkg/asset/tls/keypair.go | 12 +- pkg/asset/tls/kubecontrolplane.go | 10 +- pkg/asset/tls/kubelet.go | 28 ++- pkg/asset/tls/root.go | 19 +- pkg/asset/tls/tls.go | 139 ++++++++++-- pkg/asset/tls/tls_test.go | 201 ++++++++++++++++++ pkg/asset/tls/utils.go | 60 ++++-- pkg/asset/tls/utils_test.go | 92 ++++++++ 16 files changed, 742 insertions(+), 132 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..d14ae648bcd 100644 --- a/pkg/asset/tls/boundsasigningkey.go +++ b/pkg/asset/tls/boundsasigningkey.go @@ -2,6 +2,8 @@ package tls import ( "context" + "crypto/rsa" + "fmt" "os" "github.com/pkg/errors" @@ -50,10 +52,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, fmt.Errorf("failed to load private key from the file: %w", err) + } + rsaKey, ok := key.(*rsa.PrivateKey) + if !ok { + return false, fmt.Errorf("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..f51d93ad1ca 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" + "fmt" "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,29 +127,28 @@ 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 fmt.Errorf("failed to parse private key: %w", err) } caCert, err := PemToCertificate(parentCA.Cert()) if err != nil { logrus.Debugf("Failed to parse x509 certificate: %s", err) - return errors.Wrap(err, "failed to parse x509 certificate") + return fmt.Errorf("failed to parse x509 certificate: %w", err) } - 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") + return fmt.Errorf("failed to generate signed cert/key pair: %w", err) } - c.KeyRaw = PrivateKeyToPem(key) + c.KeyRaw, err = PrivateKeyToPem(key) + if err != nil { + return fmt.Errorf("failed to encode private key to PEM: %w", err) + } 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") + return fmt.Errorf("failed to generate self-signed cert/key pair: %w", err) } - c.KeyRaw = PrivateKeyToPem(key) + c.KeyRaw, err = PrivateKeyToPem(key) + if err != nil { + return fmt.Errorf("failed to encode private key to PEM: %w", err) + } c.CertRaw = CertToPem(crt) c.generateFiles(filenameBase) @@ -192,29 +196,28 @@ 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, fmt.Errorf("failed to parse private key: %w", err) } caCert, err := PemToCertificate(parentCA.Cert()) if err != nil { logrus.Debugf("Failed to parse x509 certificate: %s", err) - return nil, nil, errors.Wrap(err, "failed to parse x509 certificate") + return nil, nil, fmt.Errorf("failed to parse x509 certificate: %w", err) } - 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, fmt.Errorf("failed to generate signed cert/key pair: %w", generateErr) } - keyRaw := PrivateKeyToPem(key) + keyRaw, err := PrivateKeyToPem(key) + if err != nil { + return nil, nil, fmt.Errorf("failed to encode private key to PEM: %w", err) + } certRaw := CertToPem(crt) if appendParent { diff --git a/pkg/asset/tls/certkey_test.go b/pkg/asset/tls/certkey_test.go index c8801c7e3d9..a4905ca76b5 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..0b9109692af 100644 --- a/pkg/asset/tls/keypair.go +++ b/pkg/asset/tls/keypair.go @@ -2,8 +2,7 @@ package tls import ( "context" - - "github.com/pkg/errors" + "fmt" "github.com/openshift/installer/pkg/asset" ) @@ -27,15 +26,18 @@ type KeyPair struct { func (k *KeyPair) Generate(_ context.Context, filenameBase string) error { key, err := PrivateKey() if err != nil { - return errors.Wrap(err, "failed to generate private key") + return fmt.Errorf("failed to generate private key: %w", err) } pubkeyData, err := PublicKeyToPem(&key.PublicKey) if err != nil { - return errors.Wrap(err, "failed to get public key data from private key") + return fmt.Errorf("failed to get public key data from private key: %w", err) } - k.Pvt = PrivateKeyToPem(key) + k.Pvt, err = PrivateKeyToPem(key) + if err != nil { + return fmt.Errorf("failed to encode private key to PEM: %w", err) + } 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..a9700e8a696 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,111 @@ 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 + // DefaultECDSACurve is the default ECDSA curve used when PKI config is not specified. + DefaultECDSACurve = types.ECDSACurveP384 + // 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 = DefaultRSAKeySize + if keyConfig.RSA != nil { + params.RSAKeySize = keyConfig.RSA.KeySize + } + case types.KeyAlgorithmECDSA: + params.ECDSACurve = DefaultECDSACurve + if keyConfig.ECDSA != nil { + 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, fmt.Errorf("error generating RSA private key: %w", err) + } + 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, fmt.Errorf("error generating ECDSA private key: %w", err) + } + 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 +143,13 @@ 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) - if err != nil { - return nil, errors.Wrap(err, "error generating RSA private key") - } - - return rsaKey, nil + return GenerateRSAPrivateKey(DefaultRSAKeySize) } // 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 +185,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 +239,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 +270,27 @@ 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. cfg.KeyUsages is ignored — KeyUsage is +// set based on the algorithm (ECDSA gets DigitalSignature only; RSA gets +// DigitalSignature|KeyEncipherment) plus CertSign for CAs. +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 + } + + crt, err := SelfSignedCertificate(&adjustedCfg, key.(crypto.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") + }) +}