Merge branch 'pb-spike-gssapi-lib-per-server' into 'main'

Load gssapi lib per server/connection

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



Merged-by: Igor Drozdov <idrozdov@gitlab.com>
Approved-by: Ash McKenzie <amckenzie@gitlab.com>
Approved-by: Igor Drozdov <idrozdov@gitlab.com>
Reviewed-by: Igor Drozdov <idrozdov@gitlab.com>
Co-authored-by: Patrick Bajao <ebajao@gitlab.com>

cmd/gitlab-sshd/main.go

@@ -74,8 +74,6 @@ func main() {
 
 	cfg.GitalyClient.InitSidechannelRegistry(ctx)
 
-	sshd.LoadGSSAPILib(&cfg.Server.GSSAPI)
-
 	server, err := sshd.NewServer(cfg)
 	if err != nil {
 		log.WithError(err).Fatal("Failed to start GitLab built-in sshd")

internal/sshd/gssapi.go

@@ -13,10 +13,23 @@ import (
 	"gitlab.com/gitlab-org/labkit/log"
 )
 
-var lib *gssapi.Lib
+func NewGSSAPIServer(c *config.GSSAPIConfig) (*OSGSSAPIServer, error) {
+	lib, err := loadGSSAPILib(c)
+	if err != nil {
+		return nil, err
+	}
+
+	s := &OSGSSAPIServer{
+		ServicePrincipalName: c.ServicePrincipalName,
+		lib:                  lib,
+	}
+
+	return s, nil
+}
 
-func LoadGSSAPILib(config *config.GSSAPIConfig) error {
+func loadGSSAPILib(config *config.GSSAPIConfig) (*gssapi.Lib, error) {
 	var err error
+	var lib *gssapi.Lib
 
 	if config.Enabled {
 		options := &gssapi.Options{
@@ -35,7 +48,7 @@ func LoadGSSAPILib(config *config.GSSAPIConfig) error {
 		}
 	}
 
-	return err
+	return lib, err
 }
 
 type OSGSSAPIServer struct {
@@ -43,17 +56,18 @@ type OSGSSAPIServer struct {
 	ServicePrincipalName string
 
 	mutex     sync.RWMutex
+	lib       *gssapi.Lib
 	contextId *gssapi.CtxId
 }
 
-func (_ *OSGSSAPIServer) str2name(str string) (*gssapi.Name, error) {
-	strBuffer, err := lib.MakeBufferString(str)
+func (server *OSGSSAPIServer) str2name(str string) (*gssapi.Name, error) {
+	strBuffer, err := server.lib.MakeBufferString(str)
 	if err != nil {
 		return nil, err
 	}
 	defer strBuffer.Release()
 
-	return strBuffer.Name(lib.GSS_C_NO_OID)
+	return strBuffer.Name(server.lib.GSS_C_NO_OID)
 }
 
 func (server *OSGSSAPIServer) AcceptSecContext(
@@ -67,13 +81,13 @@ func (server *OSGSSAPIServer) AcceptSecContext(
 	server.mutex.Lock()
 	defer server.mutex.Unlock()
 
-	tokenBuffer, err := lib.MakeBufferBytes(token)
+	tokenBuffer, err := server.lib.MakeBufferBytes(token)
 	if err != nil {
 		return
 	}
 	defer tokenBuffer.Release()
 
-	var spn *gssapi.CredId = lib.GSS_C_NO_CREDENTIAL
+	var spn *gssapi.CredId = server.lib.GSS_C_NO_CREDENTIAL
 	if server.ServicePrincipalName != "" {
 		var name *gssapi.Name
 		name, err = server.str2name(server.ServicePrincipalName)
@@ -83,7 +97,7 @@ func (server *OSGSSAPIServer) AcceptSecContext(
 		defer name.Release()
 
 		var actualMech *gssapi.OIDSet
-		spn, actualMech, _, err = lib.AcquireCred(name, 0, lib.GSS_C_NO_OID_SET, gssapi.GSS_C_ACCEPT)
+		spn, actualMech, _, err = server.lib.AcquireCred(name, 0, server.lib.GSS_C_NO_OID_SET, gssapi.GSS_C_ACCEPT)
 		if err != nil {
 			return
 		}
@@ -91,7 +105,7 @@ func (server *OSGSSAPIServer) AcceptSecContext(
 		defer actualMech.Release()
 	}
 
-	ctxOut, srcNameName, _, outputTokenBuffer, _, _, _, err := lib.AcceptSecContext(
+	ctxOut, srcNameName, _, outputTokenBuffer, _, _, _, err := server.lib.AcceptSecContext(
 		server.contextId,
 		spn,
 		tokenBuffer,
@@ -123,12 +137,12 @@ func (server *OSGSSAPIServer) VerifyMIC(
 		return fmt.Errorf("gssapi: uninitialized contextId")
 	}
 
-	micFieldBuffer, err := lib.MakeBufferBytes(micField)
+	micFieldBuffer, err := server.lib.MakeBufferBytes(micField)
 	if err != nil {
 		return err
 	}
 	defer micFieldBuffer.Release()
-	micTokenBuffer, err := lib.MakeBufferBytes(micToken)
+	micTokenBuffer, err := server.lib.MakeBufferBytes(micToken)
 	if err != nil {
 		return err
 	}
@@ -149,7 +163,7 @@ func (server *OSGSSAPIServer) DeleteSecContext() error {
 
 	err := server.contextId.DeleteSecContext()
 	if err == nil {
-		server.contextId = nil
+		server.contextId = server.lib.GSS_C_NO_CONTEXT
 	}
 	return err
 }

internal/sshd/gssapi_test.go

@@ -10,20 +10,21 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
 )
 
-func TestLoadGSSAPILibSucces(t *testing.T) {
-	config := &config.GSSAPIConfig{Enabled: true}
-	err := LoadGSSAPILib(config)
+func NewGSSAPIServerSuccess(t *testing.T) {
+	config := &config.GSSAPIConfig{Enabled: true, ServicePrincipalName: "host/test@TEST.TEST"}
+	s, err := NewGSSAPIServer(config)
 
-	require.NotNil(t, lib)
+	require.NotNil(t, s)
+	require.NotNil(t, s.lib)
 	require.Nil(t, err)
 	require.True(t, config.Enabled)
 }
 
-func TestLoadGSSAPILibFailure(t *testing.T) {
-	config := &config.GSSAPIConfig{Enabled: true, LibPath: "/invalid"}
-	err := LoadGSSAPILib(config)
+func NewGSSAPIServerFailure(t *testing.T) {
+	config := &config.GSSAPIConfig{Enabled: true, LibPath: "/invalid", ServicePrincipalName: "host/test@TEST.TEST"}
+	s, err := NewGSSAPIServer(config)
 
-	require.Nil(t, lib)
+	require.Nil(t, s)
 	require.NotNil(t, err)
 	require.False(t, config.Enabled)
 }

internal/sshd/gssapi_unsupported.go

@@ -6,16 +6,14 @@ import (
 	"errors"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
-
-	"gitlab.com/gitlab-org/labkit/log"
 )
 
-func LoadGSSAPILib(c *config.GSSAPIConfig) error {
-	if c.Enabled {
-		log.New().Error("gssapi-with-mic disabled, built without CGO")
-		c.Enabled = false
+func NewGSSAPIServer(c *config.GSSAPIConfig) (*OSGSSAPIServer, error) {
+	s := &OSGSSAPIServer{
+		ServicePrincipalName: c.ServicePrincipalName,
 	}
-	return nil
+
+	return s, nil
 }
 
 type OSGSSAPIServer struct {

internal/sshd/server_config.go

@@ -211,22 +211,24 @@ func (s *serverConfig) handleUserCertificate(ctx context.Context, user string, c
 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{
-				ServicePrincipalName: s.cfg.Server.GSSAPI.ServicePrincipalName,
-			},
+		gssApiServer, _ := NewGSSAPIServer(&s.cfg.Server.GSSAPI)
+
+		if gssApiServer != nil {
+			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: gssApiServer,
+			}
 		}
 	}