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/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/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/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/asset/tls/adminkubeconfig.go b/pkg/asset/tls/adminkubeconfig.go index f4ec8a7bf65..85be095f89d 100644 --- a/pkg/asset/tls/adminkubeconfig.go +++ b/pkg/asset/tls/adminkubeconfig.go @@ -15,21 +15,28 @@ type AdminKubeConfigSignerCertKey struct { var _ asset.WritableAsset = (*AdminKubeConfigSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// 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). 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: 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", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. diff --git a/pkg/asset/tls/aggregator.go b/pkg/asset/tls/aggregator.go index 6088b6785b1..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. @@ -32,13 +33,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", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. @@ -102,13 +103,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", 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 8eb94e9b871..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. @@ -29,13 +30,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", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. @@ -116,21 +117,28 @@ type KubeAPIServerLocalhostSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerLocalhostSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// 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). 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: 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", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. @@ -220,21 +228,28 @@ type KubeAPIServerServiceNetworkSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerServiceNetworkSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// 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). 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: 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", signerKeyParams.PKIConfig) } // Load reads the asset files from disk. @@ -333,21 +348,28 @@ type KubeAPIServerLBSignerCertKey struct { var _ asset.WritableAsset = (*KubeAPIServerLBSignerCertKey)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// 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). 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: 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", signerKeyParams.PKIConfig) } // 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..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. @@ -26,13 +27,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", 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 bfec9000020..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. @@ -26,13 +27,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", pkidefaults.EffectiveSignerPKIConfig(installConfig.Config)) } // Name returns the human-friendly name of the asset. @@ -108,21 +109,28 @@ type KubeletBootstrapCertSigner struct { var _ asset.WritableAsset = (*KubeletBootstrapCertSigner)(nil) -// Dependencies returns the dependency of the root-ca, which is empty. +// 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). 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: 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", 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 6529cdc7890..4984d0ca814 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,21 +21,28 @@ type RootCA struct { var _ asset.WritableAsset = (*RootCA)(nil) -// Dependencies returns nothing. +// 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). 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: 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", 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 +} 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") + }) +} 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 +}