From 6f6a58a64a5da3cb33d146f2c7c077cb9869cea0 Mon Sep 17 00:00:00 2001 From: Haseeb Tariq Date: Wed, 3 Jun 2026 11:26:04 -0700 Subject: [PATCH 1/3] 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/defaults.go | 55 ++++ pkg/types/pki/defaults_test.go | 104 +++++++ 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 ++++++ 17 files changed, 1200 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/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/defaults.go b/pkg/types/pki/defaults.go new file mode 100644 index 00000000000..a8dd15aff44 --- /dev/null +++ b/pkg/types/pki/defaults.go @@ -0,0 +1,55 @@ +package pki + +import ( + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + features "github.com/openshift/api/features" + "github.com/openshift/installer/pkg/types" +) + +// 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}, + }, + }, + } +} + +// EffectiveSignerPKIConfig returns the effective PKI config for signer certificate generation. +// - If user specified pki in install-config, returns that config unchanged. +// - If ConfigurablePKI feature gate is enabled and pki is nil, returns a +// PKIConfig derived from DefaultPKIProfile().SignerCertificates. +// - If feature gate is disabled, returns nil (RSA-2048 legacy path). +func EffectiveSignerPKIConfig(ic *types.InstallConfig) *types.PKIConfig { + if ic.PKI != nil { + return ic.PKI + } + + if ic.Enabled(features.FeatureGateConfigurablePKI) { + profile := DefaultPKIProfile() + return &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithm(profile.SignerCertificates.Key.Algorithm), + RSA: types.RSAKeyConfig{KeySize: profile.SignerCertificates.Key.RSA.KeySize}, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurve(profile.SignerCertificates.Key.ECDSA.Curve)}, + }, + }, + } + } + + return nil +} diff --git a/pkg/types/pki/defaults_test.go b/pkg/types/pki/defaults_test.go new file mode 100644 index 00000000000..745d84c5b4b --- /dev/null +++ b/pkg/types/pki/defaults_test.go @@ -0,0 +1,104 @@ +package pki + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + configv1 "github.com/openshift/api/config/v1" + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/installer/pkg/types" +) + +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) +} + +func TestEffectiveSignerPKIConfig(t *testing.T) { + cases := []struct { + name string + ic *types.InstallConfig + expectNil bool + expectAlgo types.KeyAlgorithm + expectSize int32 + expectCurve types.ECDSACurve + }{ + { + name: "feature gate off, pki nil", + ic: &types.InstallConfig{ + FeatureSet: configv1.Default, + }, + expectNil: true, + }, + { + name: "feature gate on, pki nil - returns RSA-4096 from DefaultPKIProfile", + ic: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + }, + expectNil: false, + expectAlgo: types.KeyAlgorithmRSA, + expectSize: 4096, + }, + { + name: "feature gate on, pki specified - returns user config", + ic: &types.InstallConfig{ + FeatureSet: configv1.TechPreviewNoUpgrade, + PKI: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmECDSA, + ECDSA: types.ECDSAKeyConfig{Curve: types.ECDSACurveP384}, + }, + }, + }, + }, + expectNil: false, + }, + { + name: "feature gate off, pki specified - returns user config", + ic: &types.InstallConfig{ + FeatureSet: configv1.Default, + PKI: &types.PKIConfig{ + SignerCertificates: types.CertificateConfig{ + Key: types.KeyConfig{ + Algorithm: types.KeyAlgorithmRSA, + RSA: types.RSAKeyConfig{KeySize: 4096}, + }, + }, + }, + }, + expectNil: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result := EffectiveSignerPKIConfig(tc.ic) + + if tc.expectNil { + assert.Nil(t, result) + return + } + + assert.NotNil(t, result) + + if tc.ic.PKI != nil { + // Should return user's config unchanged + assert.Equal(t, tc.ic.PKI, result) + } else { + // Should return synthesized config from DefaultPKIProfile + assert.Equal(t, tc.expectAlgo, result.SignerCertificates.Key.Algorithm) + if tc.expectAlgo == types.KeyAlgorithmECDSA { + assert.Equal(t, tc.expectCurve, result.SignerCertificates.Key.ECDSA.Curve) + } else { + assert.Equal(t, tc.expectSize, result.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 cb0bd9500a278ef6ba966898a8cd2daede154611 Mon Sep 17 00:00:00 2001 From: Haseeb Tariq Date: Wed, 3 Jun 2026 12:16:38 -0700 Subject: [PATCH 2/3] 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") + }) +} From 0a3fbe65a4c268fe12b76d8f2980d455762f7d60 Mon Sep 17 00:00:00 2001 From: Haseeb Tariq Date: Wed, 3 Jun 2026 13:01:44 -0700 Subject: [PATCH 3/3] tls: wire signer certs to read PKI config via SignerKeyParams Introduce SignerKeyParams, a WritableAsset with zero dependencies that reads install-config.yaml directly from disk and extracts the effective PKI configuration via EffectiveSignerPKIConfig(). When no install-config is present, Load() returns (false, nil) so the asset store falls back to the state file between multi-step invocations (e.g. create manifests followed by create cluster, where install-config.yaml is consumed after the first step). In the agent flow where neither file nor state exists, Generate() leaves PKIConfig nil (RSA-2048). Signers that previously had zero dependencies now depend on SignerKeyParams instead of InstallConfig, avoiding validation that would reject configs valid in the agent flow. Signers that already depended on InstallConfig use EffectiveSignerPKIConfig() to resolve the effective PKI config including feature gate defaults. With TechPreview enabled and no explicit pki config, all signer certs are generated with RSA-4096 (the current DefaultPKIProfile default). Assisted-by: Claude Code (Opus 4.6) --- .../arbiter_ignition_customizations_test.go | 4 +- pkg/asset/ignition/machine/arbiter_test.go | 4 +- .../master_ignition_customizations_test.go | 4 +- pkg/asset/ignition/machine/master_test.go | 4 +- .../worker_ignition_customizations_test.go | 4 +- pkg/asset/ignition/machine/worker_test.go | 4 +- pkg/asset/tls/adminkubeconfig.go | 13 +-- pkg/asset/tls/aggregator.go | 5 +- pkg/asset/tls/apiserver.go | 42 +++++----- pkg/asset/tls/kubecontrolplane.go | 3 +- pkg/asset/tls/kubelet.go | 16 ++-- pkg/asset/tls/root.go | 13 +-- pkg/asset/tls/signerkey_params.go | 83 +++++++++++++++++++ 13 files changed, 152 insertions(+), 47 deletions(-) create mode 100644 pkg/asset/tls/signerkey_params.go diff --git a/pkg/asset/ignition/machine/arbiter_ignition_customizations_test.go b/pkg/asset/ignition/machine/arbiter_ignition_customizations_test.go index bf6b280849c..f54acdb0377 100644 --- a/pkg/asset/ignition/machine/arbiter_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/arbiter_ignition_customizations_test.go @@ -85,8 +85,10 @@ func TestArbiterIgnitionCustomizationsGenerate(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + rootCAParents := asset.Parents{} + rootCAParents.Add(&tls.SignerKeyParams{}) rootCA := &tls.RootCA{} - err := rootCA.Generate(context.Background(), nil) + err := rootCA.Generate(context.Background(), rootCAParents) assert.NoError(t, err, "unexpected error generating root CA") parents := asset.Parents{} diff --git a/pkg/asset/ignition/machine/arbiter_test.go b/pkg/asset/ignition/machine/arbiter_test.go index e69206905ec..06a75dce114 100644 --- a/pkg/asset/ignition/machine/arbiter_test.go +++ b/pkg/asset/ignition/machine/arbiter_test.go @@ -70,8 +70,10 @@ func TestArbiterGenerate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { + rootCAParents := asset.Parents{} + rootCAParents.Add(&tls.SignerKeyParams{}) rootCA := &tls.RootCA{} - err := rootCA.Generate(context.Background(), nil) + err := rootCA.Generate(context.Background(), rootCAParents) assert.NoError(t, err, "unexpected error generating root CA") parents := asset.Parents{} diff --git a/pkg/asset/ignition/machine/master_ignition_customizations_test.go b/pkg/asset/ignition/machine/master_ignition_customizations_test.go index 5a7b1deb90c..52dc12bb8fe 100644 --- a/pkg/asset/ignition/machine/master_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/master_ignition_customizations_test.go @@ -48,8 +48,10 @@ func TestMasterIgnitionCustomizationsGenerate(t *testing.T) { }, }) + rootCAParents := asset.Parents{} + rootCAParents.Add(&tls.SignerKeyParams{}) rootCA := &tls.RootCA{} - err := rootCA.Generate(context.Background(), nil) + err := rootCA.Generate(context.Background(), rootCAParents) assert.NoError(t, err, "unexpected error generating root CA") parents := asset.Parents{} diff --git a/pkg/asset/ignition/machine/master_test.go b/pkg/asset/ignition/machine/master_test.go index 23b801a1656..7672b19d39e 100644 --- a/pkg/asset/ignition/machine/master_test.go +++ b/pkg/asset/ignition/machine/master_test.go @@ -38,8 +38,10 @@ func TestMasterGenerate(t *testing.T) { }, }) + rootCAParents := asset.Parents{} + rootCAParents.Add(&tls.SignerKeyParams{}) rootCA := &tls.RootCA{} - err := rootCA.Generate(context.Background(), nil) + err := rootCA.Generate(context.Background(), rootCAParents) assert.NoError(t, err, "unexpected error generating root CA") parents := asset.Parents{} diff --git a/pkg/asset/ignition/machine/worker_ignition_customizations_test.go b/pkg/asset/ignition/machine/worker_ignition_customizations_test.go index 149fe520e26..c0230e14904 100644 --- a/pkg/asset/ignition/machine/worker_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/worker_ignition_customizations_test.go @@ -48,8 +48,10 @@ func TestWorkerIgnitionCustomizationsGenerate(t *testing.T) { }, }) + rootCAParents := asset.Parents{} + rootCAParents.Add(&tls.SignerKeyParams{}) rootCA := &tls.RootCA{} - err := rootCA.Generate(context.Background(), nil) + err := rootCA.Generate(context.Background(), rootCAParents) assert.NoError(t, err, "unexpected error generating root CA") parents := asset.Parents{} diff --git a/pkg/asset/ignition/machine/worker_test.go b/pkg/asset/ignition/machine/worker_test.go index 54cad763e3d..edd71b4c631 100644 --- a/pkg/asset/ignition/machine/worker_test.go +++ b/pkg/asset/ignition/machine/worker_test.go @@ -28,8 +28,10 @@ func TestWorkerGenerate(t *testing.T) { }, }) + rootCAParents := asset.Parents{} + rootCAParents.Add(&tls.SignerKeyParams{}) rootCA := &tls.RootCA{} - err := rootCA.Generate(context.Background(), nil) + err := rootCA.Generate(context.Background(), rootCAParents) assert.NoError(t, err, "unexpected error generating root CA") parents := asset.Parents{} diff --git a/pkg/asset/tls/adminkubeconfig.go b/pkg/asset/tls/adminkubeconfig.go index 8b2e33f9d11..85be095f89d 100644 --- a/pkg/asset/tls/adminkubeconfig.go +++ b/pkg/asset/tls/adminkubeconfig.go @@ -15,19 +15,20 @@ type AdminKubeConfigSignerCertKey struct { var _ asset.WritableAsset = (*AdminKubeConfigSignerCertKey)(nil) -// Dependencies returns no dependencies. Configurable PKI requires +// Dependencies returns SignerKeyParams. 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. +// node-joiner). SignerKeyParams reads the config directly from disk +// without triggering InstallConfig validation. func (c *AdminKubeConfigSignerCertKey) Dependencies() []asset.Asset { - return []asset.Asset{} + return []asset.Asset{&SignerKeyParams{}} } // Generate generates the root-ca key and cert pair. func (c *AdminKubeConfigSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { + signerKeyParams := &SignerKeyParams{} + parents.Get(signerKeyParams) cfg := &CertCfg{ Subject: pkix.Name{CommonName: "admin-kubeconfig-signer", OrganizationalUnit: []string{"openshift"}}, // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. @@ -35,7 +36,7 @@ func (c *AdminKubeConfigSignerCertKey) Generate(ctx context.Context, parents ass IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "admin-kubeconfig-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "admin-kubeconfig-signer", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. diff --git a/pkg/asset/tls/aggregator.go b/pkg/asset/tls/aggregator.go index 082f23d7266..e4a91b197e4 100644 --- a/pkg/asset/tls/aggregator.go +++ b/pkg/asset/tls/aggregator.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + pkidefaults "github.com/openshift/installer/pkg/types/pki" ) // AggregatorCA is the asset that generates the aggregator-ca key/cert pair. @@ -38,7 +39,7 @@ func (a *AggregatorCA) Generate(ctx context.Context, dependencies asset.Parents) IsCA: true, } - return a.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-ca", nil) + return a.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-ca", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. @@ -108,7 +109,7 @@ func (c *AggregatorSignerCertKey) Generate(ctx context.Context, parents asset.Pa IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "aggregator-signer", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/apiserver.go b/pkg/asset/tls/apiserver.go index 34659e7c663..59412e91ee7 100644 --- a/pkg/asset/tls/apiserver.go +++ b/pkg/asset/tls/apiserver.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + pkidefaults "github.com/openshift/installer/pkg/types/pki" ) // KubeAPIServerToKubeletSignerCertKey is a key/cert pair that signs the kube-apiserver to kubelet client certs. @@ -35,7 +36,7 @@ func (c *KubeAPIServerToKubeletSignerCertKey) Generate(ctx context.Context, pare IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-to-kubelet-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-to-kubelet-signer", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. @@ -116,19 +117,20 @@ type KubeAPIServerLocalhostSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerLocalhostSignerCertKey)(nil) -// Dependencies returns no dependencies. Configurable PKI requires +// Dependencies returns SignerKeyParams. 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. +// node-joiner). SignerKeyParams reads the config directly from disk +// without triggering InstallConfig validation. func (c *KubeAPIServerLocalhostSignerCertKey) Dependencies() []asset.Asset { - return []asset.Asset{} + return []asset.Asset{&SignerKeyParams{}} } // Generate generates the root-ca key and cert pair. func (c *KubeAPIServerLocalhostSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { + signerKeyParams := &SignerKeyParams{} + parents.Get(signerKeyParams) cfg := &CertCfg{ Subject: pkix.Name{CommonName: "kube-apiserver-localhost-signer", OrganizationalUnit: []string{"openshift"}}, // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. @@ -136,7 +138,7 @@ func (c *KubeAPIServerLocalhostSignerCertKey) Generate(ctx context.Context, pare IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-localhost-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-localhost-signer", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. @@ -226,19 +228,20 @@ type KubeAPIServerServiceNetworkSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerServiceNetworkSignerCertKey)(nil) -// Dependencies returns no dependencies. Configurable PKI requires +// Dependencies returns SignerKeyParams. 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. +// node-joiner). SignerKeyParams reads the config directly from disk +// without triggering InstallConfig validation. func (c *KubeAPIServerServiceNetworkSignerCertKey) Dependencies() []asset.Asset { - return []asset.Asset{} + return []asset.Asset{&SignerKeyParams{}} } // Generate generates the root-ca key and cert pair. func (c *KubeAPIServerServiceNetworkSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { + signerKeyParams := &SignerKeyParams{} + parents.Get(signerKeyParams) cfg := &CertCfg{ Subject: pkix.Name{CommonName: "kube-apiserver-service-network-signer", OrganizationalUnit: []string{"openshift"}}, // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. @@ -246,7 +249,7 @@ func (c *KubeAPIServerServiceNetworkSignerCertKey) Generate(ctx context.Context, IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-service-network-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-service-network-signer", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. @@ -345,19 +348,20 @@ type KubeAPIServerLBSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerLBSignerCertKey)(nil) -// Dependencies returns no dependencies. Configurable PKI requires +// Dependencies returns SignerKeyParams. 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. +// node-joiner). SignerKeyParams reads the config directly from disk +// without triggering InstallConfig validation. func (c *KubeAPIServerLBSignerCertKey) Dependencies() []asset.Asset { - return []asset.Asset{} + return []asset.Asset{&SignerKeyParams{}} } // Generate generates the root-ca key and cert pair. func (c *KubeAPIServerLBSignerCertKey) Generate(ctx context.Context, parents asset.Parents) error { + signerKeyParams := &SignerKeyParams{} + parents.Get(signerKeyParams) cfg := &CertCfg{ Subject: pkix.Name{CommonName: "kube-apiserver-lb-signer", OrganizationalUnit: []string{"openshift"}}, // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. @@ -365,7 +369,7 @@ func (c *KubeAPIServerLBSignerCertKey) Generate(ctx context.Context, parents ass IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-lb-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-apiserver-lb-signer", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. diff --git a/pkg/asset/tls/kubecontrolplane.go b/pkg/asset/tls/kubecontrolplane.go index 5834ffdb83d..d393774fb20 100644 --- a/pkg/asset/tls/kubecontrolplane.go +++ b/pkg/asset/tls/kubecontrolplane.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + pkidefaults "github.com/openshift/installer/pkg/types/pki" ) // KubeControlPlaneSignerCertKey is a key/cert pair that signs the kube control-plane client certs. @@ -32,7 +33,7 @@ func (c *KubeControlPlaneSignerCertKey) Generate(ctx context.Context, parents as IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-control-plane-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kube-control-plane-signer", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/kubelet.go b/pkg/asset/tls/kubelet.go index 2d585d4a1b8..a7cf404b379 100644 --- a/pkg/asset/tls/kubelet.go +++ b/pkg/asset/tls/kubelet.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + pkidefaults "github.com/openshift/installer/pkg/types/pki" ) // KubeletCSRSignerCertKey is a key/cert pair that signs the kubelet client certs. @@ -32,7 +33,7 @@ func (c *KubeletCSRSignerCertKey) Generate(ctx context.Context, parents asset.Pa IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-signer", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. @@ -108,19 +109,20 @@ type KubeletBootstrapCertSigner struct { var _ asset.WritableAsset = (*KubeletBootstrapCertSigner)(nil) -// Dependencies returns no dependencies. Configurable PKI requires +// Dependencies returns SignerKeyParams. 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. +// node-joiner). SignerKeyParams reads the config directly from disk +// without triggering InstallConfig validation. func (c *KubeletBootstrapCertSigner) Dependencies() []asset.Asset { - return []asset.Asset{} + return []asset.Asset{&SignerKeyParams{}} } // Generate generates the root-ca key and cert pair. func (c *KubeletBootstrapCertSigner) Generate(ctx context.Context, parents asset.Parents) error { + signerKeyParams := &SignerKeyParams{} + parents.Get(signerKeyParams) cfg := &CertCfg{ Subject: pkix.Name{CommonName: "kubelet-bootstrap-kubeconfig-signer", OrganizationalUnit: []string{"openshift"}}, // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. @@ -128,7 +130,7 @@ func (c *KubeletBootstrapCertSigner) Generate(ctx context.Context, parents asset IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-bootstrap-kubeconfig-signer", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "kubelet-bootstrap-kubeconfig-signer", signerKeyParams.PKIConfig) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/root.go b/pkg/asset/tls/root.go index 9ab310f68f8..4984d0ca814 100644 --- a/pkg/asset/tls/root.go +++ b/pkg/asset/tls/root.go @@ -21,19 +21,20 @@ type RootCA struct { var _ asset.WritableAsset = (*RootCA)(nil) -// Dependencies returns no dependencies. Configurable PKI requires +// Dependencies returns SignerKeyParams. 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. +// node-joiner). SignerKeyParams reads the config directly from disk +// without triggering InstallConfig validation. func (c *RootCA) Dependencies() []asset.Asset { - return []asset.Asset{} + return []asset.Asset{&SignerKeyParams{}} } // Generate generates the MCS/Ignition CA. func (c *RootCA) Generate(ctx context.Context, parents asset.Parents) error { + signerKeyParams := &SignerKeyParams{} + parents.Get(signerKeyParams) cfg := &CertCfg{ Subject: pkix.Name{CommonName: "root-ca", OrganizationalUnit: []string{"openshift"}}, // KeyUsages is set by GenerateSelfSignedCertificate based on the key algorithm. @@ -41,7 +42,7 @@ func (c *RootCA) Generate(ctx context.Context, parents asset.Parents) error { IsCA: true, } - return c.SelfSignedCertKey.Generate(ctx, cfg, "root-ca", nil) + return c.SelfSignedCertKey.Generate(ctx, cfg, "root-ca", signerKeyParams.PKIConfig) } // Name returns the human-friendly name of the asset. diff --git a/pkg/asset/tls/signerkey_params.go b/pkg/asset/tls/signerkey_params.go new file mode 100644 index 00000000000..c583f1caae0 --- /dev/null +++ b/pkg/asset/tls/signerkey_params.go @@ -0,0 +1,83 @@ +package tls + +import ( + "context" + + "sigs.k8s.io/yaml" + + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/types" + pkidefaults "github.com/openshift/installer/pkg/types/pki" +) + +// SignerKeyParams resolves the effective PKI configuration for signer +// certificates. It has no asset dependencies and reads install-config.yaml +// directly from disk so that signer certs can be generated without triggering +// standard InstallConfig validation. When no install-config is present (e.g. +// agent create certificates, node-joiner add-nodes), it defaults to nil +// PKIConfig which maps to RSA-2048. +type SignerKeyParams struct { + PKIConfig *types.PKIConfig +} + +var _ asset.WritableAsset = (*SignerKeyParams)(nil) + +// Name returns a human-friendly name for the asset. +func (*SignerKeyParams) Name() string { + return "Signer Key Parameters" +} + +// Dependencies returns no dependencies. See Load() for why this asset does +// not depend on the standard InstallConfig asset. +func (*SignerKeyParams) Dependencies() []asset.Asset { + return []asset.Asset{} +} + +// Generate is a no-op that leaves PKIConfig as nil (RSA-2048). This is the +// fallback when Load() finds no install-config on disk (e.g. agent flow). +func (s *SignerKeyParams) Generate(_ context.Context, _ asset.Parents) error { + return nil +} + +// Files returns nil — this asset has no on-disk representation. +func (*SignerKeyParams) Files() []*asset.File { + return nil +} + +// Load reads the install-config.yaml directly from disk and extracts the PKI +// configuration. Returns (true, nil) when the config is found and parsed, or +// (false, nil) when the file is missing or unparseable — never errors and +// never triggers interactive prompts. +// +// Returning (false, nil) when the file is missing allows the asset store to +// fall back to the state file between multi-step invocations (e.g. create +// manifests followed by create cluster, where install-config.yaml is consumed +// after the first step). When no state file exists either (e.g. agent flow), +// the asset store calls Generate() which leaves PKIConfig nil (RSA-2048). +// +// We parse the install-config directly rather than depending on the +// InstallConfig asset because store.load() recursively loads all dependencies +// from disk, and InstallConfig.Load() runs full platform validation via +// finish() -> ValidateInstallConfig(). In the agent flow, configs are validated +// by OptionalInstallConfig with agent-specific rules that are more lenient +// (e.g. vSphere without credentials is valid for agent installs). Adding +// InstallConfig as a dependency here would pull standard validation into the +// agent flow, rejecting configs that OptionalInstallConfig accepts. +// +// Since we only need the PKI and FeatureSet fields, we can safely unmarshal +// without validation. Any actual validation errors will be caught by the +// authoritative InstallConfig or OptionalInstallConfig asset elsewhere in the +// pipeline. +func (s *SignerKeyParams) Load(f asset.FileFetcher) (bool, error) { + // Matches installConfigFilename in pkg/asset/installconfig/installconfig.go. + file, err := f.FetchByName("install-config.yaml") + if err != nil { + return false, nil + } + config := &types.InstallConfig{} + if err := yaml.Unmarshal(file.Data, config); err != nil { + return false, nil + } + s.PKIConfig = pkidefaults.EffectiveSignerPKIConfig(config) + return true, nil +}