Merge branch '459-system-default-ssl_cert_dir-is-being-used-during-remote-gitaly-over-tls' into 'master'

Support ssl_cert_dir config setting

See merge request gitlab-org/gitlab-shell!393

9 files changed, 110 insertions, 26 deletions

diff --git a/config.yml.example b/config.yml.example
index c2c1027..60435a3 100644
--- a/config.yml.example
+++ b/config.yml.example
@@ -27,6 +27,10 @@ http_settings:
 # File used as authorized_keys for gitlab user
 auth_file: "/home/git/.ssh/authorized_keys"
 
+# SSL certificate dir where custom certificates can be placed
+# https://golang.org/pkg/crypto/x509/
+# ssl_cert_dir: /opt/gitlab/embedded/ssl/certs/
+
 # File that contains the secret key for verifying access to GitLab.
 # Default is .gitlab_shell_secret in the gitlab-shell directory.
 # secret_file: "/home/git/gitlab-shell/.gitlab_shell_secret"
diff --git a/internal/command/authorizedkeys/authorized_keys.go b/internal/command/authorizedkeys/authorized_keys.go
index f1cab45..7554761 100644
--- a/internal/command/authorizedkeys/authorized_keys.go
+++ b/internal/command/authorizedkeys/authorized_keys.go
@@ -41,7 +41,7 @@ func (c *Command) printKeyLine() error {
 		return nil
 	}
 
-	keyLine, err := keyline.NewPublicKeyLine(strconv.FormatInt(response.Id, 10), response.Key, c.Config.RootDir)
+	keyLine, err := keyline.NewPublicKeyLine(strconv.FormatInt(response.Id, 10), response.Key, c.Config)
 	if err != nil {
 		return err
 	}
