Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

identity: default to RS256 for new workload ids #18882

Merged
merged 23 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8767c8b
identity: default to RS256 for new workload ids
schmichael Oct 7, 2023
acda651
fix signing before new rsa key is generated
schmichael Oct 27, 2023
98c36bb
oh no rsa nooooo
schmichael Oct 27, 2023
893073e
can no longer use a custom region
schmichael Oct 27, 2023
bc4ffd3
Revert "oh no rsa nooooo"
schmichael Oct 30, 2023
5fd3761
Revert "can no longer use a custom region"
schmichael Oct 30, 2023
71e4858
fix log formatting
schmichael Oct 30, 2023
33d4df5
add WaitForKeying test helper
schmichael Oct 30, 2023
211c70a
make api tests wait for keys
schmichael Oct 30, 2023
80cb291
sync api and core waitForLeader
schmichael Oct 30, 2023
195c639
waitForLeader -> waitForServers
schmichael Oct 30, 2023
d406f6e
convert various tests from eddsa->rs256
schmichael Oct 30, 2023
5e99be0
make agent endpoint tests wait for keyring
schmichael Oct 31, 2023
3d62fcd
how did i forget to fix these :facepalm:
schmichael Oct 31, 2023
5f6f086
fixup more tests needing to wait for keyring
schmichael Oct 31, 2023
3af078c
more wait for keyring
schmichael Oct 31, 2023
7758c00
fix test i just broke
schmichael Oct 31, 2023
e004ead
use configured region, not global
schmichael Oct 31, 2023
9fb6ba1
hopefully the final fixes for the final failing package
schmichael Oct 31, 2023
6837ca3
yet another wait for keyring
schmichael Oct 31, 2023
baaad9a
try expanding timings
schmichael Oct 31, 2023
241aeb8
fix data race in test
schmichael Oct 31, 2023
3a14314
handle leadership changes during tests
schmichael Oct 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions api/internal/testutil/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func NewTestServer(t testing.T, cb ServerConfigCallback) *TestServer {

// Wait for the server to be ready
if nomadConfig.Server.Enabled && nomadConfig.Server.BootstrapExpect != 0 {
server.waitForLeader()
server.waitForServers()
} else {
server.waitForAPI()
}
Expand Down Expand Up @@ -296,21 +296,29 @@ func (s *TestServer) waitForAPI() {
)
}

