diff options
author | Nick Thomas <nick@gitlab.com> | 2021-04-30 14:15:30 +0100 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2021-04-30 14:32:06 +0100 |
commit | 4545fc56e23b476ca2b54cc23ec72a0aa0d7dae1 (patch) | |
tree | 8175967320e2fa8e022e5629d6f39b2ddcfe16bb | |
parent | 584643e0e10e0cbeee4f8366b5e50656dfee9ea4 (diff) | |
download | gitlab-shell-4545fc56e23b476ca2b54cc23ec72a0aa0d7dae1.tar.gz |
gitlab-sshd: Respect the ssl_cert_dir config516-handle-ssl-cert-dir-correctly
Changelog: fixed
-rw-r--r-- | cmd/gitlab-sshd/main.go | 3 | ||||
-rw-r--r-- | internal/command/command.go | 5 | ||||
-rw-r--r-- | internal/command/command_test.go | 142 | ||||
-rw-r--r-- | internal/config/config.go | 10 | ||||
-rw-r--r-- | internal/config/config_test.go | 24 |
5 files changed, 97 insertions, 87 deletions
diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index 04d6888..f777b9f 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -54,6 +54,9 @@ func main() { } log.Fatalf("configuration error: %v", err) } + + cfg.ApplyGlobalState() + logger.ConfigureStandalone(cfg) // Startup monitoring endpoint. diff --git a/internal/command/command.go b/internal/command/command.go index bb430b7..038cd5d 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -2,7 +2,6 @@ package command import ( "context" - "os" "gitlab.com/gitlab-org/gitlab-shell/internal/command/authorizedkeys" "gitlab.com/gitlab-org/gitlab-shell/internal/command/authorizedprincipals" @@ -37,10 +36,6 @@ func New(e *executable.Executable, arguments []string, env sshenv.Env, config *c } if cmd := buildCommand(e, args, config, readWriter); cmd != nil { - if config.SslCertDir != "" { - os.Setenv("SSL_CERT_DIR", config.SslCertDir) - } - return cmd, nil } diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 4b315aa..a70ea84 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -44,119 +44,97 @@ func buildEnv(command string) sshenv.Env { func TestNew(t *testing.T) { testCases := []struct { - desc string - executable *executable.Executable - env sshenv.Env - arguments []string - config *config.Config - expectedType interface{} - expectedSslCertDir string + desc string + executable *executable.Executable + env sshenv.Env + arguments []string + config *config.Config + expectedType interface{} }{ { - desc: "it returns a Discover command", - executable: gitlabShellExec, - env: buildEnv(""), - config: basicConfig, - expectedType: &discover.Command{}, - expectedSslCertDir: "", + desc: "it returns a Discover command", + executable: gitlabShellExec, + env: buildEnv(""), + config: basicConfig, + expectedType: &discover.Command{}, }, { - desc: "it returns a Discover command with SSL_CERT_DIR env var set", - executable: gitlabShellExec, - env: buildEnv(""), - config: advancedConfig, - expectedType: &discover.Command{}, - expectedSslCertDir: "/tmp/certs", + desc: "it returns a TwoFactorRecover command", + executable: gitlabShellExec, + env: buildEnv("2fa_recovery_codes"), + config: basicConfig, + expectedType: &twofactorrecover.Command{}, }, { - desc: "it returns a TwoFactorRecover command", - executable: gitlabShellExec, - env: buildEnv("2fa_recovery_codes"), - config: basicConfig, - expectedType: &twofactorrecover.Command{}, - expectedSslCertDir: "", + desc: "it returns a TwoFactorVerify command", + executable: gitlabShellExec, + env: buildEnv("2fa_verify"), + config: basicConfig, + expectedType: &twofactorverify.Command{}, }, { - desc: "it returns a TwoFactorVerify command", - executable: gitlabShellExec, - env: buildEnv("2fa_verify"), - config: basicConfig, - expectedType: &twofactorverify.Command{}, - expectedSslCertDir: "", + desc: "it returns an LfsAuthenticate command", + executable: gitlabShellExec, + env: buildEnv("git-lfs-authenticate"), + config: basicConfig, + expectedType: &lfsauthenticate.Command{}, }, { - desc: "it returns an LfsAuthenticate command", - executable: gitlabShellExec, - env: buildEnv("git-lfs-authenticate"), - config: basicConfig, - expectedType: &lfsauthenticate.Command{}, - expectedSslCertDir: "", + desc: "it returns a ReceivePack command", + executable: gitlabShellExec, + env: buildEnv("git-receive-pack"), + config: basicConfig, + expectedType: &receivepack.Command{}, }, { - desc: "it returns a ReceivePack command", - executable: gitlabShellExec, - env: buildEnv("git-receive-pack"), - config: basicConfig, - expectedType: &receivepack.Command{}, - expectedSslCertDir: "", + desc: "it returns an UploadPack command", + executable: gitlabShellExec, + env: buildEnv("git-upload-pack"), + config: basicConfig, + expectedType: &uploadpack.Command{}, }, { - desc: "it returns an UploadPack command", - executable: gitlabShellExec, - env: buildEnv("git-upload-pack"), - config: basicConfig, - expectedType: &uploadpack.Command{}, - expectedSslCertDir: "", + desc: "it returns an UploadArchive command", + executable: gitlabShellExec, + env: buildEnv("git-upload-archive"), + config: basicConfig, + expectedType: &uploadarchive.Command{}, }, { - desc: "it returns an UploadArchive command", - executable: gitlabShellExec, - env: buildEnv("git-upload-archive"), - config: basicConfig, - expectedType: &uploadarchive.Command{}, - expectedSslCertDir: "", + desc: "it returns a Healthcheck command", + executable: checkExec, + config: basicConfig, + expectedType: &healthcheck.Command{}, }, { - desc: "it returns a Healthcheck command", - executable: checkExec, - config: basicConfig, - expectedType: &healthcheck.Command{}, - expectedSslCertDir: "", + desc: "it returns a AuthorizedKeys command", + executable: authorizedKeysExec, + arguments: []string{"git", "git", "key"}, + config: basicConfig, + expectedType: &authorizedkeys.Command{}, }, { - desc: "it returns a AuthorizedKeys command", - executable: authorizedKeysExec, - arguments: []string{"git", "git", "key"}, - config: basicConfig, - expectedType: &authorizedkeys.Command{}, - expectedSslCertDir: "", + desc: "it returns a AuthorizedPrincipals command", + executable: authorizedPrincipalsExec, + arguments: []string{"key", "principal"}, + config: basicConfig, + expectedType: &authorizedprincipals.Command{}, }, { - desc: "it returns a AuthorizedPrincipals command", - executable: authorizedPrincipalsExec, - arguments: []string{"key", "principal"}, - config: basicConfig, - expectedType: &authorizedprincipals.Command{}, - expectedSslCertDir: "", - }, - { - desc: "it returns a PersonalAccessToken command", - executable: gitlabShellExec, - env: buildEnv("personal_access_token"), - config: basicConfig, - expectedType: &personalaccesstoken.Command{}, - expectedSslCertDir: "", + desc: "it returns a PersonalAccessToken command", + executable: gitlabShellExec, + env: buildEnv("personal_access_token"), + config: basicConfig, + expectedType: &personalaccesstoken.Command{}, }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - os.Unsetenv("SSL_CERT_DIR") command, err := New(tc.executable, tc.arguments, tc.env, tc.config, nil) require.NoError(t, err) require.IsType(t, tc.expectedType, command) - require.Equal(t, tc.expectedSslCertDir, os.Getenv("SSL_CERT_DIR")) }) } } diff --git a/internal/config/config.go b/internal/config/config.go index ed061a8..e3dd7c2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ import ( "errors" "io/ioutil" "net/url" + "os" "path" "path/filepath" "sync" @@ -74,6 +75,12 @@ var ( } ) +func (c *Config) ApplyGlobalState() { + if c.SslCertDir != "" { + os.Setenv("SSL_CERT_DIR", c.SslCertDir) + } +} + func (c *Config) HttpClient() *client.HttpClient { c.httpClientOnce.Do(func() { c.httpClient = client.NewHTTPClient( @@ -96,6 +103,9 @@ func NewFromDirExternal(dir string) (*Config, error) { if err != nil { return nil, err } + + cfg.ApplyGlobalState() + return cfg, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..1a79320 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,24 @@ +package config + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-shell/internal/testhelper" +) + +func TestConfigApplyGlobalState(t *testing.T) { + t.Cleanup(testhelper.TempEnv(map[string]string{"SSL_CERT_DIR": "unmodified"})) + + config := &Config{SslCertDir: ""} + config.ApplyGlobalState() + + require.Equal(t, "unmodified", os.Getenv("SSL_CERT_DIR")) + + config.SslCertDir = "foo" + config.ApplyGlobalState() + + require.Equal(t, "foo", os.Getenv("SSL_CERT_DIR")) +} |