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

oidc: more tests for client assertions (#25352)

Browse Source 
Co-authored-by: dduzgun-security <deniz.duzgun@hashicorp.com>
This commit is contained in:
Daniel Bennett
2025-03-11 16:56:26 -04:00
committed by GitHub
parent 04db81951f
commit d98d414c7f
5 changed files with 588 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
api/acl.go
View File

@@ -1023,14 +1023,14 @@ const (
// PemCert, PemCertFile may contain an x509 certificate created with
// the Key, used to derive the KeyID. Alternatively, KeyID may be set manually.
//
// PemKeyFile and PemCertFile, if set, must be present on disk on any Nomad
// servers that may become cluster leaders.
// PemKeyFile and PemCertFile, if set, must be an absolute path to a file
// present on disk on any Nomad servers that may become cluster leaders.
type OIDCClientAssertionKey struct {
// PemKey is the private key, in pem format. It is used to sign the JWT.
// Mutually exclusive with PemKeyFile.
PemKey string
// PemKeyFile is the path to a private key on server disk, in pem format.
// It is used to sign the JWT.
// PemKeyFile is an absolute path to a private key on Nomad servers' disk,
// in pem format. It is used to sign the JWT.
// Mutually exclusive with PemKey.
PemKeyFile string
@@ -1057,13 +1057,14 @@ type OIDCClientAssertionKey struct {
// Mutually exclusive with PemCert and PemCertFile.
// Allowed KeyIDHeader values: "kid" (the default)
KeyID string
// PemCert is a certificate, signed by the private key or a CA,
// in pem format. It is used to derive an x5t-style KeyID.
// PemCert is an x509 certificate, signed by the private key or a CA,
// in pem format. It is used to derive an x5t#S256 (or x5t) KeyID.
// Mutually exclusive with PemCertFile and KeyID.
// Allowed KeyIDHeader values: "x5t", "x5t#S256" (default "x5t#S256")
PemCert string
// PemCertFile is a certificate, signed by the private key or a CA,
// on server disk, in pem format. It is used to derive an x5t-style KeyID.
// PemCertFile is an absolute path to an x509 certificate on Nomad servers'
// disk, signed by the private key or a CA, in pem format.
// It is used to derive an x5t#S256 (or x5t) KeyID.
// Mutually exclusive with PemCert and KeyID.
// Allowed KeyIDHeader values: "x5t", "x5t#S256" (default "x5t#S256")
PemCertFile string

2
lib/auth/oidc/client_assertion.go
View File

@@ -153,7 +153,7 @@ func getCassCert(k *structs.OIDCClientAssertionKey) (*x509.Certificate, error) {
}
now := time.Now()
if now.Before(cert.NotBefore) || now.After(cert.NotAfter) {
return nil, errors.New("cert expired or not yet valid")
return nil, errors.New("certificate has expired or is not yet valid")
}
return cert, nil
}

520
lib/auth/oidc/client_assertion_test.go Normal file
View File

@@ -0,0 +1,520 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package oidc
import (
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"encoding/pem"
"math/big"
"os"
"path"
"path/filepath"
"testing"
"time"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/shoenig/test/must"
)
func TestBuildClientAssertionJWT_ClientSecret(t *testing.T) {
tests := []struct {
name string
config *structs.ACLAuthMethodConfig
wantErr bool
expectedErr string
}{
{
name: "valid client secret",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "1234567890abcdefghijklmnopqrstuvwxyz",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceClientSecret,
KeyAlgorithm: "HS256",
Audience: []string{"test-audience"},
ExtraHeaders: map[string]string{
"test-header": "test-value",
},
},
},
wantErr: false,
},
{
name: "nil config",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "test-client-secret",
OIDCClientAssertion: nil,
},
wantErr: true,
expectedErr: `no auth method config or client assertion`,
},
{
name: "invalid client secret length",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "test-client-secret",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceClientSecret,
KeyAlgorithm: "HS256",
Audience: []string{"test-audience"},
ExtraHeaders: map[string]string{
"test-header": "test-value",
},
},
},
wantErr: true,
expectedErr: `invalid secret length for algorithm: "HS256" must be at least 32 bytes long`,
},
{
name: "invalid client secret kid in extra header",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "1234567890abcdefghijklmnopqrstuvwxyz",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceClientSecret,
KeyAlgorithm: "HS256",
Audience: []string{"test-audience"},
ExtraHeaders: map[string]string{
"kid": "test-kid",
},
},
},
wantErr: true,
expectedErr: `WithHeaders: "kid" not allowed in WithHeaders; use WithKeyID instead`,
},
{
name: "invalid key algorithm none",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "1234567890abcdefghijklmnopqrstuvwxyz",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceClientSecret,
KeyAlgorithm: "none",
Audience: []string{"test-audience"},
ExtraHeaders: map[string]string{
"test-header": "test-value",
},
},
},
wantErr: true,
expectedErr: `unsupported algorithm "none" for client secret`,
},
{
name: "invalid key algorithm None",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "1234567890abcdefghijklmnopqrstuvwxyz",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceClientSecret,
KeyAlgorithm: "None",
Audience: []string{"test-audience"},
ExtraHeaders: map[string]string{
"test-header": "test-value",
},
},
},
wantErr: true,
expectedErr: `unsupported algorithm "None" for client secret`,
},
// expected non-nil error; got nil
{
name: "invalid missing audience",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientSecret: "1234567890abcdefghijklmnopqrstuvwxyz",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceClientSecret,
KeyAlgorithm: "HS256",
ExtraHeaders: map[string]string{
"test-header": "test-value",
},
},
},
wantErr: true,
expectedErr: "missing Audience",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.config.Canonicalize() // inherits ClientSecret from OIDCClientAssertion
jwt, err := BuildClientAssertionJWT(tt.config, nil, "")
if tt.wantErr {
must.Error(t, err)
must.StrContains(t, err.Error(), tt.expectedErr)
} else {
must.NoError(t, err)
must.NotNil(t, jwt)
}
})
}
}
func TestBuildClientAssertionJWT_PrivateKey(t *testing.T) {
nomadKey := generateTestPrivateKey(t)
nomadKeyPath := writeTestPrivateKeyToFile(t, nomadKey)
nomadKID := "anything"
nomadCert := generateTestCertificate(t, nomadKey)
nomadCertPath := writeTestCertToFile(t, nomadCert)
nonKeyCertFile := path.Join(t.TempDir(), "bad.key.cert.pem")
must.NoError(t, os.WriteFile(nonKeyCertFile, []byte("not a key or cert"), 0644))
tests := []struct {
name string
config *structs.ACLAuthMethodConfig
wantErr bool
expectedErr string
}{
{
name: "nil config",
config: &structs.ACLAuthMethodConfig{
OIDCClientAssertion: nil,
},
wantErr: true,
},
{
name: "valid private key source with pem key",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKey: encodeTestPrivateKey(nomadKey),
KeyID: nomadKID,
},
},
},
wantErr: false,
},
{
name: "valid private key source with pem key with pem cert",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKey: encodeTestPrivateKey(nomadKey),
PemCert: encodeTestCert(nomadCert),
},
},
},
wantErr: false,
},
{
name: "valid private key source with pem cert file",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKey: encodeTestPrivateKey(nomadKey),
PemCertFile: nomadCertPath,
},
},
},
wantErr: false,
},
{
name: "valid private key source with pem key file with Key ID",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKeyFile: nomadKeyPath,
KeyID: nomadKID,
},
},
},
wantErr: false,
},
// invalid pem key file location
{
name: "invalid private key source with pem key file",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKeyFile: nomadKeyPath + "/invalid",
KeyID: nomadKID,
},
},
},
wantErr: true,
expectedErr: "error reading PemKeyFile",
},
// file does exist but is not an rsa key
{
name: "invalid private key file contents",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKeyFile: nonKeyCertFile,
KeyID: nomadKID,
},
},
},
wantErr: true,
expectedErr: "error parsing PemKeyFile: invalid key:",
},
{
name: "invalid certificate file contents",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKey: encodeTestPrivateKey(nomadKey),
PemCertFile: nonKeyCertFile,
},
},
},
wantErr: true,
expectedErr: "failed to decode PemCertFile PEM block",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.config.Canonicalize() // inherits clientSecret from OIDCClientAssertion
jwt, err := BuildClientAssertionJWT(tt.config, nomadKey, nomadKID)
if tt.wantErr {
must.Error(t, err)
must.StrContains(t, err.Error(), tt.expectedErr)
} else {
must.NoError(t, err)
must.NotNil(t, jwt)
}
})
}
}
func TestBuildClientAssertionJWT_NomadKey(t *testing.T) {
nomadKey := generateTestPrivateKey(t)
nomadKID := "anything"
tests := []struct {
name string
config *structs.ACLAuthMethodConfig
wantErr bool
}{
{
name: "nil config",
config: &structs.ACLAuthMethodConfig{
OIDCClientAssertion: nil,
},
wantErr: true,
},
{
name: "nomad key source",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourceNomad,
KeyAlgorithm: "RS256",
Audience: []string{"test-audience"},
},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.config.Canonicalize() // inherits clientSecret from OIDCClientAssertion
jwt, err := BuildClientAssertionJWT(tt.config, nomadKey, nomadKID)
if tt.wantErr {
must.Error(t, err)
} else {
must.NoError(t, err)
must.NotNil(t, jwt)
}
})
}
}
func TestBuildClientAssertionJWT_PrivateKeyExpiredCert(t *testing.T) {
nomadKey := generateTestPrivateKey(t)
nomadInvalidKey := generateInvalidTestPrivateKey(t)
nomadKID := "anything"
nomadCert := generateTestCertificate(t, nomadKey)
nomadExpiredCert := generateExpiredTestCertificate(t, nomadKey)
tests := []struct {
name string
config *structs.ACLAuthMethodConfig
wantErr bool
expectedErr string
}{
{
name: "invalid PemKey",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKey: encodeTestPrivateKey(nomadInvalidKey),
PemCert: encodeTestCert(nomadCert),
},
},
},
wantErr: true,
expectedErr: "failed to parse private key",
},
{
name: "expired certificate PemCert",
config: &structs.ACLAuthMethodConfig{
OIDCClientID: "test-client-id",
OIDCClientAssertion: &structs.OIDCClientAssertion{
KeySource: structs.OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
KeyAlgorithm: "RS256",
PrivateKey: &structs.OIDCClientAssertionKey{
PemKey: encodeTestPrivateKey(nomadKey),
PemCert: encodeTestCert(nomadExpiredCert),
},
},
},
wantErr: true,
expectedErr: "certificate has expired or is not yet valid",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.config.Canonicalize() // inherits clientSecret from OIDCClientAssertion
jwt, err := BuildClientAssertionJWT(tt.config, nomadKey, nomadKID)
if tt.wantErr {
must.Error(t, err)
must.StrContains(t, err.Error(), tt.expectedErr)
} else {
must.NoError(t, err)
must.NotNil(t, jwt)
}
})
}
}
func generateTestPrivateKey(t *testing.T) *rsa.PrivateKey {
key, err := rsa.GenerateKey(rand.Reader, 2048)
must.NoError(t, err)
return key
}
func writeTestPrivateKeyToFile(t *testing.T, key *rsa.PrivateKey) string {
tmpDir := t.TempDir()
keyPath := filepath.Join(tmpDir, "testkey.pem")
keyFile, err := os.Create(keyPath)
must.NoError(t, err)
defer keyFile.Close()
keyBytes := x509.MarshalPKCS1PrivateKey(key)
block := &pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: keyBytes,
}
err = pem.Encode(keyFile, block)
must.NoError(t, err)
return keyPath
}
func encodeTestPrivateKey(key *rsa.PrivateKey) string {
keyBytes := x509.MarshalPKCS1PrivateKey(key)
block := &pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: keyBytes,
}
return string(pem.EncodeToMemory(block))
}
func generateTestCertificate(t *testing.T, key *rsa.PrivateKey) *x509.Certificate {
template := &x509.Certificate{
SerialNumber: big.NewInt(1),
NotBefore: time.Now(),
NotAfter: time.Now().Add(time.Hour),
}
certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
must.NoError(t, err)
cert, err := x509.ParseCertificate(certDER)
must.NoError(t, err)
return cert
}
func encodeTestCert(cert *x509.Certificate) string {
block := &pem.Block{
Type: "CERTIFICATE",
Bytes: cert.Raw,
}
return string(pem.EncodeToMemory(block))
}
func writeTestCertToFile(t *testing.T, cert *x509.Certificate) string {
tmpDir := t.TempDir()
certPath := filepath.Join(tmpDir, "testcert.pem")
certFile, err := os.Create(certPath)
must.NoError(t, err)
defer certFile.Close()
block := &pem.Block{
Type: "CERTIFICATE",
Bytes: cert.Raw,
}
err = pem.Encode(certFile, block)
must.NoError(t, err)
return certPath
}
func generateExpiredTestCertificate(t *testing.T, key *rsa.PrivateKey) *x509.Certificate {
template := &x509.Certificate{
SerialNumber: big.NewInt(1),
NotBefore: time.Now().Add(-2 * time.Hour),
NotAfter: time.Now().Add(-1 * time.Hour),
}
certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
must.NoError(t, err)
cert, err := x509.ParseCertificate(certDER)
must.NoError(t, err)
return cert
}
func generateInvalidTestPrivateKey(t *testing.T) *rsa.PrivateKey {
key, err := rsa.GenerateKey(rand.Reader, 2048)
must.NoError(t, err)
// Simulate an invalid key by modifying the key's modulus
key.N = big.NewInt(0) // This is just a placeholder to simulate an invalid key
return key
}

