From 1e5c4568741460f237db6135fcb2936546b04575 Mon Sep 17 00:00:00 2001 From: Marin Hannache Date: Sun, 11 Sep 2022 13:02:56 +0000 Subject: [PATCH 1/6] Add support for the gssapi-with-mic auth method Changelog: added Signed-off-by: Marin Hannache --- cmd/gitlab-shell/command/command.go | 14 +++ config.yml.example | 8 ++ go.mod | 1 + go.sum | 2 + internal/command/commandargs/shell.go | 13 +- internal/config/config.go | 7 ++ internal/gitlabnet/accessverifier/client.go | 17 +-- internal/gitlabnet/discover/client.go | 2 + internal/sshd/gssapi.go | 126 ++++++++++++++++++++ internal/sshd/server_config.go | 24 +++- internal/sshd/session.go | 19 ++- internal/sshd/sshd.go | 11 +- 12 files changed, 220 insertions(+), 24 deletions(-) create mode 100644 internal/sshd/gssapi.go diff --git a/cmd/gitlab-shell/command/command.go b/cmd/gitlab-shell/command/command.go index 08b3af69..b2a02669 100644 --- a/cmd/gitlab-shell/command/command.go +++ b/cmd/gitlab-shell/command/command.go @@ -44,6 +44,20 @@ func NewWithKey(gitlabKeyId string, env sshenv.Env, config *config.Config, readW return nil, disallowedcommand.Error } +func NewWithKrb5Principal(gitlabKrb5Principal string, env sshenv.Env, config *config.Config, readWriter *readwriter.ReadWriter) (command.Command, error) { + args, err := Parse(nil, env) + if err != nil { + return nil, err + } + + args.GitlabKrb5Principal = gitlabKrb5Principal + if cmd := Build(args, config, readWriter); cmd != nil { + return cmd, nil + } + + return nil, disallowedcommand.Error +} + func Parse(arguments []string, env sshenv.Env) (*commandargs.Shell, error) { args := &commandargs.Shell{Arguments: arguments, Env: env} diff --git a/config.yml.example b/config.yml.example index 0154723c..886fde00 100644 --- a/config.yml.example +++ b/config.yml.example @@ -107,3 +107,11 @@ sshd: - /run/secrets/ssh-hostkeys/ssh_host_rsa_key-cert.pub - /run/secrets/ssh-hostkeys/ssh_host_ecdsa_key-cert.pub - /run/secrets/ssh-hostkeys/ssh_host_ed25519_key-cert.pub + # GSSAPI-related settings + gssapi: + # Enable the gssapi-with-mic authentication method. Defaults to false. + enabled: false + # Keytab path. Defaults to "", system default (usually /etc/krb5.keytab). + keytab: "" + # The Kerberos service name to be used by sshd. Defaults to "", accepts any service name in keytab file. + service_principal_name: "" diff --git a/go.mod b/go.mod index 23eecbb6..ae5bc6f7 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 github.com/mattn/go-shellwords v1.0.11 github.com/mikesmitty/edkey v0.0.0-20170222072505-3356ea4e686a + github.com/openshift/gssapi v0.0.0-20161010215902-5fb4217df13b github.com/otiai10/copy v1.4.2 github.com/pires/go-proxyproto v0.6.2 github.com/prometheus/client_golang v1.13.1 diff --git a/go.sum b/go.sum index 29c9560b..64602d8f 100644 --- a/go.sum +++ b/go.sum @@ -271,6 +271,8 @@ github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+W github.com/onsi/ginkgo v1.10.3 h1:OoxbjfXVZyod1fmWYhI7SEyaD8B00ynP3T+D5GiyHOY= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1 h1:K0jcRCwNQM3vFGh1ppMtDh/+7ApJrjldlX8fA0jDTLQ= +github.com/openshift/gssapi v0.0.0-20161010215902-5fb4217df13b h1:it0YPE/evO6/m8t8wxis9KFI2F/aleOKsI6d9uz0cEk= +github.com/openshift/gssapi v0.0.0-20161010215902-5fb4217df13b/go.mod h1:tNrEB5k8SI+g5kOlsCmL2ELASfpqEofI0+FLBgBdN08= github.com/opentracing/opentracing-go v1.0.2/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs= diff --git a/internal/command/commandargs/shell.go b/internal/command/commandargs/shell.go index 0b1e161b..616d7e34 100644 --- a/internal/command/commandargs/shell.go +++ b/internal/command/commandargs/shell.go @@ -26,12 +26,13 @@ var ( ) type Shell struct { - Arguments []string - GitlabUsername string - GitlabKeyId string - SshArgs []string - CommandType CommandType - Env sshenv.Env + Arguments []string + GitlabUsername string + GitlabKeyId string + GitlabKrb5Principal string + SshArgs []string + CommandType CommandType + Env sshenv.Env } func (s *Shell) Parse() error { diff --git a/internal/config/config.go b/internal/config/config.go index 35d8e742..d6426162 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,6 +23,12 @@ const ( type YamlDuration time.Duration +type GSSAPIConfig struct { + Enabled bool `yaml:"enabled,omitempty"` + Keytab string `yaml:"keytab,omitempty"` + ServicePrincipalName string `yaml:"service_principal_name,omitempty"` +} + type ServerConfig struct { Listen string `yaml:"listen,omitempty"` ProxyProtocol bool `yaml:"proxy_protocol,omitempty"` @@ -41,6 +47,7 @@ type ServerConfig struct { MACs []string `yaml:"macs"` KexAlgorithms []string `yaml:"kex_algorithms"` Ciphers []string `yaml:"ciphers"` + GSSAPI GSSAPIConfig `yaml:"gssapi,omitempty"` } type HttpSettingsConfig struct { diff --git a/internal/gitlabnet/accessverifier/client.go b/internal/gitlabnet/accessverifier/client.go index ca81ad1e..8554d9bd 100644 --- a/internal/gitlabnet/accessverifier/client.go +++ b/internal/gitlabnet/accessverifier/client.go @@ -22,13 +22,14 @@ type Client struct { } type Request struct { - Action commandargs.CommandType `json:"action"` - Repo string `json:"project"` - Changes string `json:"changes"` - Protocol string `json:"protocol"` - KeyId string `json:"key_id,omitempty"` - Username string `json:"username,omitempty"` - CheckIp string `json:"check_ip,omitempty"` + Action commandargs.CommandType `json:"action"` + Repo string `json:"project"` + Changes string `json:"changes"` + Protocol string `json:"protocol"` + KeyId string `json:"key_id,omitempty"` + Username string `json:"username,omitempty"` + Krb5Principal string `json:"krb5principal,omitempty"` + CheckIp string `json:"check_ip,omitempty"` } type Gitaly struct { @@ -81,6 +82,8 @@ func (c *Client) Verify(ctx context.Context, args *commandargs.Shell, action com if args.GitlabUsername != "" { request.Username = args.GitlabUsername + } else if args.GitlabKrb5Principal != "" { + request.Krb5Principal = args.GitlabKrb5Principal } else { request.KeyId = args.GitlabKeyId } diff --git a/internal/gitlabnet/discover/client.go b/internal/gitlabnet/discover/client.go index 1e7d33fa..75b904bf 100644 --- a/internal/gitlabnet/discover/client.go +++ b/internal/gitlabnet/discover/client.go @@ -38,6 +38,8 @@ func (c *Client) GetByCommandArgs(ctx context.Context, args *commandargs.Shell) params.Add("username", args.GitlabUsername) } else if args.GitlabKeyId != "" { params.Add("key_id", args.GitlabKeyId) + } else if args.GitlabKrb5Principal != "" { + params.Add("krb5principal", args.GitlabKrb5Principal) } else { // There was no 'who' information, this matches the ruby error // message. diff --git a/internal/sshd/gssapi.go b/internal/sshd/gssapi.go new file mode 100644 index 00000000..ae15f9f7 --- /dev/null +++ b/internal/sshd/gssapi.go @@ -0,0 +1,126 @@ +package sshd + +import ( + "fmt" + + "github.com/openshift/gssapi" +) + +var lib *gssapi.Lib + +type OSGSSAPIServer struct { + Keytab string + ServicePrincipalName string + + contextId *gssapi.CtxId +} + +func load(options *gssapi.Options) (err error) { + lib, err = gssapi.Load(options) + return +} + +func str2name(str string) (*gssapi.Name, error) { + strBuffer, err := lib.MakeBufferString(str) + if err != nil { + return nil, err + } + defer strBuffer.Release() + + return strBuffer.Name(lib.GSS_C_NO_OID) +} + +func (server *OSGSSAPIServer) AcceptSecContext( + token []byte, +) ( + outputToken []byte, + srcName string, + needContinue bool, + err error, +) { + if lib == nil { + if err = load(&gssapi.Options{Krb5Ktname: server.Keytab}); err != nil { + return + } + } + + tokenBuffer, err := lib.MakeBufferBytes(token) + if err != nil { + return + } + defer tokenBuffer.Release() + + var spn *gssapi.CredId = lib.GSS_C_NO_CREDENTIAL + if server.ServicePrincipalName != "" { + var name *gssapi.Name + name, err = str2name(server.ServicePrincipalName) + if err != nil { + return + } + defer name.Release() + + var actualMech *gssapi.OIDSet + spn, actualMech, _, err = lib.AcquireCred(name, 0, lib.GSS_C_NO_OID_SET, gssapi.GSS_C_ACCEPT) + if err != nil { + return + } + defer spn.Release() + defer actualMech.Release() + } + + ctxOut, srcNameName, _, outputTokenBuffer, _, _, _, err := lib.AcceptSecContext( + server.contextId, + spn, + tokenBuffer, + nil, + ) + if err == gssapi.ErrContinueNeeded { + needContinue = true + err = nil + } else if err != nil { + return + } + defer outputTokenBuffer.Release() + defer srcNameName.Release() + + outputToken = outputTokenBuffer.Bytes() + server.contextId = ctxOut + + return outputToken, srcNameName.String(), needContinue, err +} + +func (server *OSGSSAPIServer) VerifyMIC( + micField []byte, + micToken []byte, +) error { + if server.contextId == nil { + return fmt.Errorf("gssapi: uninitialized contextId") + } + + micFieldBuffer, err := lib.MakeBufferBytes(micField) + if err != nil { + return err + } + defer micFieldBuffer.Release() + micTokenBuffer, err := lib.MakeBufferBytes(micToken) + if err != nil { + return err + } + defer micTokenBuffer.Release() + + _, err = server.contextId.VerifyMIC(micFieldBuffer, micTokenBuffer) + return err + +} + +func (server *OSGSSAPIServer) DeleteSecContext() error { + if server.contextId == nil { + return nil + } + + err := server.contextId.DeleteSecContext() + if err == nil { + server.contextId = nil + } + return err +} diff --git a/internal/sshd/server_config.go b/internal/sshd/server_config.go index c8b1e54e..48fe7c9d 100644 --- a/internal/sshd/server_config.go +++ b/internal/sshd/server_config.go @@ -146,6 +146,27 @@ func (s *serverConfig) getAuthKey(ctx context.Context, user string, key ssh.Publ } func (s *serverConfig) get(ctx context.Context) *ssh.ServerConfig { + var gssapiWithMICConfig *ssh.GSSAPIWithMICConfig + if s.cfg.Server.GSSAPI.Enabled { + gssapiWithMICConfig = &ssh.GSSAPIWithMICConfig{ + AllowLogin: func(conn ssh.ConnMetadata, srcName string) (*ssh.Permissions, error) { + if conn.User() != s.cfg.User { + return nil, fmt.Errorf("unknown user") + } + + return &ssh.Permissions{ + // Record the Kerberos principal used for authentication. + Extensions: map[string]string{ + "krb5principal": srcName, + }, + }, nil + }, + Server: &OSGSSAPIServer{ + Keytab: s.cfg.Server.GSSAPI.Keytab, + ServicePrincipalName: s.cfg.Server.GSSAPI.ServicePrincipalName, + }, + } + } sshCfg := &ssh.ServerConfig{ PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { res, err := s.getAuthKey(ctx, conn.User(), key) @@ -160,7 +181,8 @@ func (s *serverConfig) get(ctx context.Context) *ssh.ServerConfig { }, }, nil }, - ServerVersion: "SSH-2.0-GitLab-SSHD", + GSSAPIWithMICConfig: gssapiWithMICConfig, + ServerVersion: "SSH-2.0-GitLab-SSHD", } if len(s.cfg.Server.MACs) > 0 { diff --git a/internal/sshd/session.go b/internal/sshd/session.go index 48cd86a6..3394b2a5 100644 --- a/internal/sshd/session.go +++ b/internal/sshd/session.go @@ -13,6 +13,7 @@ import ( grpcstatus "google.golang.org/grpc/status" shellCmd "gitlab.com/gitlab-org/gitlab-shell/v14/cmd/gitlab-shell/command" + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/shared/disallowedcommand" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" @@ -23,10 +24,11 @@ import ( type session struct { // State set up by the connection - cfg *config.Config - channel ssh.Channel - gitlabKeyId string - remoteAddr string + cfg *config.Config + channel ssh.Channel + gitlabKeyId string + gitlabKrb5Principal string + remoteAddr string // State managed by the session execCmd string @@ -166,7 +168,14 @@ func (s *session) handleShell(ctx context.Context, req *ssh.Request) (uint32, er ErrOut: s.channel.Stderr(), } - cmd, err := shellCmd.NewWithKey(s.gitlabKeyId, env, s.cfg, rw) + var cmd command.Command + var err error + + if s.gitlabKrb5Principal != "" { + cmd, err = shellCmd.NewWithKrb5Principal(s.gitlabKrb5Principal, env, s.cfg, rw) + } else { + cmd, err = shellCmd.NewWithKey(s.gitlabKeyId, env, s.cfg, rw) + } if err != nil { if errors.Is(err, disallowedcommand.Error) { s.toStderr(ctx, "ERROR: Unknown command: %v\n", s.execCmd) diff --git a/internal/sshd/sshd.go b/internal/sshd/sshd.go index d20286a7..fbb50523 100644 --- a/internal/sshd/sshd.go +++ b/internal/sshd/sshd.go @@ -194,11 +194,12 @@ func (s *Server) handleConn(ctx context.Context, nconn net.Conn) { conn := newConnection(s.Config, nconn) conn.handle(ctx, s.serverConfig.get(ctx), func(sconn *ssh.ServerConn, channel ssh.Channel, requests <-chan *ssh.Request) error { session := &session{ - cfg: s.Config, - channel: channel, - gitlabKeyId: sconn.Permissions.Extensions["key-id"], - remoteAddr: remoteAddr, - started: time.Now(), + cfg: s.Config, + channel: channel, + gitlabKeyId: sconn.Permissions.Extensions["key-id"], + gitlabKrb5Principal: sconn.Permissions.Extensions["krb5principal"], + remoteAddr: remoteAddr, + started: time.Now(), } return session.handle(ctx, requests) -- GitLab From 6c68c7ff89a3f802f936ec04767a7fc56c5ae8c9 Mon Sep 17 00:00:00 2001 From: Marin Hannache Date: Sun, 11 Sep 2022 17:46:03 +0000 Subject: [PATCH 2/6] Add GSSAPI lib to the CI testing environment Signed-off-by: Marin Hannache --- .gitlab-ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index de0975c8..f8aa2802 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -76,6 +76,7 @@ code_navigation: image: sourcegraph/lsif-go:v1.9 allow_failure: true script: + - apt-get update -qq && apt-get install -y libkrb5-dev - lsif-go artifacts: reports: -- GitLab From f60fc085b71a7cd1c843949bfa8f97aac0e215c8 Mon Sep 17 00:00:00 2001 From: Marin Hannache Date: Fri, 16 Sep 2022 19:57:26 +0000 Subject: [PATCH 3/6] Improve test coverage Signed-off-by: Marin Hannache --- .../gitlabnet/accessverifier/client_test.go | 10 +++- internal/gitlabnet/discover/client_test.go | 17 +++++++ internal/sshd/gssapi.go | 21 ++++----- internal/sshd/server_config_test.go | 46 +++++++++++++++++++ internal/sshd/session_test.go | 36 +++++++++------ 5 files changed, 102 insertions(+), 28 deletions(-) diff --git a/internal/gitlabnet/accessverifier/client_test.go b/internal/gitlabnet/accessverifier/client_test.go index 6a187f28..f2c88a58 100644 --- a/internal/gitlabnet/accessverifier/client_test.go +++ b/internal/gitlabnet/accessverifier/client_test.go @@ -56,7 +56,7 @@ func buildExpectedResponse(who string) *Response { func TestSuccessfulResponses(t *testing.T) { okResponse := testResponse{body: responseBody(t, "allowed.json"), status: http.StatusOK} client := setup(t, - map[string]testResponse{"first": okResponse}, + map[string]testResponse{"first": okResponse, "test@TEST.TEST": okResponse}, map[string]testResponse{"1": okResponse}, ) @@ -73,6 +73,10 @@ func TestSuccessfulResponses(t *testing.T) { desc: "Provide username within the request", args: &commandargs.Shell{GitlabUsername: "first"}, who: "user-1", + }, { + desc: "Provide krb5principal within the request", + args: &commandargs.Shell{GitlabKrb5Principal: "test@TEST.TEST"}, + who: "user-1", }, } @@ -255,6 +259,10 @@ func setup(t *testing.T, userResponses, keyResponses map[string]testResponse) *C w.WriteHeader(tr.status) _, err := w.Write(tr.body) require.NoError(t, err) + } else if tr, ok := userResponses[requestBody.Krb5Principal]; ok { + w.WriteHeader(tr.status) + _, err := w.Write(tr.body) + require.NoError(t, err) } else if tr, ok := keyResponses[requestBody.KeyId]; ok { w.WriteHeader(tr.status) _, err := w.Write(tr.body) diff --git a/internal/gitlabnet/discover/client_test.go b/internal/gitlabnet/discover/client_test.go index 1506eba6..6208fb2c 100644 --- a/internal/gitlabnet/discover/client_test.go +++ b/internal/gitlabnet/discover/client_test.go @@ -38,6 +38,13 @@ func init() { Name: "Jane Doe", } json.NewEncoder(w).Encode(body) + } else if r.URL.Query().Get("krb5principal") == "john-doe@TEST.TEST" { + body := &Response{ + UserId: 3, + Username: "john-doe", + Name: "John Doe", + } + json.NewEncoder(w).Encode(body) } else if r.URL.Query().Get("username") == "broken_message" { w.WriteHeader(http.StatusForbidden) body := &client.ErrorResponse{ @@ -76,6 +83,16 @@ func TestGetByUsername(t *testing.T) { require.Equal(t, &Response{UserId: 1, Username: "jane-doe", Name: "Jane Doe"}, result) } +func TestGetByKrb5Principal(t *testing.T) { + client := setup(t) + + params := url.Values{} + params.Add("krb5principal", "john-doe@TEST.TEST") + result, err := client.getResponse(context.Background(), params) + require.NoError(t, err) + require.Equal(t, &Response{UserId: 3, Username: "john-doe", Name: "John Doe"}, result) +} + func TestMissingUser(t *testing.T) { client := setup(t) diff --git a/internal/sshd/gssapi.go b/internal/sshd/gssapi.go index ae15f9f7..dc28fa8d 100644 --- a/internal/sshd/gssapi.go +++ b/internal/sshd/gssapi.go @@ -8,6 +8,12 @@ import ( var lib *gssapi.Lib +func load(options *gssapi.Options) error { + var err error + lib, err = gssapi.Load(options) + return err +} + type OSGSSAPIServer struct { Keytab string ServicePrincipalName string @@ -15,12 +21,7 @@ type OSGSSAPIServer struct { contextId *gssapi.CtxId } -func load(options *gssapi.Options) (err error) { - lib, err = gssapi.Load(options) - return -} - -func str2name(str string) (*gssapi.Name, error) { +func (_ *OSGSSAPIServer) str2name(str string) (*gssapi.Name, error) { strBuffer, err := lib.MakeBufferString(str) if err != nil { return nil, err @@ -38,12 +39,6 @@ func (server *OSGSSAPIServer) AcceptSecContext( needContinue bool, err error, ) { - if lib == nil { - if err = load(&gssapi.Options{Krb5Ktname: server.Keytab}); err != nil { - return - } - } - tokenBuffer, err := lib.MakeBufferBytes(token) if err != nil { return @@ -53,7 +48,7 @@ func (server *OSGSSAPIServer) AcceptSecContext( var spn *gssapi.CredId = lib.GSS_C_NO_CREDENTIAL if server.ServicePrincipalName != "" { var name *gssapi.Name - name, err = str2name(server.ServicePrincipalName) + name, err = server.str2name(server.ServicePrincipalName) if err != nil { return } diff --git a/internal/sshd/server_config_test.go b/internal/sshd/server_config_test.go index d98302fe..d8e63709 100644 --- a/internal/sshd/server_config_test.go +++ b/internal/sshd/server_config_test.go @@ -171,6 +171,52 @@ func TestCustomAlgorithms(t *testing.T) { require.Equal(t, customCiphers, sshServerConfig.Ciphers) } +func TestGSSAPIWithMIC(t *testing.T) { + srvCfg := &serverConfig{ + cfg: &config.Config{ + Server: config.ServerConfig{ + GSSAPI: config.GSSAPIConfig{ + Enabled: true, + ServicePrincipalName: "host/test@TEST.TEST", + }, + }, + }, + } + sshServerConfig := srvCfg.get(context.Background()) + server := sshServerConfig.GSSAPIWithMICConfig.Server.(*OSGSSAPIServer) + + require.NotNil(t, sshServerConfig.GSSAPIWithMICConfig) + require.NotNil(t, sshServerConfig.GSSAPIWithMICConfig.AllowLogin) + require.NotNil(t, server) + require.Equal(t, server.ServicePrincipalName, "host/test@TEST.TEST") + + sshServerConfig.SetDefaults() + + require.NotNil(t, sshServerConfig.GSSAPIWithMICConfig) + require.NotNil(t, sshServerConfig.GSSAPIWithMICConfig.AllowLogin) + require.NotNil(t, server) + require.Equal(t, server.ServicePrincipalName, "host/test@TEST.TEST") +} + +func TestGSSAPIWithMICDisabled(t *testing.T) { + srvCfg := &serverConfig{ + cfg: &config.Config{ + Server: config.ServerConfig{ + GSSAPI: config.GSSAPIConfig{ + Enabled: false, + }, + }, + }, + } + sshServerConfig := srvCfg.get(context.Background()) + + require.Nil(t, sshServerConfig.GSSAPIWithMICConfig) + + sshServerConfig.SetDefaults() + + require.Nil(t, sshServerConfig.GSSAPIWithMICConfig) +} + func rsaPublicKey(t *testing.T) ssh.PublicKey { privateKey, err := rsa.GenerateKey(rand.Reader, 2048) require.NoError(t, err) diff --git a/internal/sshd/session_test.go b/internal/sshd/session_test.go index fe43342b..d1bff7e8 100644 --- a/internal/sshd/session_test.go +++ b/internal/sshd/session_test.go @@ -130,21 +130,29 @@ func TestHandleExec(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - out := &bytes.Buffer{} - f := &fakeChannel{stdErr: out} - s := &session{ - gitlabKeyId: "root", - channel: f, - cfg: &config.Config{GitlabUrl: url}, + sessions := []*session{ + { + gitlabKeyId: "root", + cfg: &config.Config{GitlabUrl: url}, + }, + { + gitlabKrb5Principal: "test@TEST.TEST", + cfg: &config.Config{GitlabUrl: url}, + }, + } + for _, s := range sessions { + out := &bytes.Buffer{} + f := &fakeChannel{stdErr: out} + r := &ssh.Request{Payload: tc.payload} + + s.channel = f + shouldContinue, err := s.handleExec(context.Background(), r) + + require.Equal(t, tc.expectedErr, err) + require.Equal(t, false, shouldContinue) + require.Equal(t, tc.sentRequestName, f.sentRequestName) + require.Equal(t, tc.sentRequestPayload, f.sentRequestPayload) } - r := &ssh.Request{Payload: tc.payload} - - shouldContinue, err := s.handleExec(context.Background(), r) - - require.Equal(t, tc.expectedErr, err) - require.Equal(t, false, shouldContinue) - require.Equal(t, tc.sentRequestName, f.sentRequestName) - require.Equal(t, tc.sentRequestPayload, f.sentRequestPayload) }) } } -- GitLab From 1bd438304e2a4b13201422e73cfcd1064050098e Mon Sep 17 00:00:00 2001 From: Marin Hannache Date: Fri, 16 Sep 2022 23:32:47 +0000 Subject: [PATCH 4/6] Disable GSSAPI authentication when lib is not loadable Signed-off-by: Marin Hannache --- cmd/gitlab-sshd/main.go | 12 ++++++++++++ internal/sshd/gssapi.go | 2 +- internal/sshd/gssapi_test.go | 23 +++++++++++++++++++++++ internal/sshd/server_config.go | 1 - 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 internal/sshd/gssapi_test.go diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index 330c25f9..9d771bef 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -8,6 +8,8 @@ import ( "syscall" "time" + "github.com/openshift/gssapi" + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/logger" @@ -71,6 +73,16 @@ func main() { cfg.GitalyClient.InitSidechannelRegistry(ctx) + if cfg.Server.GSSAPI.Enabled { + options := &gssapi.Options{ + Krb5Ktname: cfg.Server.GSSAPI.Keytab, + } + if err := sshd.LoadGSSAPILib(options); err != nil { + log.WithError(err).Error("Unable to load GSSAPI library, gssapi-with-mic is disabled") + cfg.Server.GSSAPI.Enabled = false + } + } + server, err := sshd.NewServer(cfg) if err != nil { log.WithError(err).Fatal("Failed to start GitLab built-in sshd") diff --git a/internal/sshd/gssapi.go b/internal/sshd/gssapi.go index dc28fa8d..fb481680 100644 --- a/internal/sshd/gssapi.go +++ b/internal/sshd/gssapi.go @@ -8,7 +8,7 @@ import ( var lib *gssapi.Lib -func load(options *gssapi.Options) error { +func LoadGSSAPILib(options *gssapi.Options) error { var err error lib, err = gssapi.Load(options) return err diff --git a/internal/sshd/gssapi_test.go b/internal/sshd/gssapi_test.go new file mode 100644 index 00000000..1eae6778 --- /dev/null +++ b/internal/sshd/gssapi_test.go @@ -0,0 +1,23 @@ +package sshd + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/openshift/gssapi" +) + +func TestLoadGSSAPILibSucces(t *testing.T) { + err := LoadGSSAPILib(&gssapi.Options{}) + + require.NotNil(t, lib) + require.Nil(t, err) +} + +func TestLoadGSSAPILibFailure(t *testing.T) { + err := LoadGSSAPILib(&gssapi.Options{LibPath: "/invalid"}) + + require.Nil(t, lib) + require.NotNil(t, err) +} diff --git a/internal/sshd/server_config.go b/internal/sshd/server_config.go index 48fe7c9d..3c1fdbf6 100644 --- a/internal/sshd/server_config.go +++ b/internal/sshd/server_config.go @@ -162,7 +162,6 @@ func (s *serverConfig) get(ctx context.Context) *ssh.ServerConfig { }, nil }, Server: &OSGSSAPIServer{ - Keytab: s.cfg.Server.GSSAPI.Keytab, ServicePrincipalName: s.cfg.Server.GSSAPI.ServicePrincipalName, }, } -- GitLab From d3842c2e2e2ddf938f3532e81ba83aa6e99200ab Mon Sep 17 00:00:00 2001 From: Marin Hannache Date: Thu, 17 Nov 2022 20:47:51 +0000 Subject: [PATCH 5/6] Prefix error messages with function name and improve test coverage Signed-off-by: Marin Hannache --- cmd/gitlab-sshd/main.go | 12 +----------- internal/config/config.go | 1 + internal/sshd/gssapi.go | 25 +++++++++++++++++++++++-- internal/sshd/gssapi_test.go | 10 +++++++--- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index 9d771bef..bc931f61 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -8,8 +8,6 @@ import ( "syscall" "time" - "github.com/openshift/gssapi" - "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/logger" @@ -73,15 +71,7 @@ func main() { cfg.GitalyClient.InitSidechannelRegistry(ctx) - if cfg.Server.GSSAPI.Enabled { - options := &gssapi.Options{ - Krb5Ktname: cfg.Server.GSSAPI.Keytab, - } - if err := sshd.LoadGSSAPILib(options); err != nil { - log.WithError(err).Error("Unable to load GSSAPI library, gssapi-with-mic is disabled") - cfg.Server.GSSAPI.Enabled = false - } - } + sshd.LoadGSSAPILib(&cfg.Server.GSSAPI) server, err := sshd.NewServer(cfg) if err != nil { diff --git a/internal/config/config.go b/internal/config/config.go index d6426162..41f3812e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -27,6 +27,7 @@ type GSSAPIConfig struct { Enabled bool `yaml:"enabled,omitempty"` Keytab string `yaml:"keytab,omitempty"` ServicePrincipalName string `yaml:"service_principal_name,omitempty"` + LibPath string } type ServerConfig struct { diff --git a/internal/sshd/gssapi.go b/internal/sshd/gssapi.go index fb481680..bf65a15e 100644 --- a/internal/sshd/gssapi.go +++ b/internal/sshd/gssapi.go @@ -4,13 +4,34 @@ import ( "fmt" "github.com/openshift/gssapi" + + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" + + "gitlab.com/gitlab-org/labkit/log" ) var lib *gssapi.Lib -func LoadGSSAPILib(options *gssapi.Options) error { +func LoadGSSAPILib(config *config.GSSAPIConfig) error { var err error - lib, err = gssapi.Load(options) + + if config.Enabled { + options := &gssapi.Options{ + Krb5Ktname: config.Keytab, + } + + if config.LibPath != "" { + options.LibPath = config.LibPath + } + + lib, err = gssapi.Load(options) + + if err != nil { + log.WithError(err).Error("Unable to load GSSAPI library, gssapi-with-mic is disabled") + config.Enabled = false + } + } + return err } diff --git a/internal/sshd/gssapi_test.go b/internal/sshd/gssapi_test.go index 1eae6778..f4f19cff 100644 --- a/internal/sshd/gssapi_test.go +++ b/internal/sshd/gssapi_test.go @@ -5,19 +5,23 @@ import ( "github.com/stretchr/testify/require" - "github.com/openshift/gssapi" + "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" ) func TestLoadGSSAPILibSucces(t *testing.T) { - err := LoadGSSAPILib(&gssapi.Options{}) + config := &config.GSSAPIConfig{Enabled: true} + err := LoadGSSAPILib(config) require.NotNil(t, lib) require.Nil(t, err) + require.True(t, config.Enabled) } func TestLoadGSSAPILibFailure(t *testing.T) { - err := LoadGSSAPILib(&gssapi.Options{LibPath: "/invalid"}) + config := &config.GSSAPIConfig{Enabled: true, LibPath: "/invalid"} + err := LoadGSSAPILib(config) require.Nil(t, lib) require.NotNil(t, err) + require.False(t, config.Enabled) } -- GitLab From 5829f940816ff61f10936c76b59d450e104ee546 Mon Sep 17 00:00:00 2001 From: Lee Tickett Date: Wed, 4 Jan 2023 23:39:13 +0000 Subject: [PATCH 6/6] Apply 1 suggestion(s) to 1 file(s) --- .gitlab-ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f8aa2802..de0975c8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -76,7 +76,6 @@ code_navigation: image: sourcegraph/lsif-go:v1.9 allow_failure: true script: - - apt-get update -qq && apt-get install -y libkrb5-dev - lsif-go artifacts: reports: -- GitLab