diff --git a/.changelog/19425.txt b/.changelog/19425.txt new file mode 100644 index 000000000..5cfc85173 --- /dev/null +++ b/.changelog/19425.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth: Fixed a bug where `tls.verify_server_hostname=false` was not respected, leading to authentication failures between Nomad agents +``` diff --git a/nomad/auth/auth.go b/nomad/auth/auth.go index 3d05553bc..fa79385f7 100644 --- a/nomad/auth/auth.go +++ b/nomad/auth/auth.go @@ -41,7 +41,7 @@ type Encrypter interface { type Authenticator struct { aclsEnabled bool - tlsEnabled bool + verifyTLS bool logger hclog.Logger getState StateGetter getLeaderACL LeaderACLGetter @@ -63,7 +63,7 @@ type AuthenticatorConfig struct { Logger hclog.Logger GetLeaderACLFn LeaderACLGetter AclsEnabled bool - TLSEnabled bool + VerifyTLS bool Region string Encrypter Encrypter } @@ -71,7 +71,7 @@ type AuthenticatorConfig struct { func NewAuthenticator(cfg *AuthenticatorConfig) *Authenticator { return &Authenticator{ aclsEnabled: cfg.AclsEnabled, - tlsEnabled: cfg.TLSEnabled, + verifyTLS: cfg.VerifyTLS, logger: cfg.Logger.With("auth"), getState: cfg.StateFn, getLeaderACL: cfg.GetLeaderACLFn, @@ -251,7 +251,7 @@ func (s *Authenticator) AuthenticateServerOnly(ctx RPCContext, args structs.Requ identity := &structs.AuthenticatedIdentity{RemoteIP: remoteIP} defer args.SetIdentity(identity) // always set the identity, even on errors - if s.tlsEnabled && !ctx.IsStatic() { + if s.verifyTLS && !ctx.IsStatic() { tlsCert := ctx.Certificate() if tlsCert == nil { return nil, errors.New("missing certificate information") @@ -294,7 +294,7 @@ func (s *Authenticator) AuthenticateClientOnly(ctx RPCContext, args structs.Requ identity := &structs.AuthenticatedIdentity{RemoteIP: remoteIP} defer args.SetIdentity(identity) // always set the identity, even on errors - if s.tlsEnabled && !ctx.IsStatic() { + if s.verifyTLS && !ctx.IsStatic() { tlsCert := ctx.Certificate() if tlsCert == nil { return nil, errors.New("missing certificate information") @@ -342,7 +342,7 @@ func (s *Authenticator) AuthenticateClientOnlyLegacy(ctx RPCContext, args struct identity := &structs.AuthenticatedIdentity{RemoteIP: remoteIP} defer args.SetIdentity(identity) // always set the identity, even on errors - if s.tlsEnabled && !ctx.IsStatic() { + if s.verifyTLS && !ctx.IsStatic() { tlsCert := ctx.Certificate() if tlsCert == nil { return nil, errors.New("missing certificate information") diff --git a/nomad/auth/auth_test.go b/nomad/auth/auth_test.go index 22ab2c727..c43c8152b 100644 --- a/nomad/auth/auth_test.go +++ b/nomad/auth/auth_test.go @@ -33,14 +33,14 @@ func TestAuthenticateDefault(t *testing.T) { ci.Parallel(t) testAuthenticator := func(t *testing.T, store *state.StateStore, - hasACLs, hasTLS bool) *Authenticator { + hasACLs, verifyTLS bool) *Authenticator { leaderACL := uuid.Generate() return NewAuthenticator(&AuthenticatorConfig{ StateFn: func() *state.StateStore { return store }, Logger: testlog.HCLogger(t), GetLeaderACLFn: func() string { return leaderACL }, AclsEnabled: hasACLs, - TLSEnabled: hasTLS, + VerifyTLS: verifyTLS, Region: "global", Encrypter: newTestEncrypter(), }) @@ -367,14 +367,14 @@ func TestAuthenticateServerOnly(t *testing.T) { ci.Parallel(t) testAuthenticator := func(t *testing.T, store *state.StateStore, - hasACLs, hasTLS bool) *Authenticator { + hasACLs, verifyTLS bool) *Authenticator { leaderACL := uuid.Generate() return NewAuthenticator(&AuthenticatorConfig{ StateFn: func() *state.StateStore { return store }, Logger: testlog.HCLogger(t), GetLeaderACLFn: func() string { return leaderACL }, AclsEnabled: hasACLs, - TLSEnabled: hasTLS, + VerifyTLS: verifyTLS, Region: "global", Encrypter: nil, }) @@ -400,6 +400,22 @@ func TestAuthenticateServerOnly(t *testing.T) { must.True(t, aclObj.AllowServerOp()) }, }, + { + name: "no mTLS but client cert", + testFn: func(t *testing.T) { + ctx := newTestContext(t, "client.global.nomad", "192.168.1.1") + args := &structs.GenericRequest{} + + store := testStateStore(t) + auth := testAuthenticator(t, store, true, false) + + aclObj, err := auth.AuthenticateServerOnly(ctx, args) + must.NoError(t, err) + must.NotNil(t, aclObj) + must.Eq(t, ":192.168.1.1", args.GetIdentity().String()) + must.True(t, aclObj.AllowServerOp()) + }, + }, { name: "with mTLS but client cert", testFn: func(t *testing.T) { @@ -446,7 +462,7 @@ func TestAuthenticateClientOnly(t *testing.T) { ci.Parallel(t) testAuthenticator := func(t *testing.T, store *state.StateStore, - hasACLs, hasTLS bool) *Authenticator { + hasACLs, verifyTLS bool) *Authenticator { leaderACL := uuid.Generate() return NewAuthenticator(&AuthenticatorConfig{ @@ -454,7 +470,7 @@ func TestAuthenticateClientOnly(t *testing.T) { Logger: testlog.HCLogger(t), GetLeaderACLFn: func() string { return leaderACL }, AclsEnabled: hasACLs, - TLSEnabled: hasTLS, + VerifyTLS: verifyTLS, Region: "global", Encrypter: nil, }) @@ -894,7 +910,7 @@ func TestIdentityToACLClaim(t *testing.T) { Logger: testlog.HCLogger(t), GetLeaderACLFn: func() string { return leaderACL }, AclsEnabled: true, - TLSEnabled: true, + VerifyTLS: true, Region: "global", Encrypter: newTestEncrypter(), }) @@ -1156,7 +1172,7 @@ func testDefaultAuthenticator(t *testing.T) *Authenticator { Logger: testlog.HCLogger(t), GetLeaderACLFn: func() string { return leaderACL }, AclsEnabled: true, - TLSEnabled: true, + VerifyTLS: true, Region: "global", Encrypter: nil, }) diff --git a/nomad/server.go b/nomad/server.go index c748fdb2a..c7e86fd3d 100644 --- a/nomad/server.go +++ b/nomad/server.go @@ -446,7 +446,7 @@ func NewServer(config *Config, consulCatalog consul.CatalogAPI, consulConfigFunc Logger: s.logger, GetLeaderACLFn: s.getLeaderAcl, AclsEnabled: s.config.ACLEnabled, - TLSEnabled: s.config.TLSConfig != nil && s.config.TLSConfig.EnableRPC, + VerifyTLS: s.config.TLSConfig != nil && s.config.TLSConfig.EnableRPC && s.config.TLSConfig.VerifyServerHostname, Region: s.Region(), Encrypter: s.encrypter, })