23
nomad/structs/acl.go
View File

@@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"maps"
"path"
"regexp"
"slices"
"strconv"
@@ -1378,11 +1379,13 @@ func (k *OIDCClientAssertionKey) Canonicalize() {
}
var (
ErrMissingClientAssertionKey = errors.New("missing PemKey or PemKeyFile")
ErrAmbiguousClientAssertionKey = errors.New("require only one of PemKey or PemKeyFile")
ErrMissingClientAssertionKeyID = errors.New("missing PemCert, PemCertFile, or KeyID")
ErrAmbiguousClientAssertionKeyID = errors.New("require only one of PemCert, PemCertFile, or KeyID")
ErrInvalidKeyIDHeader = errors.New("invalid KeyIDHeader")
ErrMissingClientAssertionKey = errors.New("missing PemKey or PemKeyFile")
ErrAmbiguousClientAssertionKey = errors.New("require only one of PemKey or PemKeyFile")
ErrMissingClientAssertionKeyID = errors.New("missing PemCert, PemCertFile, or KeyID")
ErrAmbiguousClientAssertionKeyID = errors.New("require only one of PemCert, PemCertFile, or KeyID")
ErrInvalidClientAssertionKeyPath = errors.New("invalid PemKeyFile")
ErrInvalidClientAssertionCertPath = errors.New("invalid PemCertFile")
ErrInvalidKeyIDHeader = errors.New("invalid KeyIDHeader")
)
// Validate ensures that one Key and one Cert or KeyID are provided,
@@ -1400,6 +1403,11 @@ func (k *OIDCClientAssertionKey) Validate() error {
if k.PemKey != "" && k.PemKeyFile != "" {
return ErrAmbiguousClientAssertionKey
}
if k.PemKeyFile != "" {
if !path.IsAbs(k.PemKeyFile) {
return fmt.Errorf("%w: must be absolute; got: %s", ErrInvalidClientAssertionKeyPath, k.PemKeyFile)
}
}
// mutually exclusive cert fields
// must have exactly one of: cert file or base64, or keyid
@@ -1415,6 +1423,11 @@ func (k *OIDCClientAssertionKey) Validate() error {
if k.KeyID != "" && (k.PemCert != "" || k.PemCertFile != "") {
return ErrAmbiguousClientAssertionKeyID
}
if k.PemCertFile != "" {
if !path.IsAbs(k.PemCertFile) {
return fmt.Errorf("%w: must be absolute; got: %s", ErrInvalidClientAssertionCertPath, k.PemCertFile)
}
}
// only allow certain key id headers
// only "kid" for KeyID

48
nomad/structs/acl_test.go
View File

@@ -1392,6 +1392,22 @@ func TestACLAuthMethod_Merge(t *testing.T) {
must.Eq(t, am1.Config.OIDCClientAssertion.PrivateKey.KeyID, "test-key-id")
}
func TestACLAuthMethodConfig_Validate(t *testing.T) {
ci.Parallel(t)
// fail everything
am := &ACLAuthMethodConfig{}
am.Canonicalize() // there is insufficient info for this to help
err := am.Validate()
must.ErrorContains(t, err, "missing OIDCDiscoveryURL")
must.ErrorContains(t, err, "missing OIDCClientID")
must.ErrorContains(t, err, "missing BoundAudiences") // TODO: BoundIssuer? it's not even documented...
am.OIDCClientAssertion = &OIDCClientAssertion{}
am.Canonicalize()
err = am.Validate()
must.ErrorContains(t, err, "invalid client assertion config:")
}
func TestACLAuthMethodConfig_Copy(t *testing.T) {
ci.Parallel(t)
@@ -1617,13 +1633,6 @@ func TestOIDCClientAssertionKey_Validate(t *testing.T) {
},
err: ErrMissingClientAssertionKey,
},
{
name: "missing kid or cert",
key: &OIDCClientAssertionKey{
PemKeyFile: "/any.key",
},
err: ErrMissingClientAssertionKeyID,
},
{
name: "ambiguous key",
key: &OIDCClientAssertionKey{
@@ -1632,6 +1641,13 @@ func TestOIDCClientAssertionKey_Validate(t *testing.T) {
},
err: ErrAmbiguousClientAssertionKey,
},
{
name: "missing keyid or cert",
key: &OIDCClientAssertionKey{
PemKeyFile: "/any.key",
},
err: ErrMissingClientAssertionKeyID,
},
{
name: "ambiguous keyid - cert file and pem",
key: &OIDCClientAssertionKey{
@@ -1659,6 +1675,22 @@ func TestOIDCClientAssertionKey_Validate(t *testing.T) {
},
err: ErrAmbiguousClientAssertionKeyID,
},
{
name: "non-absolute key path",
key: &OIDCClientAssertionKey{
PemKeyFile: "./who-knows-where-this-might-be.key",
KeyID: "key-id",
},
err: ErrInvalidClientAssertionKeyPath,
},
{
name: "non-absolute cert path",
key: &OIDCClientAssertionKey{
PemKey: "anykey",
PemCertFile: "./who-knows-where-this-might-be.cert",
},
err: ErrInvalidClientAssertionCertPath,
},
{
name: "bad key header for keyid",
key: &OIDCClientAssertionKey{
@@ -2089,7 +2121,7 @@ func validClientAssertion() *OIDCClientAssertion {
KeySource: OIDCKeySourcePrivateKey,
Audience: []string{"test-audience"},
PrivateKey: &OIDCClientAssertionKey{
PemKeyFile: "test-key-file",
PemKeyFile: "/test.key",
KeyIDHeader: OIDCClientAssertionHeaderKid,
KeyID: "test-key-id",
},