Skip to content

Commit

Permalink
OpenID Configuration Discovery Endpoint (#18691)
Browse files Browse the repository at this point in the history
Added the [OIDC Discovery](https://openid.net/specs/openid-connect-discovery-1_0.html) `/.well-known/openid-configuration` endpoint to Nomad, but it is only enabled if the `server.oidc_issuer` parameter is set. Documented the parameter, but without a tutorial trying to actually _use_ this will be very hard.

I intentionally did *not* use https://github.com/hashicorp/cap for the OIDC configuration struct because it's built to be a *compliant* OIDC provider. Nomad is *not* trying to be compliant initially because compliance to the spec does not guarantee it will actually satisfy the requirements of third parties. I want to avoid the problem where in an attempt to be standards compliant we ship configuration parameters that lock us in to a certain behavior that we end up regretting. I want to add parameters and behaviors as there's a demonstrable need.

Users always have the escape hatch of providing their own OIDC configuration endpoint. Nomad just needs to know the Issuer so that the JWTs match the OIDC configuration. There's no reason the actual OIDC configuration JSON couldn't live in S3 and get served directly from there. Unlike JWKS the OIDC configuration should be static, or at least change very rarely.

This PR is just the endpoint extracted from #18535. The `RS256` algorithm still needs to be added in hopes of supporting third parties such as [AWS IAM OIDC Provider](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc.html).

Co-authored-by: Luiz Aoqui <[email protected]>
  • Loading branch information
schmichael and lgfa29 authored Oct 21, 2023
1 parent 0020139 commit a806363
Show file tree
Hide file tree
Showing 18 changed files with 405 additions and 14 deletions.
1 change: 1 addition & 0 deletions .semgrep/rpc_endpoint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ rules:
- pattern-not: '"Status.Peers"'
- pattern-not: '"Status.Version"'
- pattern-not: '"Keyring.ListPublic"'
- pattern-not: '"Keyring.GetConfig"'
message: "RPC method $METHOD appears to be unauthenticated"
languages:
- "go"
Expand Down
2 changes: 2 additions & 0 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,8 @@ func convertServerConfig(agentConfig *Config) (*nomad.Config, error) {
conf.JobTrackedVersions = *agentConfig.Server.JobTrackedVersions
}

conf.OIDCIssuer = agentConfig.Server.OIDCIssuer

// Set up the bind addresses
rpcAddr, err := net.ResolveTCPAddr("tcp", agentConfig.normalizedAddrs.RPC)
if err != nil {
Expand Down
14 changes: 14 additions & 0 deletions command/agent/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"log"
"net/url"
"os"
"os/signal"
"path/filepath"
Expand Down Expand Up @@ -483,6 +484,19 @@ func (c *Command) IsValidConfig(config, cmdConfig *Config) bool {
if config.Server.Enabled && config.Server.BootstrapExpect%2 == 0 {
c.Ui.Error("WARNING: Number of bootstrap servers should ideally be set to an odd number.")
}

// Check OIDC Issuer if set
if config.Server.Enabled && config.Server.OIDCIssuer != "" {
issuerURL, err := url.Parse(config.Server.OIDCIssuer)
if err != nil {
c.Ui.Error(fmt.Sprintf(`Error using server.oidc_issuer = "%s" as a base URL: %s`, config.Server.OIDCIssuer, err))
return false
}

if issuerURL.Scheme != "https" {
c.Ui.Warn(fmt.Sprintf(`server.oidc_issuer = "%s" is not using https. Many OIDC implementations require https.`, config.Server.OIDCIssuer))
}
}
}

// ProtocolVersion has never been used. Warn if it is set as someone
Expand Down
11 changes: 11 additions & 0 deletions command/agent/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,17 @@ func TestIsValidConfig(t *testing.T) {
},
},
},
{
name: "BadOIDCIssuer",
conf: Config{
DataDir: "/tmp",
Server: &ServerConfig{
Enabled: true,
OIDCIssuer: ":/example.com",
},
},
err: "missing protocol scheme",
},
}

