auth: respect default tls.verify_server_hostname=false

In Nomad 1.7.0, we refactored much of the authentication code to eliminate nil
ACLs and create "virtual" ACL objects that can be used to reduce the risk of
fail-open security bugs. In doing so, we accidentally dropped support for the
default `tls.verify_server_hostname=false` option.

Fix the bug by including the field in the set of conditions we check for the TLS
argument we pass into the constructor (this keeps "policy" separate from
"mechanism" in the auth code and reduces the number of dimensions we need to
test). Change the field name in the Authenticator to better match the intent.

.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
+```

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,15 +63,15 @@ type AuthenticatorConfig struct {
 	Logger         hclog.Logger
 	GetLeaderACLFn LeaderACLGetter
 	AclsEnabled    bool
-	TLSEnabled     bool
+	VerifyTLS      bool
 	Region         string
 	Encrypter      Encrypter
 }
 
 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")

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,15 +462,15 @@ 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{
 			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,
 		})
@@ -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,
 	})

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,
 	})