// waitForLeader waits for the Nomad server's HTTP API to become
// available, and then waits for a known leader and an index of
// 1 or more to be observed to confirm leader election is done.
func (s *TestServer) waitForLeader() {
// waitForServers waits for the Nomad server's HTTP API to become available,
// and then waits for the keyring to be intialized. This implies a leader has
// been elected and Raft writes have occurred.
func (s *TestServer) waitForServers() {
Comment on lines +299 to +302
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this and the corresponding change in testutil/server.go I decided to switch all tests to wait for the keyring to be initialized.

I did this because I think these tests are more like integration tests and less concerned with the implementation details than nomad/ package tests. Therefore I thought it appropriate to let these tests block until servers are ready by default.

f := func() error {
// Query the API and check the status code
// Using this endpoint as it is does not have restricted access
resp, err := s.HTTPClient.Get(s.url("/v1/status/leader"))
resp, err := s.HTTPClient.Get(s.url("/.well-known/jwks.json"))
if err != nil {
return fmt.Errorf("failed to get leader: %w", err)
return fmt.Errorf("failed to contact leader: %w", err)
}
defer func() { _ = resp.Body.Close() }()
if err = s.requireOK(resp); err != nil {
return fmt.Errorf("leader response is not ok: %w", err)
}

jwks := struct {
Keys []interface{} `json:"keys"`
}{}
if err := json.NewDecoder(resp.Body).Decode(&jwks); err != nil {
return fmt.Errorf("error decoding jwks response: %w", err)
}
if len(jwks.Keys) == 0 {
return fmt.Errorf("no keys found")
}
return nil
}
must.Wait(s.t,
Expand Down
2 changes: 1 addition & 1 deletion client/vaultclient/vaultclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
jwtAuthConfigTemplate = `
{
"jwks_url": "<<.JWKSURL>>",
"jwt_supported_algs": ["EdDSA"],
"jwt_supported_algs": ["RS256", "EdDSA"],
"default_role": "nomad-workloads"
}
`
Expand Down
11 changes: 6 additions & 5 deletions client/widmgr/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
package widmgr

import (
"crypto/ed25519"
"crypto/rand"
"crypto/rsa"
"fmt"
"slices"
"time"
Expand All @@ -22,13 +23,13 @@ type MockWIDSigner struct {
// SignIdentities will use it to find expirations or reject invalid identity
// names
wids map[string]*structs.WorkloadIdentity
key ed25519.PrivateKey
key *rsa.PrivateKey
keyID string
mockNow time.Time // allows moving the clock
}

func NewMockWIDSigner(wids []*structs.WorkloadIdentity) *MockWIDSigner {
_, privKey, err := ed25519.GenerateKey(nil)
privKey, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -63,7 +64,7 @@ func (m *MockWIDSigner) JSONWebKeySet() *jose.JSONWebKeySet {
jwk := jose.JSONWebKey{
Key: m.key.Public(),
KeyID: m.keyID,
Algorithm: "EdDSA",
Algorithm: "RS256",
Use: "sig",
}
return &jose.JSONWebKeySet{
Expand Down Expand Up @@ -95,7 +96,7 @@ func (m *MockWIDSigner) SignIdentities(minIndex uint64, req []*structs.WorkloadI
}
}
opts := (&jose.SignerOptions{}).WithHeader("kid", m.keyID).WithType("JWT")
sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.EdDSA, Key: m.key}, opts)
sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: m.key}, opts)
if err != nil {
return nil, fmt.Errorf("error creating signer: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions command/agent/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,7 @@ func benchmarkJsonEncoding(b *testing.B, handle *codec.JsonHandle) {
func httpTest(t testing.TB, cb func(c *Config), f func(srv *TestAgent)) {
s := makeHTTPServer(t, cb)
defer s.Shutdown()
testutil.WaitForLeader(t, s.Agent.RPC)
testutil.WaitForKeyring(t, s.Agent.RPC, s.Config.Region)
f(s)
}

Expand All @@ -1572,7 +1572,7 @@ func httpACLTest(t testing.TB, cb func(c *Config), f func(srv *TestAgent)) {
}
})
defer s.Shutdown()
testutil.WaitForLeader(t, s.Agent.RPC)
testutil.WaitForKeyring(t, s.Agent.RPC, s.Config.Region)
f(s)
}

Expand Down
10 changes: 1 addition & 9 deletions command/agent/testagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,7 @@ RETRY:

failed := false
if a.Config.NomadConfig.BootstrapExpect == 1 && a.Config.Server.Enabled {
testutil.WaitForResult(func() (bool, error) {
args := &structs.GenericRequest{}
var leader string
err := a.RPC("Status.Leader", args, &leader)
return leader != "", err
}, func(err error) {
a.T.Logf("failed to find leader: %v", err)
failed = true
})
testutil.WaitForKeyring(a.T, a.RPC, a.Config.Region)
} else {
testutil.WaitForResult(func() (bool, error) {
req, _ := http.NewRequest(http.MethodGet, "/v1/agent/self", nil)
Expand Down
2 changes: 1 addition & 1 deletion command/recommendation_dismiss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func TestRecommendationDismissCommand_Run(t *testing.T) {
}

func TestRecommendationDismissCommand_AutocompleteArgs(t *testing.T) {
ci.Parallel(t)
srv, client, url := testServer(t, false, nil)
defer srv.Shutdown()

Expand All @@ -113,7 +114,6 @@ func TestRecommendationDismissCommand_AutocompleteArgs(t *testing.T) {
}

func testRecommendationAutocompleteCommand(t *testing.T, client *api.Client, srv *agent.TestAgent, cmd *RecommendationAutocompleteCommand) {
ci.Parallel(t)
require := require.New(t)

// Register a test job to write a recommendation against.
Expand Down
13 changes: 9 additions & 4 deletions command/var_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/ci"
"github.com/mitchellh/cli"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -130,7 +131,7 @@ func TestVarListCommand_Online(t *testing.T) {

nsList := []string{api.DefaultNamespace, "ns1"}
pathList := []string{"a/b/c", "a/b/c/d", "z/y", "z/y/x"}
variables := setupTestVariables(client, nsList, pathList)
variables := setupTestVariables(t, client, nsList, pathList)

testTmpl := `{{ range $i, $e := . }}{{if ne $i 0}}{{print "•"}}{{end}}{{printf "%v\t%v" .Namespace .Path}}{{end}}`

Expand Down Expand Up @@ -349,14 +350,14 @@ type testSVNamespacePath struct {
Path string
}

func setupTestVariables(c *api.Client, nsList, pathList []string) SVMSlice {
func setupTestVariables(t *testing.T, c *api.Client, nsList, pathList []string) SVMSlice {

out := make(SVMSlice, 0, len(nsList)*len(pathList))

for _, ns := range nsList {
c.Namespaces().Register(&api.Namespace{Name: ns}, nil)
for _, p := range pathList {
setupTestVariable(c, ns, p, &out)
must.NoError(t, setupTestVariable(c, ns, p, &out))
}
}

Expand All @@ -369,8 +370,12 @@ func setupTestVariable(c *api.Client, ns, p string, out *SVMSlice) error {
Path: p,
Items: map[string]string{"k": "v"}}
v, _, err := c.Variables().Create(testVar, &api.WriteOptions{Namespace: ns})
if err != nil {
return err
}

*out = append(*out, *v.Metadata())
return err
return nil
}

type NSPather interface {
Expand Down
2 changes: 1 addition & 1 deletion e2e/vaultcompat/cluster_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var roleLegacy = map[string]interface{}{
func authConfigJWT(jwksURL string) map[string]any {
return map[string]any{
"jwks_url": jwksURL,
"jwt_supported_algs": []string{"EdDSA"},
"jwt_supported_algs": []string{"RS256", "EdDSA"},
"default_role": "nomad-workloads",
}
}
Expand Down
15 changes: 9 additions & 6 deletions nomad/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
package auth

import (
"crypto/ed25519"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"errors"
Expand All @@ -20,7 +21,6 @@ import (

"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/helper/crypto"
"github.com/hashicorp/nomad/helper/pointer"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/helper/uuid"
Expand Down Expand Up @@ -1217,20 +1217,23 @@ func (ctx *testContext) Certificate() *x509.Certificate {
}

type testEncrypter struct {
key ed25519.PrivateKey
key *rsa.PrivateKey
}

func newTestEncrypter() *testEncrypter {
buf, _ := crypto.Bytes(32)
k, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
panic(err)
}
return &testEncrypter{
key: ed25519.NewKeyFromSeed(buf),
key: k,
}
}

func (te *testEncrypter) signClaim(claims *structs.IdentityClaims) (string, error) {

opts := (&jose.SignerOptions{}).WithType("JWT")
sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.EdDSA, Key: te.key}, opts)
sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: te.key}, opts)
if err != nil {
return "", err
}
Expand Down
19 changes: 13 additions & 6 deletions nomad/client_agent_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,20 @@ func TestMonitor_Monitor_RemoteServer(t *testing.T) {
servers := []*Server{s1, s2}
var nonLeader *Server
var leader *Server
for _, s := range servers {
if !s.IsLeader() {
nonLeader = s
} else {
leader = s
testutil.WaitForResult(func() (bool, error) {
for _, s := range servers {
if !s.IsLeader() {
nonLeader = s
} else {
leader = s
}
}
}

return nonLeader != nil && leader != nil && nonLeader != leader, fmt.Errorf(
"leader=%p follower=%p", nonLeader, leader)
}, func(err error) {
t.Fatalf("timed out trying to find a leader: %v", err)
})

cases := []struct {
desc string
Expand Down
6 changes: 3 additions & 3 deletions nomad/core_sched_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2444,7 +2444,7 @@ func TestCoreScheduler_CSIBadState_ClaimGC(t *testing.T) {
}
}
return true
}, time.Second*5, 10*time.Millisecond, "invalid claims should be marked for GC")
}, 10*time.Second, 50*time.Millisecond, "invalid claims should be marked for GC")

}

Expand All @@ -2454,7 +2454,7 @@ func TestCoreScheduler_RootKeyGC(t *testing.T) {

srv, cleanup := TestServer(t, nil)
defer cleanup()
testutil.WaitForLeader(t, srv.RPC)
testutil.WaitForKeyring(t, srv.RPC, "global")

// reset the time table
srv.fsm.timetable.table = make([]TimeTableEntry, 1, 10)
Expand Down Expand Up @@ -2559,7 +2559,7 @@ func TestCoreScheduler_VariablesRekey(t *testing.T) {

srv, cleanup := TestServer(t, nil)
defer cleanup()
testutil.WaitForLeader(t, srv.RPC)
testutil.WaitForKeyring(t, srv.RPC, "global")

store := srv.fsm.State()
key0, err := store.GetActiveRootKeyMeta(nil)
Expand Down
Loading