From 2685d0d4c7533e527ff9f475083c9bc1b3800e17 Mon Sep 17 00:00:00 2001 From: Paul Maidment Date: Wed, 4 Sep 2024 13:48:56 +0300 Subject: [PATCH] MGMT-18684: WIP: Cryptographically verify CSRs of joining nodes. --- src/common/common.go | 60 +++++++++++++++++++++++++++ src/ignition/ignition.go | 34 ++++++++++++++++ src/ignition/mock_ignition.go | 14 +++++++ src/installer/installer.go | 68 +++++++++++++++++++++++++++++++ src/installer/installer_test.go | 8 ++++ src/k8s_client/k8s_client.go | 26 ++++++++++++ src/k8s_client/mock_k8s_client.go | 28 +++++++++++++ 7 files changed, 238 insertions(+) diff --git a/src/common/common.go b/src/common/common.go index c202ee1e22..7af3aecf2b 100644 --- a/src/common/common.go +++ b/src/common/common.go @@ -3,6 +3,10 @@ package common import ( "bytes" "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/x509" + "encoding/pem" "fmt" "io" "os" @@ -16,6 +20,8 @@ import ( "github.com/openshift/assisted-installer/src/utils" "github.com/openshift/assisted-service/models" + cryptorand "crypto/rand" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/thoas/go-funk" @@ -35,6 +41,8 @@ const ( installConfigMapAttribute = "invoker" InvokerAssisted = "assisted-service" InvokerAgent = "agent-installer" + // ECPrivateKeyBlockType is a possible value for pem.Block.Type. + ECPrivateKeyBlockType = "EC PRIVATE KEY" ) func GetHostsInStatus(hosts map[string]inventory_client.HostData, status []string, isMatch bool) map[string]inventory_client.HostData { @@ -306,3 +314,55 @@ func DownloadKubeconfigNoingress(ctx context.Context, dir string, ic inventory_c return kubeconfigPath, nil } + +func ParsePrivateKeyPEM(keyData []byte) (*ecdsa.PrivateKey, error) { + var privateKeyPemBlock *pem.Block + for { + privateKeyPemBlock, keyData = pem.Decode(keyData) + if privateKeyPemBlock == nil { + break + } + + switch privateKeyPemBlock.Type { + case ECPrivateKeyBlockType: + // ECDSA Private Key in ASN.1 format + if key, err := x509.ParseECPrivateKey(privateKeyPemBlock.Bytes); err == nil { + return key, nil + } + } + } + + // we read all the PEM blocks and didn't recognize one + return nil, fmt.Errorf("data does not contain a valid RSA or ECDSA private key") +} + +func GetPublicKeyBytesFromPrivateKeyPEM(keyData []byte) ([]byte, error) { + privateKey, err := ParsePrivateKeyPEM(keyData) + if err != nil { + return nil, errors.Wrap(err, "could not parse public key from private key PEM") + } + ecdhPubKey, err := privateKey.PublicKey.ECDH() + if err != nil { + return nil, errors.Wrap(err, "could not fetch public key from private key") + } + return ecdhPubKey.Bytes(), nil +} + +func MakeEllipticPrivatePublicKeyPems() ([]byte, []byte, error) { + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), cryptorand.Reader) + if err != nil { + return nil, nil, err + } + derBytes, err := x509.MarshalECPrivateKey(privateKey) + if err != nil { + return nil, nil, err + } + ecdhPubKey, err := privateKey.PublicKey.ECDH() + if err != nil { + return nil, nil, err + } + return pem.EncodeToMemory(&pem.Block{ + Type: ECPrivateKeyBlockType, + Bytes: derBytes, + }), ecdhPubKey.Bytes(), nil +} diff --git a/src/ignition/ignition.go b/src/ignition/ignition.go index 79f14a9321..4a0445cc5e 100644 --- a/src/ignition/ignition.go +++ b/src/ignition/ignition.go @@ -2,13 +2,17 @@ package ignition import ( "encoding/json" + "fmt" "os" + "encoding/base64" + ignitionConfigPrevVersion "github.com/coreos/ignition/v2/config/v3_1" ignitionConfig "github.com/coreos/ignition/v2/config/v3_2" "github.com/coreos/ignition/v2/config/v3_2/translate" "github.com/coreos/ignition/v2/config/v3_2/types" "github.com/coreos/ignition/v2/config/validate" + "github.com/go-openapi/swag" "github.com/pkg/errors" ) @@ -21,6 +25,7 @@ type Ignition interface { ParseIgnitionFile(path string) (*types.Config, error) WriteIgnitionFile(path string, config *types.Config) error MergeIgnitionConfig(base *types.Config, overrides *types.Config) (*types.Config, error) + InjectKubeletTempPrivateKey(pathToSourceIgnition string, privateKeyBytes []byte, certPathToInject string) error } type ignition struct{} @@ -78,3 +83,32 @@ func (i *ignition) MergeIgnitionConfig(base *types.Config, overrides *types.Conf } return &config, nil } + +func (i *ignition) InjectKubeletTempPrivateKey(pathToSourceIgnition string, privateKeyBytes []byte, certPathToInject string) error { + sourceIgnition, err := i.ParseIgnitionFile(pathToSourceIgnition) + if err != nil { + return errors.Wrapf(err, "unable to parse ignition file %s", pathToSourceIgnition) + } + privateKeyFile := types.File{ + Node: types.Node{ + Overwrite: swag.Bool(true), + Path: certPathToInject, + User: types.NodeUser{ + Name: swag.String("root"), + }, + }, + FileEmbedded1: types.FileEmbedded1{ + Contents: types.Resource{ + Source: swag.String( + fmt.Sprintf( + "data:text/plain;charset=utf-8;base64,%s", + base64.StdEncoding.EncodeToString([]byte(privateKeyBytes)), + ), + ), + }, + Mode: swag.Int(420), + }, + } + sourceIgnition.Storage.Files = append(sourceIgnition.Storage.Files, privateKeyFile) + return nil +} \ No newline at end of file diff --git a/src/ignition/mock_ignition.go b/src/ignition/mock_ignition.go index 67aa6b7fd6..849af091a2 100644 --- a/src/ignition/mock_ignition.go +++ b/src/ignition/mock_ignition.go @@ -38,6 +38,20 @@ func (m *MockIgnition) EXPECT() *MockIgnitionMockRecorder { return m.recorder } +// InjectKubeletTempPrivateKey mocks base method. +func (m *MockIgnition) InjectKubeletTempPrivateKey(pathToSourceIgnition string, privateKeyBytes []byte, certPathToInject string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InjectKubeletTempPrivateKey", pathToSourceIgnition, privateKeyBytes, certPathToInject) + ret0, _ := ret[0].(error) + return ret0 +} + +// InjectKubeletTempPrivateKey indicates an expected call of InjectKubeletTempPrivateKey. +func (mr *MockIgnitionMockRecorder) InjectKubeletTempPrivateKey(pathToSourceIgnition, privateKeyBytes, certPathToInject interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InjectKubeletTempPrivateKey", reflect.TypeOf((*MockIgnition)(nil).InjectKubeletTempPrivateKey), pathToSourceIgnition, privateKeyBytes, certPathToInject) +} + // MergeIgnitionConfig mocks base method. func (m *MockIgnition) MergeIgnitionConfig(base, overrides *types.Config) (*types.Config, error) { m.ctrl.T.Helper() diff --git a/src/installer/installer.go b/src/installer/installer.go index 1f23d7e836..e2002b0f76 100644 --- a/src/installer/installer.go +++ b/src/installer/installer.go @@ -34,6 +34,7 @@ import ( "github.com/openshift/assisted-installer/src/utils" "github.com/openshift/assisted-service/models" preinstallUtils "github.com/rh-ecosystem-edge/preinstall-utils/pkg" + ) // In dry run mode we prefer to get quick feedback about errors rather @@ -51,6 +52,7 @@ const ( singleNodeMasterIgnitionPath = "/opt/openshift/master.ign" waitingForMastersStatusInfo = "Waiting for masters to join bootstrap control plane" waitingForBootstrapToPrepare = "Waiting for bootstrap node preparation" + ECPrivateKeyBlockType = "EC PRIVATE KEY" ) var generalWaitTimeout = 30 * time.Second @@ -96,6 +98,19 @@ func (i *installer) FormatDisks() { } } +func (i *installer) StorePublicKeyInControlPlane(publicKey []byte, client k8s_client.K8SClient) error { + err := client.CreateNamespace("node-public-keys") + if err != nil { + i.log.Error("unable to create node-public-keys namespace on control plane", err) + } + err = client.CreateConfigMap(fmt.Sprintf("node-key-%s", i.HostID), "node-public-keys", map[string]string{"content": string(publicKey[:])}) + if err != nil { + i.log.WithError(err).Errorf("unable to create config map for host ID %s for on control plane", i.HostID) + } + return nil +} + + func (i *installer) InstallNode() error { i.log.Infof("Installing node with role: %s", i.Config.Role) @@ -141,6 +156,14 @@ func (i *installer) InstallNode() error { } + // privateKeyBytes, publicKeyBytes, err := ignition.MakePrivatePublicKeyPairForKubelet() + privateKeyBytes, publicKeyBytes, err := common.MakeEllipticPrivatePublicKeyPems() + if err != nil { + return errors.Wrap(err, "could not generate private/public key pair for temporary kubelet certificate") + } + ign := ignition.NewIgnition() + ign.InjectKubeletTempPrivateKey(ignitionPath, privateKeyBytes, "/var/lib/kubelet/pki/kubelet-client.key.tmp") + if err = i.writeImageToDisk(ignitionPath); err != nil { return err } @@ -150,6 +173,15 @@ func (i *installer) InstallNode() error { if err = i.workerWaitFor2ReadyMasters(ctx); err != nil { return err } + + kc, err := i.kcBuilder(KubeconfigPath, i.log) + if err != nil { + i.log.Error(err) + return err + } + if err := i.StorePublicKeyInControlPlane(publicKeyBytes, kc); err != nil { + return errors.Wrap(err, "unable to store public key for node in control plane") + } } if i.EnableSkipMcoReboot { @@ -171,6 +203,14 @@ func (i *installer) InstallNode() error { if err = i.waitForControlPlane(ctx); err != nil { return err } + kc, err := i.kcBuilder(KubeconfigPath, i.log) + if err != nil { + i.log.Error(err) + return err + } + if err := i.StorePublicKeyInControlPlane(publicKeyBytes, kc); err != nil { + return errors.Wrap(err, "unable to store public key for node in control plane") + } i.log.Info("Setting bootstrap node new role to master") } @@ -522,6 +562,21 @@ func (i *installer) waitForControlPlane(ctx context.Context) error { return err } + // // Build a namespace and config map to store some gathered information on hosts + // // So that this information may be used post reboot of the bootstrap to verify CSRs + // hostNames, err := i.getInventoryHostNames() + // if err != nil { + // i.log.Warnf("Failed to get hosts info from inventory, err %s", err) + // } + // err = kc.CreateNamespace("bootstrap-host-info") + // if err != nil { + // i.log.Error("unable to create bootstrap-host-info namespace on control plane", err) + // } + // err = kc.CreateConfigMap("known-hosts", "bootstrap-host-info", map[string]string{"content": strings.Join(hostNames, "\n")}) + // if err != nil { + // i.log.Error("unable to create known-hosts cm on control plane", err) + // } + // waiting for controller pod to be running if err = i.waitForController(kc); err != nil { i.log.Error(err) @@ -531,6 +586,19 @@ func (i *installer) waitForControlPlane(ctx context.Context) error { return nil } +// func (i *installer) getInventoryHostNames() ([]string, error) { +// var keys []string +// var inventoryHostsMap map[string]inventory_client.HostData +// inventoryHostsMap, err := i.getInventoryHostsMap(inventoryHostsMap) +// if err != nil { +// return nil, errors.Wrap(err, "failed to get hosts info from inventory") +// } +// for k := range inventoryHostsMap { +// keys = append(keys, k) +// } +// return keys, nil +// } + func (i *installer) waitForETCDBootstrap(ctx context.Context) error { i.UpdateHostInstallProgress(models.HostStageWaitingForBootkube, "waiting for ETCD bootstrap to be complete") i.log.Infof("Started waiting for ETCD bootstrap to complete") diff --git a/src/installer/installer_test.go b/src/installer/installer_test.go index a87e402db2..3212c696b8 100644 --- a/src/installer/installer_test.go +++ b/src/installer/installer_test.go @@ -310,6 +310,8 @@ var _ = Describe("installer HostRoleMaster role", func() { for _, version := range []string{"4.7", "4.7.1", "4.7-pre-release", "4.8"} { Context(version, func() { BeforeEach(func() { + mockk8sclient.EXPECT().CreateNamespace("node-public-keys").Times(1) + mockk8sclient.EXPECT().CreateConfigMap(gomock.Any(), "node-public-keys", gomock.Any()) conf.OpenshiftVersion = version }) AfterEach(func() { @@ -442,6 +444,8 @@ var _ = Describe("installer HostRoleMaster role", func() { Expect(ret).To(HaveOccurred()) }) It("bootstrap role extract ignition retry", func() { + mockk8sclient.EXPECT().CreateNamespace("node-public-keys").Times(1) + mockk8sclient.EXPECT().CreateConfigMap(gomock.Any(), "node-public-keys", gomock.Any()) updateProgressSuccess([][]string{{string(models.HostStageStartingInstallation), conf.Role}, {string(models.HostStageWaitingForControlPlane), waitingForBootstrapToPrepare}, {string(models.HostStageWaitingForControlPlane), waitingForMastersStatusInfo}, @@ -554,6 +558,8 @@ var _ = Describe("installer HostRoleMaster role", func() { }) It(fmt.Sprintf("for platform type %v is expected to remove uninitialized taint = %v", platformType, expectedRemoveUninitializedTaint), func() { + mockk8sclient.EXPECT().CreateNamespace("node-public-keys").Times(1) + mockk8sclient.EXPECT().CreateConfigMap(gomock.Any(), "node-public-keys", gomock.Any()) updateProgressSuccess([][]string{{string(models.HostStageStartingInstallation), conf.Role}, {string(models.HostStageWaitingForControlPlane), waitingForBootstrapToPrepare}, {string(models.HostStageWaitingForControlPlane), waitingForMastersStatusInfo}, @@ -894,6 +900,8 @@ var _ = Describe("installer HostRoleMaster role", func() { evaluateDiskSymlinkSuccess() }) It("worker role happy flow", func() { + mockk8sclient.EXPECT().CreateNamespace("node-public-keys").Times(1) + mockk8sclient.EXPECT().CreateConfigMap(gomock.Any(), "node-public-keys", gomock.Any()) updateProgressSuccess([][]string{{string(models.HostStageStartingInstallation), conf.Role}, {string(models.HostStageInstalling), conf.Role}, {string(models.HostStageWritingImageToDisk)}, diff --git a/src/k8s_client/k8s_client.go b/src/k8s_client/k8s_client.go index 79dd447665..7c54a388d7 100644 --- a/src/k8s_client/k8s_client.go +++ b/src/k8s_client/k8s_client.go @@ -88,6 +88,8 @@ type K8SClient interface { IsClusterCapabilityEnabled(configv1.ClusterVersionCapability) (bool, error) UntaintNode(name string) error PatchMachineConfigPoolPaused(pause bool, mcpName string) error + CreateNamespace(name string) error + CreateConfigMap(name string, namespace string, data map[string]string) error } type K8SClientBuilder func(configPath string, logger logrus.FieldLogger) (K8SClient, error) @@ -691,3 +693,27 @@ func (c *k8sClient) PatchMachineConfigPoolPaused(pause bool, mcpName string) err c.log.Infof("Setting pause MCP %s to %t", mcpName, pause) return c.runtimeClient.Patch(context.TODO(), mcp, runtimeclient.RawPatch(types.MergePatchType, pausePatch)) } + +func (c *k8sClient) CreateNamespace(name string) error { + if _, err := c.client.CoreV1().Namespaces().Create(context.TODO(), &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + }, metav1.CreateOptions{}); err != nil { + return errors.Wrap(err, "unable to create namespace for known hosts") + } + return nil +} + +func (c *k8sClient) CreateConfigMap(name string, namespace string, data map[string]string) error { + if _, err := c.client.CoreV1().ConfigMaps(namespace).Create(context.TODO(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: data, + }, metav1.CreateOptions{}); err != nil { + return errors.Wrap(err, "unable to create config map for known hosts") + } + return nil +} diff --git a/src/k8s_client/mock_k8s_client.go b/src/k8s_client/mock_k8s_client.go index 45d0d1c9a1..cebeec2841 100644 --- a/src/k8s_client/mock_k8s_client.go +++ b/src/k8s_client/mock_k8s_client.go @@ -61,6 +61,20 @@ func (mr *MockK8SClientMockRecorder) ApproveCsr(csr any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApproveCsr", reflect.TypeOf((*MockK8SClient)(nil).ApproveCsr), csr) } +// CreateConfigMap mocks base method. +func (m *MockK8SClient) CreateConfigMap(name, namespace string, data map[string]string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateConfigMap", name, namespace, data) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateConfigMap indicates an expected call of CreateConfigMap. +func (mr *MockK8SClientMockRecorder) CreateConfigMap(name, namespace, data interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateConfigMap", reflect.TypeOf((*MockK8SClient)(nil).CreateConfigMap), name, namespace, data) +} + // CreateEvent mocks base method. func (m *MockK8SClient) CreateEvent(namespace, name, message, component string) (*v12.Event, error) { m.ctrl.T.Helper() @@ -76,6 +90,20 @@ func (mr *MockK8SClientMockRecorder) CreateEvent(namespace, name, message, compo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateEvent", reflect.TypeOf((*MockK8SClient)(nil).CreateEvent), namespace, name, message, component) } +// CreateNamespace mocks base method. +func (m *MockK8SClient) CreateNamespace(name string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateNamespace", name) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateNamespace indicates an expected call of CreateNamespace. +func (mr *MockK8SClientMockRecorder) CreateNamespace(name interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNamespace", reflect.TypeOf((*MockK8SClient)(nil).CreateNamespace), name) +} + // DeleteInstallPlan mocks base method. func (m *MockK8SClient) DeleteInstallPlan(installPlan types.NamespacedName) error { m.ctrl.T.Helper()