Merge branch 'id-ssh-certificates-restriction' into 'main'

Allow only git commands for auth via SSH certs

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/864



Merged-by: Ash McKenzie <amckenzie@gitlab.com>
Approved-by: Alejandro Rodríguez <alejandro@gitlab.com>
Approved-by: Ash McKenzie <amckenzie@gitlab.com>
Co-authored-by: Igor Drozdov <idrozdov@gitlab.com>

cmd/gitlab-shell/command/command.go

@@ -64,6 +64,22 @@ func NewWithUsername(gitlabUsername string, env sshenv.Env, config *config.Confi
 		return nil, err
 	}
 
+	// When 1.21+ only Golang is supported, it can be refactored by using slices.Contains
+	if env.NamespacePath != "" {
+		exists := false
+		for _, gitCmd := range commandargs.GitCommands {
+			if args.CommandType == gitCmd {
+				exists = true
+
+				break
+			}
+		}
+
+		if !exists {
+			return nil, disallowedcommand.Error
+		}
+	}
+
 	args.GitlabUsername = gitlabUsername
 	if cmd := Build(args, config, readWriter); cmd != nil {
 		return cmd, nil

cmd/gitlab-shell/command/command_test.go

@@ -302,14 +302,80 @@ func TestParseFailure(t *testing.T) {
 }
 
 func TestNewWithUsername(t *testing.T) {
-	env := sshenv.Env{IsSSHConnection: true, OriginalCommand: "git-receive-pack 'group/repo'"}
-	c, err := cmd.NewWithUsername("username", env, nil, nil)
-	require.NoError(t, err)
-	require.IsType(t, &receivepack.Command{}, c)
-	require.Equal(t, c.(*receivepack.Command).Args.GitlabUsername, "username")
+	tests := []struct {
+		desc         string
+		command      string
+		namespace    string
+		expectedErr  error
+		expectedType interface{}
+	}{
+		{
+			desc:        "valid command",
+			command:     "git-receive-pack 'group/repo'",
+			expectedErr: nil,
+			expectedType: &receivepack.Command{
+				Args: &commandargs.Shell{
+					CommandType:    commandargs.ReceivePack,
+					GitlabUsername: "username",
+					SshArgs:        []string{"git-receive-pack", "group/repo"},
+					Env: sshenv.Env{
+						IsSSHConnection: true,
+						OriginalCommand: "git-receive-pack 'group/repo'",
+					},
+				},
+			},
+		}, {
+			desc:        "valid non-git command",
+			command:     "2fa_recovery_codes",
+			expectedErr: nil,
+			expectedType: &twofactorrecover.Command{
+				Args: &commandargs.Shell{
+					CommandType:    commandargs.TwoFactorRecover,
+					GitlabUsername: "username",
+					SshArgs:        []string{"2fa_recovery_codes"},
+					Env: sshenv.Env{
+						IsSSHConnection: true,
+						OriginalCommand: "2fa_recovery_codes",
+					},
+				},
+			},
+		}, {
+			desc:         "invalid command",
+			command:      "invalid",
+			expectedErr:  disallowedcommand.Error,
+			expectedType: nil,
+		}, {
+			desc:        "git command with namespace",
+			command:     "git-receive-pack 'group/repo'",
+			namespace:   "group",
+			expectedErr: nil,
+			expectedType: &receivepack.Command{
+				Args: &commandargs.Shell{
+					CommandType:    commandargs.ReceivePack,
+					GitlabUsername: "username",
+					SshArgs:        []string{"git-receive-pack", "group/repo"},
+					Env: sshenv.Env{
+						IsSSHConnection: true,
+						OriginalCommand: "git-receive-pack 'group/repo'",
+						NamespacePath:   "group",
+					},
+				},
+			},
+		}, {
+			desc:         "non-git command with namespace",
+			command:      "2fa_recovery_codes",
+			namespace:    "group",
+			expectedErr:  disallowedcommand.Error,
+			expectedType: nil,
+		},
+	}
 
-	env = sshenv.Env{IsSSHConnection: true, OriginalCommand: "invalid"}
-	c, err = cmd.NewWithUsername("username", env, nil, nil)
-	require.Error(t, err)
-	require.Nil(t, c)
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			env := sshenv.Env{IsSSHConnection: true, OriginalCommand: tc.command, NamespacePath: tc.namespace}
+			c, err := cmd.NewWithUsername("username", env, nil, nil)
+			require.IsType(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedType, c)
+		})
+	}
 }

internal/command/commandargs/shell.go

@@ -23,6 +23,8 @@ const (
 var (
 	whoKeyRegex      = regexp.MustCompile(`\Akey-(?P<keyid>\d+)\z`)
 	whoUsernameRegex = regexp.MustCompile(`\Ausername-(?P<username>\S+)\z`)
+
+	GitCommands = []CommandType{LfsAuthenticate, UploadPack, ReceivePack, UploadArchive}
 )
 
 type Shell struct {