for _, tc := range cases {
Expand Down
9 changes: 9 additions & 0 deletions command/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,11 @@ type ServerConfig struct {

// JobTrackedVersions is the number of historic job versions that are kept.
JobTrackedVersions *int `hcl:"job_tracked_versions"`

// OIDCIssuer if set enables OIDC Discovery and uses this value as the
// issuer. Third parties such as AWS IAM OIDC Provider expect the issuer to
// be a publically accessible HTTPS URL signed by a trusted well-known CA.
OIDCIssuer string `hcl:"oidc_issuer"`
}

func (s *ServerConfig) Copy() *ServerConfig {
Expand Down Expand Up @@ -2121,6 +2126,10 @@ func (s *ServerConfig) Merge(b *ServerConfig) *ServerConfig {
result.JobTrackedVersions = b.JobTrackedVersions
}

if b.OIDCIssuer != "" {
result.OIDCIssuer = b.OIDCIssuer
}

// Add the schedulers
result.EnabledSchedulers = append(result.EnabledSchedulers, b.EnabledSchedulers...)

Expand Down
2 changes: 2 additions & 0 deletions command/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func TestConfig_Merge(t *testing.T) {
NodeThreshold: 100,
NodeWindow: 11 * time.Minute,
},
OIDCIssuer: "https://oidc.test.nomadproject.io",
},
ACL: &ACLConfig{
Enabled: true,
Expand Down Expand Up @@ -408,6 +409,7 @@ func TestConfig_Merge(t *testing.T) {
},
JobMaxPriority: pointer.Of(200),
JobDefaultPriority: pointer.Of(100),
OIDCIssuer: "https://oidc.test.nomadproject.io",
},
ACL: &ACLConfig{
Enabled: true,
Expand Down
5 changes: 3 additions & 2 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,9 @@ func (s *HTTPServer) registerHandlers(enableDebug bool) {
s.mux.Handle("/v1/vars", wrapCORS(s.wrap(s.VariablesListRequest)))
s.mux.Handle("/v1/var/", wrapCORSWithAllowedMethods(s.wrap(s.VariableSpecificRequest), "HEAD", "GET", "PUT", "DELETE"))

// JWKS Handler
s.mux.HandleFunc("/.well-known/jwks.json", s.wrap(s.JWKSRequest))
// OIDC Handlers
s.mux.HandleFunc(structs.JWKSPath, s.wrap(s.JWKSRequest))
s.mux.HandleFunc("/.well-known/openid-configuration", s.wrap(s.OIDCDiscoveryRequest))

agentConfig := s.agent.GetConfig()
uiConfigEnabled := agentConfig.UI != nil && agentConfig.UI.Enabled
Expand Down
25 changes: 25 additions & 0 deletions command/agent/keyring_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,31 @@ func (s *HTTPServer) JWKSRequest(resp http.ResponseWriter, req *http.Request) (a
return out, nil
}

// OIDCDiscoveryRequest implements the OIDC Discovery protocol for using
// workload identity JWTs with external services.
//
// See https://openid.net/specs/openid-connect-discovery-1_0.html for details.
func (s *HTTPServer) OIDCDiscoveryRequest(resp http.ResponseWriter, req *http.Request) (any, error) {
if req.Method != http.MethodGet {
return nil, CodedError(http.StatusMethodNotAllowed, ErrInvalidMethod)
}

args := structs.GenericRequest{}
if s.parse(resp, req, &args.Region, &args.QueryOptions) {
return nil, nil
}
var rpcReply structs.KeyringGetConfigResponse
if err := s.agent.RPC("Keyring.GetConfig", &args, &rpcReply); err != nil {
return nil, err
}

if rpcReply.OIDCDiscovery == nil {
return nil, CodedError(http.StatusNotFound, "OIDC Discovery endpoint disabled")
}

return rpcReply.OIDCDiscovery, nil
}

// KeyringRequest is used route operator/raft API requests to the implementing
// functions.
func (s *HTTPServer) KeyringRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
Expand Down
83 changes: 83 additions & 0 deletions command/agent/keyring_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ package agent
import (
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
"time"

"github.com/go-jose/go-jose/v3"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"

"github.com/hashicorp/nomad/ci"
Expand Down Expand Up @@ -77,3 +82,81 @@ func TestHTTP_Keyring_CRUD(t *testing.T) {
require.Len(t, listResp, 1)
})
}

// TestHTTP_Keyring_JWKS asserts the JWKS endpoint is enabled by default and
// caches relative to the key rotation threshold.
func TestHTTP_Keyring_JWKS(t *testing.T) {
ci.Parallel(t)

threshold := 3 * 24 * time.Hour
cb := func(c *Config) {
c.Server.RootKeyRotationThreshold = threshold.String()
}

httpTest(t, cb, func(s *TestAgent) {
respW := httptest.NewRecorder()

req, err := http.NewRequest(http.MethodGet, structs.JWKSPath, nil)
must.NoError(t, err)

obj, err := s.Server.JWKSRequest(respW, req)
must.NoError(t, err)

jwks := obj.(*jose.JSONWebKeySet)
must.SliceLen(t, 1, jwks.Keys)

// Assert that caching headers are set to < the rotation threshold
cacheHeaders := respW.Header().Values("Cache-Control")
must.SliceLen(t, 1, cacheHeaders)
must.StrHasPrefix(t, "max-age=", cacheHeaders[0])
parts := strings.Split(cacheHeaders[0], "=")
ttl, err := strconv.Atoi(parts[1])
must.NoError(t, err)
must.Less(t, int(threshold.Seconds()), ttl)
})
}

// TestHTTP_Keyring_OIDCDisco_Disabled asserts that the OIDC Discovery endpoint
// is disabled by default.
func TestHTTP_Keyring_OIDCDisco_Disabled(t *testing.T) {
ci.Parallel(t)

httpTest(t, nil, func(s *TestAgent) {
respW := httptest.NewRecorder()

req, err := http.NewRequest(http.MethodGet, structs.JWKSPath, nil)
must.NoError(t, err)

_, err = s.Server.OIDCDiscoveryRequest(respW, req)
must.ErrorContains(t, err, "OIDC Discovery endpoint disabled")
codedErr := err.(HTTPCodedError)
must.Eq(t, http.StatusNotFound, codedErr.Code())
})
}

// TestHTTP_Keyring_OIDCDisco_Enabled asserts that the OIDC Discovery endpoint
// is enabled when OIDCIssuer is set.
func TestHTTP_Keyring_OIDCDisco_Enabled(t *testing.T) {
ci.Parallel(t)

// Set OIDCIssuer to a valid looking (but fake) issuer
const testIssuer = "https://oidc.test.nomadproject.io"

cb := func(c *Config) {
c.Server.OIDCIssuer = testIssuer
}

httpTest(t, cb, func(s *TestAgent) {
respW := httptest.NewRecorder()

req, err := http.NewRequest(http.MethodGet, structs.JWKSPath, nil)
must.NoError(t, err)

obj, err := s.Server.OIDCDiscoveryRequest(respW, req)
must.NoError(t, err)

oidcConf := obj.(*structs.OIDCDiscoveryConfig)
must.Eq(t, testIssuer, oidcConf.Issuer)
must.StrHasPrefix(t, testIssuer, oidcConf.JWKS)
})
}
5 changes: 5 additions & 0 deletions nomad/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,11 @@ type Config struct {
JobTrackedVersions int

Reporting *config.ReportingConfig

// OIDCIssuer is the URL for the OIDC Issuer field in Workload Identity JWTs.
// If this is not configured the /.well-known/openid-configuration endpoint
// will not be available.
OIDCIssuer string
}

func (c *Config) Copy() *Config {
Expand Down
20 changes: 16 additions & 4 deletions nomad/encrypter.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ type Encrypter struct {
srv *Server
keystorePath string

// issuer is the OIDC Issuer to use for workload identities if configured
issuer string

keyring map[string]*keyset
lock sync.RWMutex
}
Expand All @@ -62,6 +65,7 @@ func NewEncrypter(srv *Server, keystorePath string) (*Encrypter, error) {
srv: srv,
keystorePath: keystorePath,
keyring: make(map[string]*keyset),
issuer: srv.GetConfig().OIDCIssuer,
}

err := encrypter.loadKeystore()
Expand Down Expand Up @@ -162,9 +166,11 @@ func (e *Encrypter) Decrypt(ciphertext []byte, keyID string) ([]byte, error) {
const keyIDHeader = "kid"

// SignClaims signs the identity claim for the task and returns an encoded JWT
// (including both the claim and its signature), the key ID of the key used to
// sign it, and any error.
func (e *Encrypter) SignClaims(claim *structs.IdentityClaims) (string, string, error) {
// (including both the claim and its signature) and the key ID of the key used
// to sign it, or an error.
//
// SignClaims adds the Issuer claim prior to signing.
func (e *Encrypter) SignClaims(claims *structs.IdentityClaims) (string, string, error) {

// If a key is rotated immediately following a leader election, plans that
// are in-flight may get signed before the new leader has the key. Allow for
Expand All @@ -187,12 +193,18 @@ func (e *Encrypter) SignClaims(claim *structs.IdentityClaims) (string, string, e
}
}

// Add Issuer claim from server configuration
if e.issuer != "" {
claims.Issuer = e.issuer
}

opts := (&jose.SignerOptions{}).WithHeader("kid", keyset.rootKey.Meta.KeyID).WithType("JWT")
sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.EdDSA, Key: keyset.privateKey}, opts)
if err != nil {
return "", "", err
}
raw, err := jwt.Signed(sig).Claims(claim).CompactSerialize()

raw, err := jwt.Signed(sig).Claims(claims).CompactSerialize()
if err != nil {
return "", "", err
}
Expand Down
49 changes: 44 additions & 5 deletions nomad/encrypter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,13 @@ func (s *mockSigner) SignClaims(c *structs.IdentityClaims) (token, keyID string,
func TestEncrypter_LoadSave(t *testing.T) {
ci.Parallel(t)

srv, cleanupSrv := TestServer(t, func(c *Config) {
c.NumSchedulers = 0
})
t.Cleanup(cleanupSrv)

tmpDir := t.TempDir()
encrypter, err := NewEncrypter(&Server{shutdownCtx: context.Background()}, tmpDir)
encrypter, err := NewEncrypter(srv, tmpDir)
require.NoError(t, err)

algos := []structs.EncryptionAlgorithm{
Expand Down Expand Up @@ -394,11 +399,45 @@ func TestEncrypter_SignVerify(t *testing.T) {
require.NoError(t, err)

got, err := e.VerifyClaim(out)
must.NoError(t, err)
must.NotNil(t, got)
must.Eq(t, alloc.ID, got.AllocationID)
must.Eq(t, alloc.JobID, got.JobID)
must.Eq(t, "web", got.TaskName)

// By default an issuer should not be set. See _Issuer test.
must.Eq(t, "", got.Issuer)
}

// TestEncrypter_SignVerify_Issuer asserts that the signer adds an issuer if it
// is configured.
func TestEncrypter_SignVerify_Issuer(t *testing.T) {
// Set OIDCIssuer to a valid looking (but fake) issuer
const testIssuer = "https://oidc.test.nomadproject.io"

ci.Parallel(t)
srv, shutdown := TestServer(t, func(c *Config) {
c.NumSchedulers = 0 // Prevent automatic dequeue

c.OIDCIssuer = testIssuer
})
defer shutdown()
testutil.WaitForLeader(t, srv.RPC)

alloc := mock.Alloc()
claims := structs.NewIdentityClaims(alloc.Job, alloc, wiHandle, alloc.LookupTask("web").Identity, time.Now())
e := srv.encrypter

out, _, err := e.SignClaims(claims)
require.NoError(t, err)
require.NotNil(t, got)
require.Equal(t, alloc.ID, got.AllocationID)
require.Equal(t, alloc.JobID, got.JobID)
require.Equal(t, "web", got.TaskName)

got, err := e.VerifyClaim(out)
must.NoError(t, err)
must.NotNil(t, got)
must.Eq(t, alloc.ID, got.AllocationID)
must.Eq(t, alloc.JobID, got.JobID)
must.Eq(t, "web", got.TaskName)
must.Eq(t, testIssuer, got.Issuer)
}

func TestEncrypter_SignVerify_AlgNone(t *testing.T) {
Expand Down
21 changes: 21 additions & 0 deletions nomad/keyring_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,3 +412,24 @@ func (k *Keyring) ListPublic(args *structs.GenericRequest, reply *structs.Keyrin
}
return k.srv.blockingRPC(&opts)
}

// GetConfig for workload identities. This RPC is used to back an OIDC
// Discovery endpoint.
//
// Unauthenticated because OIDC Discovery endpoints must be publically
// available.
func (k *Keyring) GetConfig(args *structs.GenericRequest, reply *structs.KeyringGetConfigResponse) error {

// JWKS is a public endpoint: intentionally ignore auth errors and only
// authenticate to measure rate metrics.
k.srv.Authenticate(k.ctx, args)
if done, err := k.srv.forward("Keyring.GetConfig", args, args, reply); done {
return err
}
k.srv.MeasureRPCRate("keyring", structs.RateMetricList, args)

defer metrics.MeasureSince([]string{"nomad", "keyring", "get_config"}, time.Now())

reply.OIDCDiscovery = k.srv.oidcDisco
return nil
}
Loading

0 comments on commit a806363

Please sign in to comment.