From 5cbd0d25961b0e02ebbab7cadc311ef114dfe974 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Tue, 15 Oct 2024 14:10:46 -0500 Subject: [PATCH 01/17] add algorithm --- master/internal/api_shell.go | 2 +- master/internal/api_user_intg_test.go | 2 +- master/internal/config/config.go | 18 +++- master/internal/core.go | 2 +- master/internal/core_intg_test.go | 2 +- master/internal/experiment.go | 2 +- master/internal/trial_intg_test.go | 1 - master/pkg/ssh/ssh.go | 120 +++++++++++++++++++++++--- master/pkg/tasks/task.go | 3 +- 9 files changed, 131 insertions(+), 21 deletions(-) diff --git a/master/internal/api_shell.go b/master/internal/api_shell.go index dbadba57147..ccd13e3035a 100644 --- a/master/internal/api_shell.go +++ b/master/internal/api_shell.go @@ -266,7 +266,7 @@ func (a *apiServer) LaunchShell( } } - keys, err := ssh.GenerateKey(launchReq.Spec.Base.SSHRsaSize, passphrase) + keys, err := ssh.GenerateKey(launchReq.Spec.Base.SSHConfig, passphrase) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/master/internal/api_user_intg_test.go b/master/internal/api_user_intg_test.go index c0f1d673abf..812d2394e6a 100644 --- a/master/internal/api_user_intg_test.go +++ b/master/internal/api_user_intg_test.go @@ -118,7 +118,7 @@ func setupAPITest(t *testing.T, pgdb *db.PgDB, TaskContainerDefaults: model.TaskContainerDefaultsConfig{}, ResourceConfig: *config.DefaultResourceConfig(), }, - taskSpec: &tasks.TaskSpec{SSHRsaSize: 1024}, + taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, CryptoSystem: "RSA"}}, allRms: map[string]rm.ResourceManager{config.DefaultClusterName: mockRM}, }, } diff --git a/master/internal/config/config.go b/master/internal/config/config.go index 306cc1b27bf..f99192ac83b 100644 --- a/master/internal/config/config.go +++ b/master/internal/config/config.go @@ -42,6 +42,15 @@ const ( preemptionScheduler = "preemption" ) +const ( + // RSACryptoSystem uses RSA. + RSACryptoSystem = "RSA" + // ECDSACryptoSystem uses ECDSA. + ECDSACryptoSystem = "ECDSA" + // ED25519CryptoSystem uses ED25519. + ED25519CryptoSystem = "ED25519" +) + type ( // ExperimentConfigPatch is the updatedble fields for patching an experiment. ExperimentConfigPatch struct { @@ -108,7 +117,8 @@ func DefaultConfig() *Config { Group: "root", }, SSH: SSHConfig{ - RsaKeySize: 1024, + RsaKeySize: 1024, + CryptoSystem: RSACryptoSystem, }, AuthZ: *DefaultAuthZConfig(), }, @@ -452,7 +462,8 @@ type SecurityConfig struct { // SSHConfig is the configuration setting for SSH. type SSHConfig struct { - RsaKeySize int `json:"rsa_key_size"` + RsaKeySize int `json:"rsa_key_size"` + CryptoSystem string `json:"crypto_system"` } // TLSConfig is the configuration for setting up serving over TLS. @@ -475,6 +486,9 @@ func (t *TLSConfig) Validate() []error { // Validate implements the check.Validatable interface. func (t *SSHConfig) Validate() []error { var errs []error + if t.CryptoSystem != RSACryptoSystem && t.CryptoSystem != ECDSACryptoSystem && t.CryptoSystem != ED25519CryptoSystem { + errs = append(errs, errors.New("Crypto system must be one of 'RSA', 'ECDSA' or 'ED25519'")) + } if t.RsaKeySize < 1 { errs = append(errs, errors.New("RSA Key size must be greater than 0")) } else if t.RsaKeySize > 16384 { diff --git a/master/internal/core.go b/master/internal/core.go index 3b0bd270197..6640b8332d6 100644 --- a/master/internal/core.go +++ b/master/internal/core.go @@ -1242,7 +1242,7 @@ func (m *Master) Run(ctx context.Context, gRPCLogInitDone chan struct{}) error { HarnessPath: filepath.Join(m.config.Root, "wheels"), TaskContainerDefaults: m.config.TaskContainerDefaults, MasterCert: config.GetCertPEM(cert), - SSHRsaSize: m.config.Security.SSH.RsaKeySize, + SSHConfig: m.config.Security.SSH, SegmentEnabled: m.config.Telemetry.Enabled && m.config.Telemetry.SegmentMasterKey != "", SegmentAPIKey: m.config.Telemetry.SegmentMasterKey, LogRetentionDays: m.config.RetentionPolicy.LogRetentionDays, diff --git a/master/internal/core_intg_test.go b/master/internal/core_intg_test.go index d8386407c0f..239ff047b57 100644 --- a/master/internal/core_intg_test.go +++ b/master/internal/core_intg_test.go @@ -121,7 +121,7 @@ func TestRun(t *testing.T) { DefaultLoggingConfig: &model.DefaultLoggingConfig{}, }, }, - taskSpec: &tasks.TaskSpec{SSHRsaSize: 1024}, + taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, CryptoSystem: "RSA"}}, } require.NoError(t, m.config.Resolve()) m.config.DB = config.DBConfig{ diff --git a/master/internal/experiment.go b/master/internal/experiment.go index c980ccdb1e5..ff0429f1e66 100644 --- a/master/internal/experiment.go +++ b/master/internal/experiment.go @@ -158,7 +158,7 @@ func newExperiment( taskSpec.AgentUserGroup = agentUserGroup - generatedKeys, err := ssh.GenerateKey(taskSpec.SSHRsaSize, nil) + generatedKeys, err := ssh.GenerateKey(taskSpec.SSHConfig, nil) if err != nil { return nil, nil, errors.Wrap(err, "generating ssh keys for trials") } diff --git a/master/internal/trial_intg_test.go b/master/internal/trial_intg_test.go index 59b5b079749..5681a1065ab 100644 --- a/master/internal/trial_intg_test.go +++ b/master/internal/trial_intg_test.go @@ -172,7 +172,6 @@ func setup(t *testing.T) ( &model.Checkpoint{}, &tasks.TaskSpec{ AgentUserGroup: &model.AgentUserGroup{}, - SSHRsaSize: 1024, Workspace: model.DefaultWorkspaceName, }, ssh.PrivateAndPublicKeys{}, diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index 026a87523db..c3aa2c4b1ed 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -1,6 +1,9 @@ package ssh import ( + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -8,10 +11,14 @@ import ( "github.com/pkg/errors" sshlib "golang.org/x/crypto/ssh" + + "github.com/determined-ai/determined/master/internal/config" ) const ( - trialPEMBlockType = "RSA PRIVATE KEY" + rsaPEMBlockType = "RSA PRIVATE KEY" + ecdsaPEMBlockType = "ECDSA PRIVATE KEY" + ed25519PEMBlockType = "ED25519 PRIVATE KEY" ) // PrivateAndPublicKeys contains a private and public key. @@ -21,11 +28,25 @@ type PrivateAndPublicKeys struct { } // GenerateKey returns a private and public SSH key. -func GenerateKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, error) { +func GenerateKey(conf config.SSHConfig, passphrase *string) (PrivateAndPublicKeys, error) { + var generatedKeys PrivateAndPublicKeys + switch conf.CryptoSystem { + case config.RSACryptoSystem: + return generateRSAKey(conf.RsaKeySize, passphrase) + case config.ECDSACryptoSystem: + return generateECDSAKey(passphrase) + case config.ED25519CryptoSystem: + return generateED25519Key(passphrase) + default: + return generatedKeys, errors.New("Invalid crypto system") + } +} + +func generateRSAKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys privateKey, err := rsa.GenerateKey(rand.Reader, rsaKeySize) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate private key") + return generatedKeys, errors.Wrap(err, "unable to generate RSA private key") } if err = privateKey.Validate(); err != nil { @@ -33,22 +54,88 @@ func GenerateKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, erro } block := &pem.Block{ - Type: trialPEMBlockType, + Type: rsaPEMBlockType, Bytes: x509.MarshalPKCS1PrivateKey(privateKey), } - if passphrase != nil { - // TODO: Replace usage of deprecated x509.EncryptPEMBlock. - block, err = x509.EncryptPEMBlock( //nolint: staticcheck - rand.Reader, block.Type, block.Bytes, []byte(*passphrase), x509.PEMCipherAES256) - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to encrypt private key") - } + block, err = encodePassphrase(block, passphrase) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to encrypt RSA private key") + } + + publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to generate RSA public key") + } + + generatedKeys = PrivateAndPublicKeys{ + PrivateKey: pem.EncodeToMemory(block), + PublicKey: sshlib.MarshalAuthorizedKey(publicKey), + } + + return generatedKeys, nil +} + +func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { + var generatedKeys PrivateAndPublicKeys + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to generate ECDS private key") + } + + privateKeyBytes, err := x509.MarshalECPrivateKey(privateKey) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to marshal ECDS private key") + } + + block := &pem.Block{ + Type: ecdsaPEMBlockType, + Bytes: privateKeyBytes, + } + + block, err = encodePassphrase(block, passphrase) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to encrypt ECDS private key") } publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate public key") + return generatedKeys, errors.Wrap(err, "unable to generate ECDS public key") + } + + generatedKeys = PrivateAndPublicKeys{ + PrivateKey: pem.EncodeToMemory(block), + PublicKey: sshlib.MarshalAuthorizedKey(publicKey), + } + + return generatedKeys, nil +} + +func generateED25519Key(passphrase *string) (PrivateAndPublicKeys, error) { + var generatedKeys PrivateAndPublicKeys + + ed25519PublicKey, privateKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to generate ED25519 private key") + } + privateKeyBytes, err := x509.MarshalPKCS8PrivateKey(privateKey) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") + } + + block := &pem.Block{ + Type: ed25519PEMBlockType, + Bytes: privateKeyBytes, + } + + block, err = encodePassphrase(block, passphrase) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to encrypt ED25519 private key") + } + + publicKey, err := sshlib.NewPublicKey(ed25519PublicKey) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to generate ECDS public key") } generatedKeys = PrivateAndPublicKeys{ @@ -58,3 +145,12 @@ func GenerateKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, erro return generatedKeys, nil } + +func encodePassphrase(block *pem.Block, passphrase *string) (*pem.Block, error) { + if passphrase != nil { + // TODO: Replace usage of deprecated x509.EncryptPEMBlock. + return x509.EncryptPEMBlock( //nolint: staticcheck + rand.Reader, block.Type, block.Bytes, []byte(*passphrase), x509.PEMCipherAES256) + } + return block, nil +} diff --git a/master/pkg/tasks/task.go b/master/pkg/tasks/task.go index 12b000c2de3..bc563197d66 100644 --- a/master/pkg/tasks/task.go +++ b/master/pkg/tasks/task.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/types/mount" "github.com/jinzhu/copier" + "github.com/determined-ai/determined/master/internal/config" "github.com/determined-ai/determined/master/pkg/archive" "github.com/determined-ai/determined/master/pkg/cproto" "github.com/determined-ai/determined/master/pkg/device" @@ -70,7 +71,7 @@ type TaskSpec struct { ClusterID string HarnessPath string MasterCert []byte - SSHRsaSize int + SSHConfig config.SSHConfig SegmentEnabled bool SegmentAPIKey string From 2a05eb1d1173e582dc620ab952e9d7e8306eb5e8 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Wed, 16 Oct 2024 15:50:14 -0500 Subject: [PATCH 02/17] fix header and ED25519 --- master/pkg/ssh/ssh.go | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index c3aa2c4b1ed..278f26abe17 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -16,9 +16,8 @@ import ( ) const ( - rsaPEMBlockType = "RSA PRIVATE KEY" - ecdsaPEMBlockType = "ECDSA PRIVATE KEY" - ed25519PEMBlockType = "ED25519 PRIVATE KEY" + rsaPEMBlockType = "RSA PRIVATE KEY" + ecdsaPEMBlockType = "EC PRIVATE KEY" ) // PrivateAndPublicKeys contains a private and public key. @@ -80,12 +79,12 @@ func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ECDS private key") + return generatedKeys, errors.Wrap(err, "unable to generate ECDSA private key") } privateKeyBytes, err := x509.MarshalECPrivateKey(privateKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to marshal ECDS private key") + return generatedKeys, errors.Wrap(err, "unable to marshal ECDSA private key") } block := &pem.Block{ @@ -95,12 +94,12 @@ func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { block, err = encodePassphrase(block, passphrase) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to encrypt ECDS private key") + return generatedKeys, errors.Wrap(err, "unable to encrypt ECDSA private key") } publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ECDS public key") + return generatedKeys, errors.Wrap(err, "unable to generate ECDSA public key") } generatedKeys = PrivateAndPublicKeys{ @@ -114,28 +113,28 @@ func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { func generateED25519Key(passphrase *string) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys - ed25519PublicKey, privateKey, err := ed25519.GenerateKey(rand.Reader) + ed25519PublicKey, privateKey, err := ed25519.GenerateKey(nil) if err != nil { return generatedKeys, errors.Wrap(err, "unable to generate ED25519 private key") } - privateKeyBytes, err := x509.MarshalPKCS8PrivateKey(privateKey) - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") - } - block := &pem.Block{ - Type: ed25519PEMBlockType, - Bytes: privateKeyBytes, - } - - block, err = encodePassphrase(block, passphrase) - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to encrypt ED25519 private key") + // Before OpenSSH 9.6, for ED25519 keys, only the OpenSSH private key format was supported. + var block *pem.Block + if passphrase != nil { + block, err = sshlib.MarshalPrivateKeyWithPassphrase(privateKey, "", []byte(*passphrase)) + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") + } + } else { + block, err = sshlib.MarshalPrivateKey(privateKey, "") + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") + } } publicKey, err := sshlib.NewPublicKey(ed25519PublicKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ECDS public key") + return generatedKeys, errors.Wrap(err, "unable to generate ED25519 public key") } generatedKeys = PrivateAndPublicKeys{ From b46f97f3341c6e336f44dabc1d7c0546c1b61187 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Wed, 16 Oct 2024 17:36:56 -0500 Subject: [PATCH 03/17] add release note --- docs/release-notes/ssh-crypto-system.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/release-notes/ssh-crypto-system.rst diff --git a/docs/release-notes/ssh-crypto-system.rst b/docs/release-notes/ssh-crypto-system.rst new file mode 100644 index 00000000000..c6cc2659ec2 --- /dev/null +++ b/docs/release-notes/ssh-crypto-system.rst @@ -0,0 +1,5 @@ +:orphan: + +**Improvements** + +- Master Configuration: Add support for crypto system configuration for ssh connection. ``security -> crypto_system`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. \ No newline at end of file From ae88b020d955c32567e35868e2a4389b074943aa Mon Sep 17 00:00:00 2001 From: gt2345 Date: Thu, 17 Oct 2024 10:08:05 -0500 Subject: [PATCH 04/17] remove passphrase when launching shell --- harness/determined/cli/shell.py | 11 ----- master/internal/api_shell.go | 16 +------- master/internal/experiment.go | 2 +- master/pkg/ssh/ssh.go | 47 +++++----------------- proto/pkg/apiv1/shell.pb.go | 2 +- proto/src/determined/api/v1/shell.proto | 2 +- webui/react/src/services/api-ts-sdk/api.ts | 2 +- 7 files changed, 15 insertions(+), 67 deletions(-) diff --git a/harness/determined/cli/shell.py b/harness/determined/cli/shell.py index e9af72da874..cc3e211dff0 100644 --- a/harness/determined/cli/shell.py +++ b/harness/determined/cli/shell.py @@ -1,7 +1,6 @@ import argparse import contextlib import functools -import getpass import os import pathlib import platform @@ -22,9 +21,6 @@ def start_shell(args: argparse.Namespace) -> None: sess = cli.setup_session(args) - data = {} - if args.passphrase: - data["passphrase"] = getpass.getpass("Enter new passphrase: ") config = ntsc.parse_config(args.config_file, None, args.config, args.volume) workspace_id = cli.workspace.get_workspace_id_from_args(args) @@ -35,7 +31,6 @@ def start_shell(args: argparse.Namespace) -> None: args.template, context_path=args.context, includes=args.include, - data=data, workspace_id=workspace_id, ) shell = bindings.v1LaunchShellResponse.from_json(resp).shell @@ -280,12 +275,6 @@ def _open_shell( help=ntsc.INCLUDE_DESC, ), cli.Arg("--config", action="append", default=[], help=ntsc.CONFIG_DESC), - cli.Arg( - "-p", - "--passphrase", - action="store_true", - help="passphrase to encrypt the shell private key", - ), cli.Arg( "--template", type=str, diff --git a/master/internal/api_shell.go b/master/internal/api_shell.go index ccd13e3035a..4e744f1974c 100644 --- a/master/internal/api_shell.go +++ b/master/internal/api_shell.go @@ -3,7 +3,6 @@ package internal import ( "archive/tar" "context" - "encoding/json" "fmt" "strconv" @@ -253,20 +252,7 @@ func (a *apiServer) LaunchShell( } maps.Copy(launchReq.Spec.Base.ExtraEnvVars, oidcPachydermEnvVars) - var passphrase *string - if len(req.Data) > 0 { - var data map[string]interface{} - if err = json.Unmarshal(req.Data, &data); err != nil { - return nil, status.Errorf(codes.Internal, "failed to parse data %s: %s", req.Data, err) - } - if pwd, ok := data["passphrase"]; ok { - if typed, typedOK := pwd.(string); typedOK { - passphrase = &typed - } - } - } - - keys, err := ssh.GenerateKey(launchReq.Spec.Base.SSHConfig, passphrase) + keys, err := ssh.GenerateKey(launchReq.Spec.Base.SSHConfig) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/master/internal/experiment.go b/master/internal/experiment.go index ff0429f1e66..8d2d51c79d0 100644 --- a/master/internal/experiment.go +++ b/master/internal/experiment.go @@ -158,7 +158,7 @@ func newExperiment( taskSpec.AgentUserGroup = agentUserGroup - generatedKeys, err := ssh.GenerateKey(taskSpec.SSHConfig, nil) + generatedKeys, err := ssh.GenerateKey(taskSpec.SSHConfig) if err != nil { return nil, nil, errors.Wrap(err, "generating ssh keys for trials") } diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index 278f26abe17..db03fb4b600 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -27,21 +27,21 @@ type PrivateAndPublicKeys struct { } // GenerateKey returns a private and public SSH key. -func GenerateKey(conf config.SSHConfig, passphrase *string) (PrivateAndPublicKeys, error) { +func GenerateKey(conf config.SSHConfig) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys switch conf.CryptoSystem { case config.RSACryptoSystem: - return generateRSAKey(conf.RsaKeySize, passphrase) + return generateRSAKey(conf.RsaKeySize) case config.ECDSACryptoSystem: - return generateECDSAKey(passphrase) + return generateECDSAKey() case config.ED25519CryptoSystem: - return generateED25519Key(passphrase) + return generateED25519Key() default: return generatedKeys, errors.New("Invalid crypto system") } } -func generateRSAKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, error) { +func generateRSAKey(rsaKeySize int) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys privateKey, err := rsa.GenerateKey(rand.Reader, rsaKeySize) if err != nil { @@ -57,11 +57,6 @@ func generateRSAKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, e Bytes: x509.MarshalPKCS1PrivateKey(privateKey), } - block, err = encodePassphrase(block, passphrase) - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to encrypt RSA private key") - } - publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) if err != nil { return generatedKeys, errors.Wrap(err, "unable to generate RSA public key") @@ -75,7 +70,7 @@ func generateRSAKey(rsaKeySize int, passphrase *string) (PrivateAndPublicKeys, e return generatedKeys, nil } -func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { +func generateECDSAKey() (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { @@ -92,11 +87,6 @@ func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { Bytes: privateKeyBytes, } - block, err = encodePassphrase(block, passphrase) - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to encrypt ECDSA private key") - } - publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) if err != nil { return generatedKeys, errors.Wrap(err, "unable to generate ECDSA public key") @@ -110,7 +100,7 @@ func generateECDSAKey(passphrase *string) (PrivateAndPublicKeys, error) { return generatedKeys, nil } -func generateED25519Key(passphrase *string) (PrivateAndPublicKeys, error) { +func generateED25519Key() (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys ed25519PublicKey, privateKey, err := ed25519.GenerateKey(nil) @@ -119,17 +109,9 @@ func generateED25519Key(passphrase *string) (PrivateAndPublicKeys, error) { } // Before OpenSSH 9.6, for ED25519 keys, only the OpenSSH private key format was supported. - var block *pem.Block - if passphrase != nil { - block, err = sshlib.MarshalPrivateKeyWithPassphrase(privateKey, "", []byte(*passphrase)) - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") - } - } else { - block, err = sshlib.MarshalPrivateKey(privateKey, "") - if err != nil { - return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") - } + block, err := sshlib.MarshalPrivateKey(privateKey, "") + if err != nil { + return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") } publicKey, err := sshlib.NewPublicKey(ed25519PublicKey) @@ -144,12 +126,3 @@ func generateED25519Key(passphrase *string) (PrivateAndPublicKeys, error) { return generatedKeys, nil } - -func encodePassphrase(block *pem.Block, passphrase *string) (*pem.Block, error) { - if passphrase != nil { - // TODO: Replace usage of deprecated x509.EncryptPEMBlock. - return x509.EncryptPEMBlock( //nolint: staticcheck - rand.Reader, block.Type, block.Bytes, []byte(*passphrase), x509.PEMCipherAES256) - } - return block, nil -} diff --git a/proto/pkg/apiv1/shell.pb.go b/proto/pkg/apiv1/shell.pb.go index 8138e6f888b..e5d9c40b5d7 100644 --- a/proto/pkg/apiv1/shell.pb.go +++ b/proto/pkg/apiv1/shell.pb.go @@ -570,7 +570,7 @@ type LaunchShellRequest struct { TemplateName string `protobuf:"bytes,2,opt,name=template_name,json=templateName,proto3" json:"template_name,omitempty"` // The files to run with the command. Files []*utilv1.File `protobuf:"bytes,3,rep,name=files,proto3" json:"files,omitempty"` - // Additional data. + // Deprecated: Do not use. Data []byte `protobuf:"bytes,4,opt,name=data,proto3" json:"data,omitempty"` // Workspace ID. Defaults to 'Uncategorized' workspace if not specified. WorkspaceId int32 `protobuf:"varint,5,opt,name=workspace_id,json=workspaceId,proto3" json:"workspace_id,omitempty"` diff --git a/proto/src/determined/api/v1/shell.proto b/proto/src/determined/api/v1/shell.proto index b3a2c1fe7af..76504a078bf 100644 --- a/proto/src/determined/api/v1/shell.proto +++ b/proto/src/determined/api/v1/shell.proto @@ -102,7 +102,7 @@ message LaunchShellRequest { string template_name = 2; // The files to run with the command. repeated determined.util.v1.File files = 3; - // Additional data. + // Deprecated: Do not use. bytes data = 4; // Workspace ID. Defaults to 'Uncategorized' workspace if not specified. int32 workspace_id = 5; diff --git a/webui/react/src/services/api-ts-sdk/api.ts b/webui/react/src/services/api-ts-sdk/api.ts index 2329eb9787d..a92ff0b34b5 100644 --- a/webui/react/src/services/api-ts-sdk/api.ts +++ b/webui/react/src/services/api-ts-sdk/api.ts @@ -6154,7 +6154,7 @@ export interface V1LaunchShellRequest { */ files?: Array; /** - * Additional data. + * Deprecated: Do not use. * @type {string} * @memberof V1LaunchShellRequest */ From e444bc4f16f626718d1c816ce5b73b3ba7b1a161 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Thu, 17 Oct 2024 10:54:52 -0500 Subject: [PATCH 05/17] docs fmt --- docs/release-notes/ssh-crypto-system.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/ssh-crypto-system.rst b/docs/release-notes/ssh-crypto-system.rst index c6cc2659ec2..9f878728103 100644 --- a/docs/release-notes/ssh-crypto-system.rst +++ b/docs/release-notes/ssh-crypto-system.rst @@ -2,4 +2,5 @@ **Improvements** -- Master Configuration: Add support for crypto system configuration for ssh connection. ``security -> crypto_system`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. \ No newline at end of file +- Master Configuration: Add support for crypto system configuration for ssh connection. ``security + -> crypto_system`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. From 5061abe03d233b539fc9099c323f1427fc757ce1 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Thu, 17 Oct 2024 12:48:41 -0500 Subject: [PATCH 06/17] update docs --- docs/release-notes/ssh-crypto-system.rst | 4 ++-- master/pkg/ssh/ssh.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/ssh-crypto-system.rst b/docs/release-notes/ssh-crypto-system.rst index 9f878728103..75268915006 100644 --- a/docs/release-notes/ssh-crypto-system.rst +++ b/docs/release-notes/ssh-crypto-system.rst @@ -2,5 +2,5 @@ **Improvements** -- Master Configuration: Add support for crypto system configuration for ssh connection. ``security - -> crypto_system`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. +- Master Configuration: Add support for crypto system configuration for ssh connection. + ``security.crypto_system`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index db03fb4b600..6508d6c43ce 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -72,6 +72,7 @@ func generateRSAKey(rsaKeySize int) (PrivateAndPublicKeys, error) { func generateECDSAKey() (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys + // Curve size currently not configurable, using the NIST recommendation. privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { return generatedKeys, errors.Wrap(err, "unable to generate ECDSA private key") From 1a169d4f1f6e2a7029bcbd92313481417809a1c5 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Thu, 17 Oct 2024 12:53:43 -0500 Subject: [PATCH 07/17] update docs --- docs/reference/deploy/master-config-reference.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/reference/deploy/master-config-reference.rst b/docs/reference/deploy/master-config-reference.rst index dc474b3df4d..92413c3adcb 100644 --- a/docs/reference/deploy/master-config-reference.rst +++ b/docs/reference/deploy/master-config-reference.rst @@ -1524,6 +1524,11 @@ Specifies configuration settings for SSH. Number of bits to use when generating RSA keys for SSH for tasks. Maximum size is 16384. +``crypto_system`` +================= + +Specifies the crypto system for SSH. Currently accepts ``RSA``, ``ECDSA`` or ``ED25519``. + ``authz`` ========= From 078d1459a7606ec70f7cfe688a907550939568d0 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Thu, 17 Oct 2024 14:26:02 -0500 Subject: [PATCH 08/17] e2e --- .../devcluster/ssh-ecdsa.devcluster.yaml | 75 +++++++++++++++++++ .../devcluster/ssh-ed25519.devcluster.yaml | 75 +++++++++++++++++++ .circleci/real_config.yml | 22 ++++++ e2e_tests/pytest.ini | 1 + e2e_tests/tests/command/test_shell.py | 1 + 5 files changed, 174 insertions(+) create mode 100644 .circleci/devcluster/ssh-ecdsa.devcluster.yaml create mode 100644 .circleci/devcluster/ssh-ed25519.devcluster.yaml diff --git a/.circleci/devcluster/ssh-ecdsa.devcluster.yaml b/.circleci/devcluster/ssh-ecdsa.devcluster.yaml new file mode 100644 index 00000000000..c0595494b63 --- /dev/null +++ b/.circleci/devcluster/ssh-ecdsa.devcluster.yaml @@ -0,0 +1,75 @@ +stages: + - db: + name: db + + - master: + pre: + - sh: make -C tools prep-root + config_file: + db: + host: localhost + port: 5432 + password: postgres + user: postgres + name: determined + checkpoint_storage: + type: shared_fs + host_path: /tmp + storage_path: determined-cp + log: + level: debug + root: tools/build + cache: + cache_dir: /tmp/determined-cache + launch_error: false + telemetry: + enabled: false + security: + initial_user_password: $INITIAL_USER_PASSWORD + ssh: + crypto_system: ECDSA + resource_manager: + type: agent + default_aux_resource_pool: default + default_compute_resource_pool: default + resource_pools: + - pool_name: default + task_container_defaults: # for test_default_pool_task_container_defaults. + environment_variables: + - SOMEVAR=SOMEVAL + startup_hook: "echo 'hello from rp tcd startup hook'" + task_container_defaults: + startup_hook: "echo 'hello from master tcd startup hook'" + scim: + enabled: true + auth: + type: basic + username: determined + password: password + + - custom: + name: proxy + cmd: ["socat", "-d", "-d", "TCP-LISTEN:8081,reuseaddr,fork", "TCP:localhost:8080"] + post: + - conncheck: + port: 8081 + + - agent: + name: agent1 + config_file: + master_host: 127.0.0.1 + master_port: 8081 + agent_id: agent1 + container_master_host: $DOCKER_LOCALHOST + container_auto_remove_disabled: true + hooks: + on_connection_lost: ["touch", "/tmp/agent1-connection-lost"] + + - agent: + name: agent2 + config_file: + master_host: 127.0.0.1 + master_port: 8081 + agent_id: agent2 + container_master_host: $DOCKER_LOCALHOST + container_auto_remove_disabled: true diff --git a/.circleci/devcluster/ssh-ed25519.devcluster.yaml b/.circleci/devcluster/ssh-ed25519.devcluster.yaml new file mode 100644 index 00000000000..333e0af4fd2 --- /dev/null +++ b/.circleci/devcluster/ssh-ed25519.devcluster.yaml @@ -0,0 +1,75 @@ +stages: + - db: + name: db + + - master: + pre: + - sh: make -C tools prep-root + config_file: + db: + host: localhost + port: 5432 + password: postgres + user: postgres + name: determined + checkpoint_storage: + type: shared_fs + host_path: /tmp + storage_path: determined-cp + log: + level: debug + root: tools/build + cache: + cache_dir: /tmp/determined-cache + launch_error: false + telemetry: + enabled: false + security: + initial_user_password: $INITIAL_USER_PASSWORD + ssh: + crypto_system: ED25519 + resource_manager: + type: agent + default_aux_resource_pool: default + default_compute_resource_pool: default + resource_pools: + - pool_name: default + task_container_defaults: # for test_default_pool_task_container_defaults. + environment_variables: + - SOMEVAR=SOMEVAL + startup_hook: "echo 'hello from rp tcd startup hook'" + task_container_defaults: + startup_hook: "echo 'hello from master tcd startup hook'" + scim: + enabled: true + auth: + type: basic + username: determined + password: password + + - custom: + name: proxy + cmd: ["socat", "-d", "-d", "TCP-LISTEN:8081,reuseaddr,fork", "TCP:localhost:8080"] + post: + - conncheck: + port: 8081 + + - agent: + name: agent1 + config_file: + master_host: 127.0.0.1 + master_port: 8081 + agent_id: agent1 + container_master_host: $DOCKER_LOCALHOST + container_auto_remove_disabled: true + hooks: + on_connection_lost: ["touch", "/tmp/agent1-connection-lost"] + + - agent: + name: agent2 + config_file: + master_host: 127.0.0.1 + master_port: 8081 + agent_id: agent2 + container_master_host: $DOCKER_LOCALHOST + container_auto_remove_disabled: true diff --git a/.circleci/real_config.yml b/.circleci/real_config.yml index 3657b0b4e1f..55f832c44cb 100644 --- a/.circleci/real_config.yml +++ b/.circleci/real_config.yml @@ -4403,6 +4403,28 @@ workflows: mark: port_registry target-stage: agent + - test-e2e: + name: test-e2e-shell-ecdsa + context: + - dev-ci-cluster-default-user-credentials + requires: + - build-go-ee + parallelism: 1 + devcluster-config: ssh-ecdsa.devcluster.yaml + mark: e2e_shell_ssh + target-stage: agent2 + + - test-e2e: + name: test-e2e-shell-ed25519 + context: + - dev-ci-cluster-default-user-credentials + requires: + - build-go-ee + parallelism: 1 + devcluster-config: ssh-ed25519.devcluster.yaml + mark: e2e_shell_ssh + target-stage: agent2 + - test-e2e: name: test-e2e-cpu-elastic context: diff --git a/e2e_tests/pytest.ini b/e2e_tests/pytest.ini index 3305b6b1c66..34844f02621 100644 --- a/e2e_tests/pytest.ini +++ b/e2e_tests/pytest.ini @@ -16,6 +16,7 @@ markers = e2e_gpu: end to end GPU tests e2e_multi_k8s: end to end for multirm k8s e2e_single_k8s: end to end for single k8s + e2e_shell_ssh: end to end test for shell with different crypto system e2e_k8s: end to end tests specific to k8s (only used in test-e2e-gke-single-cpu currently) e2e_pbs: end to end pbs integration tests e2e_saml: tests for saml with okta diff --git a/e2e_tests/tests/command/test_shell.py b/e2e_tests/tests/command/test_shell.py index cf3a784de64..285a8ed8608 100644 --- a/e2e_tests/tests/command/test_shell.py +++ b/e2e_tests/tests/command/test_shell.py @@ -13,6 +13,7 @@ @pytest.mark.e2e_slurm @pytest.mark.e2e_pbs @pytest.mark.e2e_multi_k8s +@pytest.mark.e2e_shell_ssh def test_start_and_write_to_shell(tmp_path: pathlib.Path) -> None: sess = api_utils.user_session() with cmd.interactive_command(sess, ["shell", "start"]) as shell: From 305b16eb9838dc42fa042784ce75257cae59ebb2 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Fri, 18 Oct 2024 15:50:34 -0500 Subject: [PATCH 09/17] rename to key_type --- .circleci/devcluster/ssh-ecdsa.devcluster.yaml | 2 +- .circleci/devcluster/ssh-ed25519.devcluster.yaml | 2 +- docs/reference/deploy/master-config-reference.rst | 2 +- docs/release-notes/ssh-crypto-system.rst | 2 +- master/internal/api_user_intg_test.go | 2 +- master/internal/config/config.go | 13 ++++++++----- master/internal/core_intg_test.go | 2 +- master/pkg/ssh/ssh.go | 2 +- 8 files changed, 15 insertions(+), 12 deletions(-) diff --git a/.circleci/devcluster/ssh-ecdsa.devcluster.yaml b/.circleci/devcluster/ssh-ecdsa.devcluster.yaml index c0595494b63..8731a79d424 100644 --- a/.circleci/devcluster/ssh-ecdsa.devcluster.yaml +++ b/.circleci/devcluster/ssh-ecdsa.devcluster.yaml @@ -27,7 +27,7 @@ stages: security: initial_user_password: $INITIAL_USER_PASSWORD ssh: - crypto_system: ECDSA + key_type: ECDSA resource_manager: type: agent default_aux_resource_pool: default diff --git a/.circleci/devcluster/ssh-ed25519.devcluster.yaml b/.circleci/devcluster/ssh-ed25519.devcluster.yaml index 333e0af4fd2..5cc29ba4a80 100644 --- a/.circleci/devcluster/ssh-ed25519.devcluster.yaml +++ b/.circleci/devcluster/ssh-ed25519.devcluster.yaml @@ -27,7 +27,7 @@ stages: security: initial_user_password: $INITIAL_USER_PASSWORD ssh: - crypto_system: ED25519 + key_type: ED25519 resource_manager: type: agent default_aux_resource_pool: default diff --git a/docs/reference/deploy/master-config-reference.rst b/docs/reference/deploy/master-config-reference.rst index 92413c3adcb..656ecb160a8 100644 --- a/docs/reference/deploy/master-config-reference.rst +++ b/docs/reference/deploy/master-config-reference.rst @@ -1524,7 +1524,7 @@ Specifies configuration settings for SSH. Number of bits to use when generating RSA keys for SSH for tasks. Maximum size is 16384. -``crypto_system`` +``key_type`` ================= Specifies the crypto system for SSH. Currently accepts ``RSA``, ``ECDSA`` or ``ED25519``. diff --git a/docs/release-notes/ssh-crypto-system.rst b/docs/release-notes/ssh-crypto-system.rst index 75268915006..29a8b8b957e 100644 --- a/docs/release-notes/ssh-crypto-system.rst +++ b/docs/release-notes/ssh-crypto-system.rst @@ -3,4 +3,4 @@ **Improvements** - Master Configuration: Add support for crypto system configuration for ssh connection. - ``security.crypto_system`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. + ``security.key_type`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. diff --git a/master/internal/api_user_intg_test.go b/master/internal/api_user_intg_test.go index 812d2394e6a..96476f11a7b 100644 --- a/master/internal/api_user_intg_test.go +++ b/master/internal/api_user_intg_test.go @@ -118,7 +118,7 @@ func setupAPITest(t *testing.T, pgdb *db.PgDB, TaskContainerDefaults: model.TaskContainerDefaultsConfig{}, ResourceConfig: *config.DefaultResourceConfig(), }, - taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, CryptoSystem: "RSA"}}, + taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, KeyType: "RSA"}}, allRms: map[string]rm.ResourceManager{config.DefaultClusterName: mockRM}, }, } diff --git a/master/internal/config/config.go b/master/internal/config/config.go index f99192ac83b..ce8ded92eca 100644 --- a/master/internal/config/config.go +++ b/master/internal/config/config.go @@ -117,8 +117,8 @@ func DefaultConfig() *Config { Group: "root", }, SSH: SSHConfig{ - RsaKeySize: 1024, - CryptoSystem: RSACryptoSystem, + RsaKeySize: 1024, + KeyType: RSACryptoSystem, }, AuthZ: *DefaultAuthZConfig(), }, @@ -462,8 +462,8 @@ type SecurityConfig struct { // SSHConfig is the configuration setting for SSH. type SSHConfig struct { - RsaKeySize int `json:"rsa_key_size"` - CryptoSystem string `json:"crypto_system"` + RsaKeySize int `json:"rsa_key_size"` + KeyType string `json:"key_type"` } // TLSConfig is the configuration for setting up serving over TLS. @@ -486,9 +486,12 @@ func (t *TLSConfig) Validate() []error { // Validate implements the check.Validatable interface. func (t *SSHConfig) Validate() []error { var errs []error - if t.CryptoSystem != RSACryptoSystem && t.CryptoSystem != ECDSACryptoSystem && t.CryptoSystem != ED25519CryptoSystem { + if t.KeyType != RSACryptoSystem && t.KeyType != ECDSACryptoSystem && t.KeyType != ED25519CryptoSystem { errs = append(errs, errors.New("Crypto system must be one of 'RSA', 'ECDSA' or 'ED25519'")) } + if t.KeyType != RSACryptoSystem { + return errs + } if t.RsaKeySize < 1 { errs = append(errs, errors.New("RSA Key size must be greater than 0")) } else if t.RsaKeySize > 16384 { diff --git a/master/internal/core_intg_test.go b/master/internal/core_intg_test.go index 239ff047b57..ced6e233e31 100644 --- a/master/internal/core_intg_test.go +++ b/master/internal/core_intg_test.go @@ -121,7 +121,7 @@ func TestRun(t *testing.T) { DefaultLoggingConfig: &model.DefaultLoggingConfig{}, }, }, - taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, CryptoSystem: "RSA"}}, + taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, KeyType: "RSA"}}, } require.NoError(t, m.config.Resolve()) m.config.DB = config.DBConfig{ diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index 6508d6c43ce..77a0ce528a7 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -29,7 +29,7 @@ type PrivateAndPublicKeys struct { // GenerateKey returns a private and public SSH key. func GenerateKey(conf config.SSHConfig) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys - switch conf.CryptoSystem { + switch conf.KeyType { case config.RSACryptoSystem: return generateRSAKey(conf.RsaKeySize) case config.ECDSACryptoSystem: From e32501d449020736306b6243a7541e859100f4c5 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Mon, 21 Oct 2024 13:44:18 -0500 Subject: [PATCH 10/17] remove intg tests --- .../devcluster/ssh-ecdsa.devcluster.yaml | 75 ------------------- .../devcluster/ssh-ed25519.devcluster.yaml | 75 ------------------- .circleci/real_config.yml | 22 ------ e2e_tests/pytest.ini | 1 - e2e_tests/tests/command/test_shell.py | 1 - 5 files changed, 174 deletions(-) delete mode 100644 .circleci/devcluster/ssh-ecdsa.devcluster.yaml delete mode 100644 .circleci/devcluster/ssh-ed25519.devcluster.yaml diff --git a/.circleci/devcluster/ssh-ecdsa.devcluster.yaml b/.circleci/devcluster/ssh-ecdsa.devcluster.yaml deleted file mode 100644 index 8731a79d424..00000000000 --- a/.circleci/devcluster/ssh-ecdsa.devcluster.yaml +++ /dev/null @@ -1,75 +0,0 @@ -stages: - - db: - name: db - - - master: - pre: - - sh: make -C tools prep-root - config_file: - db: - host: localhost - port: 5432 - password: postgres - user: postgres - name: determined - checkpoint_storage: - type: shared_fs - host_path: /tmp - storage_path: determined-cp - log: - level: debug - root: tools/build - cache: - cache_dir: /tmp/determined-cache - launch_error: false - telemetry: - enabled: false - security: - initial_user_password: $INITIAL_USER_PASSWORD - ssh: - key_type: ECDSA - resource_manager: - type: agent - default_aux_resource_pool: default - default_compute_resource_pool: default - resource_pools: - - pool_name: default - task_container_defaults: # for test_default_pool_task_container_defaults. - environment_variables: - - SOMEVAR=SOMEVAL - startup_hook: "echo 'hello from rp tcd startup hook'" - task_container_defaults: - startup_hook: "echo 'hello from master tcd startup hook'" - scim: - enabled: true - auth: - type: basic - username: determined - password: password - - - custom: - name: proxy - cmd: ["socat", "-d", "-d", "TCP-LISTEN:8081,reuseaddr,fork", "TCP:localhost:8080"] - post: - - conncheck: - port: 8081 - - - agent: - name: agent1 - config_file: - master_host: 127.0.0.1 - master_port: 8081 - agent_id: agent1 - container_master_host: $DOCKER_LOCALHOST - container_auto_remove_disabled: true - hooks: - on_connection_lost: ["touch", "/tmp/agent1-connection-lost"] - - - agent: - name: agent2 - config_file: - master_host: 127.0.0.1 - master_port: 8081 - agent_id: agent2 - container_master_host: $DOCKER_LOCALHOST - container_auto_remove_disabled: true diff --git a/.circleci/devcluster/ssh-ed25519.devcluster.yaml b/.circleci/devcluster/ssh-ed25519.devcluster.yaml deleted file mode 100644 index 5cc29ba4a80..00000000000 --- a/.circleci/devcluster/ssh-ed25519.devcluster.yaml +++ /dev/null @@ -1,75 +0,0 @@ -stages: - - db: - name: db - - - master: - pre: - - sh: make -C tools prep-root - config_file: - db: - host: localhost - port: 5432 - password: postgres - user: postgres - name: determined - checkpoint_storage: - type: shared_fs - host_path: /tmp - storage_path: determined-cp - log: - level: debug - root: tools/build - cache: - cache_dir: /tmp/determined-cache - launch_error: false - telemetry: - enabled: false - security: - initial_user_password: $INITIAL_USER_PASSWORD - ssh: - key_type: ED25519 - resource_manager: - type: agent - default_aux_resource_pool: default - default_compute_resource_pool: default - resource_pools: - - pool_name: default - task_container_defaults: # for test_default_pool_task_container_defaults. - environment_variables: - - SOMEVAR=SOMEVAL - startup_hook: "echo 'hello from rp tcd startup hook'" - task_container_defaults: - startup_hook: "echo 'hello from master tcd startup hook'" - scim: - enabled: true - auth: - type: basic - username: determined - password: password - - - custom: - name: proxy - cmd: ["socat", "-d", "-d", "TCP-LISTEN:8081,reuseaddr,fork", "TCP:localhost:8080"] - post: - - conncheck: - port: 8081 - - - agent: - name: agent1 - config_file: - master_host: 127.0.0.1 - master_port: 8081 - agent_id: agent1 - container_master_host: $DOCKER_LOCALHOST - container_auto_remove_disabled: true - hooks: - on_connection_lost: ["touch", "/tmp/agent1-connection-lost"] - - - agent: - name: agent2 - config_file: - master_host: 127.0.0.1 - master_port: 8081 - agent_id: agent2 - container_master_host: $DOCKER_LOCALHOST - container_auto_remove_disabled: true diff --git a/.circleci/real_config.yml b/.circleci/real_config.yml index 55f832c44cb..3657b0b4e1f 100644 --- a/.circleci/real_config.yml +++ b/.circleci/real_config.yml @@ -4403,28 +4403,6 @@ workflows: mark: port_registry target-stage: agent - - test-e2e: - name: test-e2e-shell-ecdsa - context: - - dev-ci-cluster-default-user-credentials - requires: - - build-go-ee - parallelism: 1 - devcluster-config: ssh-ecdsa.devcluster.yaml - mark: e2e_shell_ssh - target-stage: agent2 - - - test-e2e: - name: test-e2e-shell-ed25519 - context: - - dev-ci-cluster-default-user-credentials - requires: - - build-go-ee - parallelism: 1 - devcluster-config: ssh-ed25519.devcluster.yaml - mark: e2e_shell_ssh - target-stage: agent2 - - test-e2e: name: test-e2e-cpu-elastic context: diff --git a/e2e_tests/pytest.ini b/e2e_tests/pytest.ini index 34844f02621..3305b6b1c66 100644 --- a/e2e_tests/pytest.ini +++ b/e2e_tests/pytest.ini @@ -16,7 +16,6 @@ markers = e2e_gpu: end to end GPU tests e2e_multi_k8s: end to end for multirm k8s e2e_single_k8s: end to end for single k8s - e2e_shell_ssh: end to end test for shell with different crypto system e2e_k8s: end to end tests specific to k8s (only used in test-e2e-gke-single-cpu currently) e2e_pbs: end to end pbs integration tests e2e_saml: tests for saml with okta diff --git a/e2e_tests/tests/command/test_shell.py b/e2e_tests/tests/command/test_shell.py index 285a8ed8608..cf3a784de64 100644 --- a/e2e_tests/tests/command/test_shell.py +++ b/e2e_tests/tests/command/test_shell.py @@ -13,7 +13,6 @@ @pytest.mark.e2e_slurm @pytest.mark.e2e_pbs @pytest.mark.e2e_multi_k8s -@pytest.mark.e2e_shell_ssh def test_start_and_write_to_shell(tmp_path: pathlib.Path) -> None: sess = api_utils.user_session() with cmd.interactive_command(sess, ["shell", "start"]) as shell: From 3c6475f5b574fb992de7f177ba2257cfc3263109 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Mon, 21 Oct 2024 13:44:23 -0500 Subject: [PATCH 11/17] fmt docs --- docs/reference/deploy/master-config-reference.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/deploy/master-config-reference.rst b/docs/reference/deploy/master-config-reference.rst index 656ecb160a8..661b08461cf 100644 --- a/docs/reference/deploy/master-config-reference.rst +++ b/docs/reference/deploy/master-config-reference.rst @@ -1525,7 +1525,7 @@ Specifies configuration settings for SSH. Number of bits to use when generating RSA keys for SSH for tasks. Maximum size is 16384. ``key_type`` -================= +============ Specifies the crypto system for SSH. Currently accepts ``RSA``, ``ECDSA`` or ``ED25519``. From 76227ad46f52664f632593f4eb82875d332489a2 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Mon, 21 Oct 2024 16:24:17 -0500 Subject: [PATCH 12/17] unit test --- master/pkg/ssh/ssh_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 master/pkg/ssh/ssh_test.go diff --git a/master/pkg/ssh/ssh_test.go b/master/pkg/ssh/ssh_test.go new file mode 100644 index 00000000000..ea3a925fc6f --- /dev/null +++ b/master/pkg/ssh/ssh_test.go @@ -0,0 +1,33 @@ +package ssh + +import ( + "testing" + + "golang.org/x/crypto/ssh" + "gotest.tools/assert" + + "github.com/determined-ai/determined/master/internal/config" +) + +func verifyKeys(t *testing.T, keys PrivateAndPublicKeys) { + privateKey, err := ssh.ParsePrivateKey(keys.PrivateKey) + assert.NilError(t, err) + + publickKey, _, _, _, err := ssh.ParseAuthorizedKey(keys.PublicKey) + assert.NilError(t, err) + assert.Equal(t, string(publickKey.Marshal()), string(privateKey.PublicKey().Marshal())) +} + +func TestSSHKeyGenerate(t *testing.T) { + keys, err := GenerateKey(config.SSHConfig{KeyType: config.RSACryptoSystem, RsaKeySize: 1024}) + assert.NilError(t, err) + verifyKeys(t, keys) + + keys, err = GenerateKey(config.SSHConfig{KeyType: config.ECDSACryptoSystem}) + assert.NilError(t, err) + verifyKeys(t, keys) + + keys, err = GenerateKey(config.SSHConfig{KeyType: config.ED25519CryptoSystem}) + assert.NilError(t, err) + verifyKeys(t, keys) +} From 6442f9acf173234e81c8b14c0c865644cfc6bcfb Mon Sep 17 00:00:00 2001 From: gt2345 Date: Mon, 21 Oct 2024 16:27:47 -0500 Subject: [PATCH 13/17] change default to ED25519 --- master/internal/config/config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/master/internal/config/config.go b/master/internal/config/config.go index ce8ded92eca..6fe439fa114 100644 --- a/master/internal/config/config.go +++ b/master/internal/config/config.go @@ -117,8 +117,7 @@ func DefaultConfig() *Config { Group: "root", }, SSH: SSHConfig{ - RsaKeySize: 1024, - KeyType: RSACryptoSystem, + KeyType: ED25519CryptoSystem, }, AuthZ: *DefaultAuthZConfig(), }, From 6d53f1ac2129d90f6a66e6a1b9daf03653a4e008 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Mon, 21 Oct 2024 16:37:17 -0500 Subject: [PATCH 14/17] nolint:dogsled --- master/pkg/ssh/ssh_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/master/pkg/ssh/ssh_test.go b/master/pkg/ssh/ssh_test.go index ea3a925fc6f..638c380ff78 100644 --- a/master/pkg/ssh/ssh_test.go +++ b/master/pkg/ssh/ssh_test.go @@ -13,7 +13,7 @@ func verifyKeys(t *testing.T, keys PrivateAndPublicKeys) { privateKey, err := ssh.ParsePrivateKey(keys.PrivateKey) assert.NilError(t, err) - publickKey, _, _, _, err := ssh.ParseAuthorizedKey(keys.PublicKey) + publickKey, _, _, _, err := ssh.ParseAuthorizedKey(keys.PublicKey) //nolint:dogsled assert.NilError(t, err) assert.Equal(t, string(publickKey.Marshal()), string(privateKey.PublicKey().Marshal())) } From c97313e68573e57948d654eaa97a7bafd43b837f Mon Sep 17 00:00:00 2001 From: gt2345 Date: Tue, 22 Oct 2024 10:49:15 -0500 Subject: [PATCH 15/17] update name and docs --- docs/release-notes/ssh-crypto-system.rst | 4 +++- master/internal/config/config.go | 18 +++++++++--------- master/pkg/ssh/ssh.go | 6 +++--- master/pkg/ssh/ssh_test.go | 6 +++--- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/docs/release-notes/ssh-crypto-system.rst b/docs/release-notes/ssh-crypto-system.rst index 29a8b8b957e..acd54812832 100644 --- a/docs/release-notes/ssh-crypto-system.rst +++ b/docs/release-notes/ssh-crypto-system.rst @@ -3,4 +3,6 @@ **Improvements** - Master Configuration: Add support for crypto system configuration for ssh connection. - ``security.key_type`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. + ``security.key_type`` now accepts ``RSA``, ``ECDSA`` or ``ED25519``. Default key type is changed + from ``1024-bit RSA`` to ``ED25519``, since ``ED25519`` keys are faster and more secure than the + old default, and ``ED25519`` is also the default key type for ``ssh-keygen``. diff --git a/master/internal/config/config.go b/master/internal/config/config.go index 6fe439fa114..874f17d3a27 100644 --- a/master/internal/config/config.go +++ b/master/internal/config/config.go @@ -43,12 +43,12 @@ const ( ) const ( - // RSACryptoSystem uses RSA. - RSACryptoSystem = "RSA" - // ECDSACryptoSystem uses ECDSA. - ECDSACryptoSystem = "ECDSA" - // ED25519CryptoSystem uses ED25519. - ED25519CryptoSystem = "ED25519" + // RSAKeyType uses RSA. + RSAKeyType = "RSA" + // ECDSAKeyType uses ECDSA. + ECDSAKeyType = "ECDSA" + // ED25519KeyType uses ED25519. + ED25519KeyType = "ED25519" ) type ( @@ -117,7 +117,7 @@ func DefaultConfig() *Config { Group: "root", }, SSH: SSHConfig{ - KeyType: ED25519CryptoSystem, + KeyType: ED25519KeyType, }, AuthZ: *DefaultAuthZConfig(), }, @@ -485,10 +485,10 @@ func (t *TLSConfig) Validate() []error { // Validate implements the check.Validatable interface. func (t *SSHConfig) Validate() []error { var errs []error - if t.KeyType != RSACryptoSystem && t.KeyType != ECDSACryptoSystem && t.KeyType != ED25519CryptoSystem { + if t.KeyType != RSAKeyType && t.KeyType != ECDSAKeyType && t.KeyType != ED25519KeyType { errs = append(errs, errors.New("Crypto system must be one of 'RSA', 'ECDSA' or 'ED25519'")) } - if t.KeyType != RSACryptoSystem { + if t.KeyType != RSAKeyType { return errs } if t.RsaKeySize < 1 { diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index 77a0ce528a7..e5a361a031c 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -30,11 +30,11 @@ type PrivateAndPublicKeys struct { func GenerateKey(conf config.SSHConfig) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys switch conf.KeyType { - case config.RSACryptoSystem: + case config.RSAKeyType: return generateRSAKey(conf.RsaKeySize) - case config.ECDSACryptoSystem: + case config.ECDSAKeyType: return generateECDSAKey() - case config.ED25519CryptoSystem: + case config.ED25519KeyType: return generateED25519Key() default: return generatedKeys, errors.New("Invalid crypto system") diff --git a/master/pkg/ssh/ssh_test.go b/master/pkg/ssh/ssh_test.go index 638c380ff78..f0e6bf72c23 100644 --- a/master/pkg/ssh/ssh_test.go +++ b/master/pkg/ssh/ssh_test.go @@ -19,15 +19,15 @@ func verifyKeys(t *testing.T, keys PrivateAndPublicKeys) { } func TestSSHKeyGenerate(t *testing.T) { - keys, err := GenerateKey(config.SSHConfig{KeyType: config.RSACryptoSystem, RsaKeySize: 1024}) + keys, err := GenerateKey(config.SSHConfig{KeyType: config.RSAKeyType, RsaKeySize: 512}) assert.NilError(t, err) verifyKeys(t, keys) - keys, err = GenerateKey(config.SSHConfig{KeyType: config.ECDSACryptoSystem}) + keys, err = GenerateKey(config.SSHConfig{KeyType: config.ECDSAKeyType}) assert.NilError(t, err) verifyKeys(t, keys) - keys, err = GenerateKey(config.SSHConfig{KeyType: config.ED25519CryptoSystem}) + keys, err = GenerateKey(config.SSHConfig{KeyType: config.ED25519KeyType}) assert.NilError(t, err) verifyKeys(t, keys) } From bf788802cd26047c3acc30514558a76798c30dfe Mon Sep 17 00:00:00 2001 From: gt2345 Date: Wed, 23 Oct 2024 18:04:54 -0500 Subject: [PATCH 16/17] comments --- master/internal/api_user_intg_test.go | 2 +- master/internal/config/config.go | 29 +++++++++--------- master/internal/core_intg_test.go | 2 +- master/pkg/ssh/ssh.go | 44 +++++++++++---------------- master/pkg/ssh/ssh_test.go | 28 ++++++++++------- 5 files changed, 51 insertions(+), 54 deletions(-) diff --git a/master/internal/api_user_intg_test.go b/master/internal/api_user_intg_test.go index 96476f11a7b..3e55576a552 100644 --- a/master/internal/api_user_intg_test.go +++ b/master/internal/api_user_intg_test.go @@ -118,7 +118,7 @@ func setupAPITest(t *testing.T, pgdb *db.PgDB, TaskContainerDefaults: model.TaskContainerDefaultsConfig{}, ResourceConfig: *config.DefaultResourceConfig(), }, - taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, KeyType: "RSA"}}, + taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{KeyType: "ED25519"}}, allRms: map[string]rm.ResourceManager{config.DefaultClusterName: mockRM}, }, } diff --git a/master/internal/config/config.go b/master/internal/config/config.go index 874f17d3a27..f68f1cc6095 100644 --- a/master/internal/config/config.go +++ b/master/internal/config/config.go @@ -43,12 +43,12 @@ const ( ) const ( - // RSAKeyType uses RSA. - RSAKeyType = "RSA" - // ECDSAKeyType uses ECDSA. - ECDSAKeyType = "ECDSA" - // ED25519KeyType uses ED25519. - ED25519KeyType = "ED25519" + // KeyTypeRSA uses RSA. + KeyTypeRSA = "RSA" + // KeyTypeECDSA uses ECDSA. + KeyTypeECDSA = "ECDSA" + // KeyTypeED25519 uses ED25519. + KeyTypeED25519 = "ED25519" ) type ( @@ -117,7 +117,7 @@ func DefaultConfig() *Config { Group: "root", }, SSH: SSHConfig{ - KeyType: ED25519KeyType, + KeyType: KeyTypeED25519, }, AuthZ: *DefaultAuthZConfig(), }, @@ -485,16 +485,15 @@ func (t *TLSConfig) Validate() []error { // Validate implements the check.Validatable interface. func (t *SSHConfig) Validate() []error { var errs []error - if t.KeyType != RSAKeyType && t.KeyType != ECDSAKeyType && t.KeyType != ED25519KeyType { + if t.KeyType != KeyTypeRSA && t.KeyType != KeyTypeECDSA && t.KeyType != KeyTypeED25519 { errs = append(errs, errors.New("Crypto system must be one of 'RSA', 'ECDSA' or 'ED25519'")) } - if t.KeyType != RSAKeyType { - return errs - } - if t.RsaKeySize < 1 { - errs = append(errs, errors.New("RSA Key size must be greater than 0")) - } else if t.RsaKeySize > 16384 { - errs = append(errs, errors.New("RSA Key size must be less than 16,384")) + if t.KeyType == KeyTypeRSA { + if t.RsaKeySize < 1 { + errs = append(errs, errors.New("RSA Key size must be greater than 0")) + } else if t.RsaKeySize > 16384 { + errs = append(errs, errors.New("RSA Key size must be less than 16,384")) + } } return errs } diff --git a/master/internal/core_intg_test.go b/master/internal/core_intg_test.go index ced6e233e31..4935cc38a05 100644 --- a/master/internal/core_intg_test.go +++ b/master/internal/core_intg_test.go @@ -121,7 +121,7 @@ func TestRun(t *testing.T) { DefaultLoggingConfig: &model.DefaultLoggingConfig{}, }, }, - taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{RsaKeySize: 1024, KeyType: "RSA"}}, + taskSpec: &tasks.TaskSpec{SSHConfig: config.SSHConfig{KeyType: "ED25519"}}, } require.NoError(t, m.config.Resolve()) m.config.DB = config.DBConfig{ diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index e5a361a031c..24e4fb1114c 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -30,11 +30,11 @@ type PrivateAndPublicKeys struct { func GenerateKey(conf config.SSHConfig) (PrivateAndPublicKeys, error) { var generatedKeys PrivateAndPublicKeys switch conf.KeyType { - case config.RSAKeyType: + case config.KeyTypeRSA: return generateRSAKey(conf.RsaKeySize) - case config.ECDSAKeyType: + case config.KeyTypeECDSA: return generateECDSAKey() - case config.ED25519KeyType: + case config.KeyTypeED25519: return generateED25519Key() default: return generatedKeys, errors.New("Invalid crypto system") @@ -42,14 +42,13 @@ func GenerateKey(conf config.SSHConfig) (PrivateAndPublicKeys, error) { } func generateRSAKey(rsaKeySize int) (PrivateAndPublicKeys, error) { - var generatedKeys PrivateAndPublicKeys privateKey, err := rsa.GenerateKey(rand.Reader, rsaKeySize) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate RSA private key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate RSA private key") } if err = privateKey.Validate(); err != nil { - return generatedKeys, err + return PrivateAndPublicKeys{}, err } block := &pem.Block{ @@ -59,28 +58,25 @@ func generateRSAKey(rsaKeySize int) (PrivateAndPublicKeys, error) { publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate RSA public key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate RSA public key") } - generatedKeys = PrivateAndPublicKeys{ + return PrivateAndPublicKeys{ PrivateKey: pem.EncodeToMemory(block), PublicKey: sshlib.MarshalAuthorizedKey(publicKey), - } - - return generatedKeys, nil + }, nil } func generateECDSAKey() (PrivateAndPublicKeys, error) { - var generatedKeys PrivateAndPublicKeys // Curve size currently not configurable, using the NIST recommendation. privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ECDSA private key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate ECDSA private key") } privateKeyBytes, err := x509.MarshalECPrivateKey(privateKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to marshal ECDSA private key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to marshal ECDSA private key") } block := &pem.Block{ @@ -90,40 +86,36 @@ func generateECDSAKey() (PrivateAndPublicKeys, error) { publicKey, err := sshlib.NewPublicKey(&privateKey.PublicKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ECDSA public key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate ECDSA public key") } - generatedKeys = PrivateAndPublicKeys{ + return PrivateAndPublicKeys{ PrivateKey: pem.EncodeToMemory(block), PublicKey: sshlib.MarshalAuthorizedKey(publicKey), - } - - return generatedKeys, nil + }, nil } func generateED25519Key() (PrivateAndPublicKeys, error) { - var generatedKeys PrivateAndPublicKeys ed25519PublicKey, privateKey, err := ed25519.GenerateKey(nil) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ED25519 private key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate ED25519 private key") } // Before OpenSSH 9.6, for ED25519 keys, only the OpenSSH private key format was supported. block, err := sshlib.MarshalPrivateKey(privateKey, "") if err != nil { - return generatedKeys, errors.Wrap(err, "unable to marshal ED25519 private key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to marshal ED25519 private key") } publicKey, err := sshlib.NewPublicKey(ed25519PublicKey) if err != nil { - return generatedKeys, errors.Wrap(err, "unable to generate ED25519 public key") + return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate ED25519 public key") } - generatedKeys = PrivateAndPublicKeys{ + return PrivateAndPublicKeys{ PrivateKey: pem.EncodeToMemory(block), PublicKey: sshlib.MarshalAuthorizedKey(publicKey), - } + }, nil - return generatedKeys, nil } diff --git a/master/pkg/ssh/ssh_test.go b/master/pkg/ssh/ssh_test.go index f0e6bf72c23..a5397ac1321 100644 --- a/master/pkg/ssh/ssh_test.go +++ b/master/pkg/ssh/ssh_test.go @@ -19,15 +19,21 @@ func verifyKeys(t *testing.T, keys PrivateAndPublicKeys) { } func TestSSHKeyGenerate(t *testing.T) { - keys, err := GenerateKey(config.SSHConfig{KeyType: config.RSAKeyType, RsaKeySize: 512}) - assert.NilError(t, err) - verifyKeys(t, keys) - - keys, err = GenerateKey(config.SSHConfig{KeyType: config.ECDSAKeyType}) - assert.NilError(t, err) - verifyKeys(t, keys) - - keys, err = GenerateKey(config.SSHConfig{KeyType: config.ED25519KeyType}) - assert.NilError(t, err) - verifyKeys(t, keys) + t.Run("generate RSA key", func(t *testing.T) { + keys, err := GenerateKey(config.SSHConfig{KeyType: config.KeyTypeRSA, RsaKeySize: 512}) + assert.NilError(t, err) + verifyKeys(t, keys) + }) + + t.Run("generate ECDSA key", func(t *testing.T) { + keys, err := GenerateKey(config.SSHConfig{KeyType: config.KeyTypeECDSA}) + assert.NilError(t, err) + verifyKeys(t, keys) + }) + + t.Run("generate ED25519 key", func(t *testing.T) { + keys, err := GenerateKey(config.SSHConfig{KeyType: config.KeyTypeED25519}) + assert.NilError(t, err) + verifyKeys(t, keys) + }) } From 1690bb7958a63004e76b37ad8744f630fd9d2e18 Mon Sep 17 00:00:00 2001 From: gt2345 Date: Wed, 23 Oct 2024 19:07:36 -0500 Subject: [PATCH 17/17] fmt --- master/pkg/ssh/ssh.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/master/pkg/ssh/ssh.go b/master/pkg/ssh/ssh.go index 24e4fb1114c..73a506b09e6 100644 --- a/master/pkg/ssh/ssh.go +++ b/master/pkg/ssh/ssh.go @@ -96,7 +96,6 @@ func generateECDSAKey() (PrivateAndPublicKeys, error) { } func generateED25519Key() (PrivateAndPublicKeys, error) { - ed25519PublicKey, privateKey, err := ed25519.GenerateKey(nil) if err != nil { return PrivateAndPublicKeys{}, errors.Wrap(err, "unable to generate ED25519 private key") @@ -117,5 +116,4 @@ func generateED25519Key() (PrivateAndPublicKeys, error) { PrivateKey: pem.EncodeToMemory(block), PublicKey: sshlib.MarshalAuthorizedKey(publicKey), }, nil - }