kemko/nomad
1
0
Fork 0
mirror of https://github.com/kemko/nomad.git synced 2026-01-01 16:05:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

auth: respect default tls.verify_server_hostname=false (#19425)

Browse Source 
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.
This commit is contained in:
Tim Gross
2023-12-11 14:20:13 -05:00
committed by GitHub
parent b1654016c0
commit 7f87ede1e2
4 changed files with 34 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/19425.txt Normal file
View File

@@ -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
```

12
nomad/auth/auth.go
View File

@@ -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")

32
nomad/auth/auth_test.go
View File

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

2
nomad/server.go
View File

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