diff --git a/internal/command/authorizedkeys/authorized_keys_test.go b/internal/command/authorizedkeys/authorized_keys_test.go
index 4aa7586..e12f4fa 100644
--- a/internal/command/authorizedkeys/authorized_keys_test.go
+++ b/internal/command/authorizedkeys/authorized_keys_test.go
@@ -45,8 +45,12 @@ func TestExecute(t *testing.T) {
 	url, cleanup := testserver.StartSocketHttpServer(t, requests)
 	defer cleanup()
 
+	defaultConfig := &config.Config{RootDir: "/tmp", GitlabUrl: url}
+	configWithSslCertDir := &config.Config{RootDir: "/tmp", GitlabUrl: url, SslCertDir: "/tmp/certs"}
+
 	testCases := []struct {
 		desc           string
+		config         *config.Config
 		arguments      *commandargs.AuthorizedKeys
 		expectedOutput string
 	}{
@@ -56,6 +60,12 @@ func TestExecute(t *testing.T) {
 			expectedOutput: "command=\"/tmp/bin/gitlab-shell key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key\n",
 		},
 		{
+			desc:           "With SSL cert dir",
+			config:         configWithSslCertDir,
+			arguments:      &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "key"},
+			expectedOutput: "command=\"SSL_CERT_DIR=/tmp/certs /tmp/bin/gitlab-shell key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key\n",
+		},
+		{
 			desc:           "When key doesn't match any existing key",
 			arguments:      &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "not-found"},
 			expectedOutput: "# No key was found for not-found\n",
@@ -75,8 +85,14 @@ func TestExecute(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			buffer := &bytes.Buffer{}
+
+			config := defaultConfig
+			if tc.config != nil {
+				config = tc.config
+			}
+
 			cmd := &Command{
-				Config:     &config.Config{RootDir: "/tmp", GitlabUrl: url},
+				Config:     config,
 				Args:       tc.arguments,
 				ReadWriter: &readwriter.ReadWriter{Out: buffer},
 			}
diff --git a/internal/command/authorizedprincipals/authorized_principals.go b/internal/command/authorizedprincipals/authorized_principals.go
index 10ae70e..ab5f2f8 100644
--- a/internal/command/authorizedprincipals/authorized_principals.go
+++ b/internal/command/authorizedprincipals/authorized_principals.go
@@ -36,7 +36,7 @@ func (c *Command) printPrincipalLines() error {
 }
 
 func (c *Command) printPrincipalLine(principal string) error {
-	principalKeyLine, err := keyline.NewPrincipalKeyLine(c.Args.KeyId, principal, c.Config.RootDir)
+	principalKeyLine, err := keyline.NewPrincipalKeyLine(c.Args.KeyId, principal, c.Config)
 	if err != nil {
 		return err
 	}
diff --git a/internal/command/authorizedprincipals/authorized_principals_test.go b/internal/command/authorizedprincipals/authorized_principals_test.go
index f0334e5..f11dd0f 100644
--- a/internal/command/authorizedprincipals/authorized_principals_test.go
+++ b/internal/command/authorizedprincipals/authorized_principals_test.go
@@ -12,8 +12,12 @@ import (
 )
 
 func TestExecute(t *testing.T) {
+	defaultConfig := &config.Config{RootDir: "/tmp"}
+	configWithSslCertDir := &config.Config{RootDir: "/tmp", SslCertDir: "/tmp/certs"}
+
 	testCases := []struct {
 		desc           string
+		config         *config.Config
 		arguments      *commandargs.AuthorizedPrincipals
 		expectedOutput string
 	}{
@@ -23,6 +27,12 @@ func TestExecute(t *testing.T) {
 			expectedOutput: "command=\"/tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal\n",
 		},
 		{
+			desc:           "With SSL cert dir",
+			config:         configWithSslCertDir,
+			arguments:      &commandargs.AuthorizedPrincipals{KeyId: "key", Principals: []string{"principal"}},
+			expectedOutput: "command=\"SSL_CERT_DIR=/tmp/certs /tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal\n",
+		},
+		{
 			desc:           "With multiple principals",
 			arguments:      &commandargs.AuthorizedPrincipals{KeyId: "key", Principals: []string{"principal-1", "principal-2"}},
 			expectedOutput: "command=\"/tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal-1\ncommand=\"/tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal-2\n",
@@ -32,8 +42,14 @@ func TestExecute(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			buffer := &bytes.Buffer{}
+
+			config := defaultConfig
+			if tc.config != nil {
+				config = tc.config
+			}
+
 			cmd := &Command{
-				Config:     &config.Config{RootDir: "/tmp"},
+				Config:     config,
 				Args:       tc.arguments,
 				ReadWriter: &readwriter.ReadWriter{Out: buffer},
 			}
diff --git a/internal/config/config.go b/internal/config/config.go
index deed74d..be9efa3 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -34,6 +34,7 @@ type Config struct {
 	GitlabTracing  string             `yaml:"gitlab_tracing"`
 	SecretFilePath string             `yaml:"secret_file"`
 	Secret         string             `yaml:"secret"`
+	SslCertDir     string             `yaml:"ssl_cert_dir"`
 	HttpSettings   HttpSettingsConfig `yaml:"http_settings"`
 	HttpClient     *client.HttpClient
 }
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 202db6d..77351f2 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -29,6 +29,7 @@ func TestParseConfig(t *testing.T) {
 		format       string
 		gitlabUrl    string
 		secret       string
+		sslCertDir   string
 		httpSettings HttpSettingsConfig
 	}{
 		{
@@ -80,6 +81,13 @@ func TestParseConfig(t *testing.T) {
 			secret: "an inline secret",
 		},
 		{
+			yaml:       "ssl_cert_dir: /tmp/certs",
+			path:       path.Join(testRoot, "gitlab-shell.log"),
+			format:     "text",
+			secret:     "default-secret-content",
+			sslCertDir: "/tmp/certs",
+		},
+		{
 			yaml:         "http_settings:\n  user: user_basic_auth\n  password: password_basic_auth\n  read_timeout: 500",
 			path:         path.Join(testRoot, "gitlab-shell.log"),
 			format:       "text",
@@ -106,6 +114,7 @@ func TestParseConfig(t *testing.T) {
 			assert.Equal(t, tc.format, cfg.LogFormat)
 			assert.Equal(t, tc.gitlabUrl, cfg.GitlabUrl)
 			assert.Equal(t, tc.secret, cfg.Secret)
+			assert.Equal(t, tc.sslCertDir, cfg.SslCertDir)
 			assert.Equal(t, tc.httpSettings, cfg.HttpSettings)
 		})
 	}
diff --git a/internal/keyline/key_line.go b/internal/keyline/key_line.go
index c29a320..e2abb82 100644
--- a/internal/keyline/key_line.go
+++ b/internal/keyline/key_line.go
@@ -7,6 +7,7 @@ import (
 	"regexp"
 	"strings"
 
+	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/executable"
 )
 
@@ -21,32 +22,45 @@ const (
 )
 
 type KeyLine struct {
-	Id      string // This can be either an ID of a Key or username
-	Value   string // This can be either a public key or a principal name
-	Prefix  string
-	RootDir string
+	Id     string // This can be either an ID of a Key or username
+	Value  string // This can be either a public key or a principal name
+	Prefix string
+	Config *config.Config
 }
 
-func NewPublicKeyLine(id string, publicKey string, rootDir string) (*KeyLine, error) {
-	return newKeyLine(id, publicKey, PublicKeyPrefix, rootDir)
+func NewPublicKeyLine(id, publicKey string, config *config.Config) (*KeyLine, error) {
+	return newKeyLine(id, publicKey, PublicKeyPrefix, config)
 }
 
-func NewPrincipalKeyLine(keyId string, principal string, rootDir string) (*KeyLine, error) {
-	return newKeyLine(keyId, principal, PrincipalPrefix, rootDir)
+func NewPrincipalKeyLine(keyId, principal string, config *config.Config) (*KeyLine, error) {
+	return newKeyLine(keyId, principal, PrincipalPrefix, config)
 }
 
 func (k *KeyLine) ToString() string {
-	command := fmt.Sprintf("%s %s-%s", path.Join(k.RootDir, executable.BinDir, executable.GitlabShell), k.Prefix, k.Id)
+	sslCertDirEnvVar := k.sslCertDirEnvVar()
+	command := fmt.Sprintf("%s %s-%s", path.Join(k.Config.RootDir, executable.BinDir, executable.GitlabShell), k.Prefix, k.Id)
 
-	return fmt.Sprintf(`command="%s",%s %s`, command, SshOptions, k.Value)
+	if sslCertDirEnvVar != "" {
+		sslCertDirEnvVar = fmt.Sprintf(`%s `, sslCertDirEnvVar)
+	}
+
+	return fmt.Sprintf(`command="%s%s",%s %s`, sslCertDirEnvVar, command, SshOptions, k.Value)
+}
+
+func (k *KeyLine) sslCertDirEnvVar() string {
+	if k.Config.SslCertDir != "" {
+		return fmt.Sprintf(`SSL_CERT_DIR=%s`, k.Config.SslCertDir)
+	}
+
+	return ""
 }
 
-func newKeyLine(id string, value string, prefix string, rootDir string) (*KeyLine, error) {
+func newKeyLine(id, value, prefix string, config *config.Config) (*KeyLine, error) {
 	if err := validate(id, value); err != nil {
 		return nil, err
 	}
 
-	return &KeyLine{Id: id, Value: value, Prefix: prefix, RootDir: rootDir}, nil
+	return &KeyLine{Id: id, Value: value, Prefix: prefix, Config: config}, nil
 }
 
 func validate(id string, value string) error {
diff --git a/internal/keyline/key_line_test.go b/internal/keyline/key_line_test.go
index c6883c0..095de78 100644
--- a/internal/keyline/key_line_test.go
+++ b/internal/keyline/key_line_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
 )
 
 func TestFailingNewPublicKeyLine(t *testing.T) {
@@ -29,7 +30,7 @@ func TestFailingNewPublicKeyLine(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			result, err := NewPublicKeyLine(tc.id, tc.publicKey, "root-dir")
+			result, err := NewPublicKeyLine(tc.id, tc.publicKey, &config.Config{RootDir: "/tmp", SslCertDir: "/tmp/certs"})
 
 			require.Empty(t, result)
 			require.EqualError(t, err, tc.expectedError)
@@ -60,7 +61,7 @@ func TestFailingNewPrincipalKeyLine(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			result, err := NewPrincipalKeyLine(tc.keyId, tc.principal, "root-dir")
+			result, err := NewPrincipalKeyLine(tc.keyId, tc.principal, &config.Config{RootDir: "/tmp", SslCertDir: "/tmp/certs"})
 
 			require.Empty(t, result)
 			require.EqualError(t, err, tc.expectedError)
@@ -69,14 +70,37 @@ func TestFailingNewPrincipalKeyLine(t *testing.T) {
 }
 
 func TestToString(t *testing.T) {
-	keyLine := &KeyLine{
-		Id:      "1",
-		Value:   "public-key",
-		Prefix:  "key",
-		RootDir: "/tmp",
+	testCases := []struct {
+		desc           string
+		keyLine        *KeyLine
+		expectedOutput string
+	}{
+		{
+			desc: "Without SSL cert dir",
+			keyLine: &KeyLine{
+				Id:     "1",
+				Value:  "public-key",
+				Prefix: "key",
+				Config: &config.Config{RootDir: "/tmp"},
+			},
+			expectedOutput: `command="/tmp/bin/gitlab-shell key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key`,
+		},
+		{
+			desc: "With SSL cert dir",
+			keyLine: &KeyLine{
+				Id:     "1",
+				Value:  "public-key",
+				Prefix: "key",
+				Config: &config.Config{RootDir: "/tmp", SslCertDir: "/tmp/certs"},
+			},
+			expectedOutput: `command="SSL_CERT_DIR=/tmp/certs /tmp/bin/gitlab-shell key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key`,
+		},
 	}
 
-	result := keyLine.ToString()
-
-	require.Equal(t, `command="/tmp/bin/gitlab-shell key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key`, result)
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			result := tc.keyLine.ToString()
+			require.Equal(t, tc.expectedOutput, result)
+		})
+	}
 }