diff --git a/http/sys_generate_root.go b/http/sys_generate_root.go index 85050616f..f60d1e3f8 100644 --- a/http/sys_generate_root.go +++ b/http/sys_generate_root.go @@ -71,7 +71,7 @@ func handleSysGenerateRootAttemptGet(core *vault.Core, w http.ResponseWriter, r Progress: progress, Required: sealConfig.SecretThreshold, Complete: false, - OTPLength: vault.TokenLength, + OTPLength: vault.TokenLength + 2, OTP: otp, } if generationConfig != nil { @@ -98,7 +98,7 @@ func handleSysGenerateRootAttemptPut(core *vault.Core, w http.ResponseWriter, r case len(req.PGPKey) > 0, len(req.OTP) > 0: default: genned = true - req.OTP, err = base62.Random(vault.TokenLength, true) + req.OTP, err = base62.Random(vault.TokenLength+2, true) if err != nil { respondError(w, http.StatusInternalServerError, err) return diff --git a/vault/generate_root.go b/vault/generate_root.go index 1b7d6e50d..434434244 100644 --- a/vault/generate_root.go +++ b/vault/generate_root.go @@ -121,7 +121,7 @@ func (c *Core) GenerateRootInit(otp, pgpKey string, strategy GenerateRootStrateg var fingerprint string switch { case len(otp) > 0: - if len(otp) != TokenLength { + if len(otp) != TokenLength+2 { return fmt.Errorf("OTP string is wrong length") } diff --git a/vault/generate_root_test.go b/vault/generate_root_test.go index f9a90d079..9eb366849 100644 --- a/vault/generate_root_test.go +++ b/vault/generate_root_test.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "testing" + "github.com/hashicorp/vault/helper/base62" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/pgpkeys" "github.com/hashicorp/vault/helper/xor" @@ -44,13 +45,13 @@ func testCore_GenerateRoot_Lifecycle_Common(t *testing.T, c *Core, keys [][]byte t.Fatalf("err: %v", err) } - otpBytes, err := GenerateRandBytes(16) + otp, err := base62.Random(26, true) if err != nil { t.Fatal(err) } // Start a root generation - err = c.GenerateRootInit(base64.StdEncoding.EncodeToString(otpBytes), "", GenerateStandardRootTokenStrategy) + err = c.GenerateRootInit(otp, "", GenerateStandardRootTokenStrategy) if err != nil { t.Fatalf("err: %v", err) } @@ -87,12 +88,12 @@ func TestCore_GenerateRoot_Init(t *testing.T) { } func testCore_GenerateRoot_Init_Common(t *testing.T, c *Core) { - otpBytes, err := GenerateRandBytes(16) + otp, err := base62.Random(26, true) if err != nil { t.Fatal(err) } - err = c.GenerateRootInit(base64.StdEncoding.EncodeToString(otpBytes), "", GenerateStandardRootTokenStrategy) + err = c.GenerateRootInit(otp, "", GenerateStandardRootTokenStrategy) if err != nil { t.Fatalf("err: %v", err) } @@ -112,12 +113,12 @@ func TestCore_GenerateRoot_InvalidMasterNonce(t *testing.T) { } func testCore_GenerateRoot_InvalidMasterNonce_Common(t *testing.T, c *Core, keys [][]byte) { - otpBytes, err := GenerateRandBytes(16) + otp, err := base62.Random(26, true) if err != nil { t.Fatal(err) } - err = c.GenerateRootInit(base64.StdEncoding.EncodeToString(otpBytes), "", GenerateStandardRootTokenStrategy) + err = c.GenerateRootInit(otp, "", GenerateStandardRootTokenStrategy) if err != nil { t.Fatalf("err: %v", err) } @@ -152,16 +153,15 @@ func TestCore_GenerateRoot_Update_OTP(t *testing.T) { } func testCore_GenerateRoot_Update_OTP_Common(t *testing.T, c *Core, keys [][]byte) { - otpBytes, err := GenerateRandBytes(16) + otp, err := base62.Random(26, true) if err != nil { t.Fatal(err) } - otp := base64.StdEncoding.EncodeToString(otpBytes) // Start a root generation err = c.GenerateRootInit(otp, "", GenerateStandardRootTokenStrategy) if err != nil { - t.Fatalf("err: %v", err) + t.Fatal(err) } // Fetch new config with generated nonce @@ -208,7 +208,7 @@ func testCore_GenerateRoot_Update_OTP_Common(t *testing.T, c *Core, keys [][]byt t.Fatalf("bad: %v", conf) } - tokenBytes, err := base64.StdEncoding.DecodeString(encodedToken) + tokenBytes, err := base64.RawStdEncoding.DecodeString(encodedToken) if err != nil { t.Fatal(err) } diff --git a/vault/router.go b/vault/router.go index 18d07fdfb..66c1ef237 100644 --- a/vault/router.go +++ b/vault/router.go @@ -557,8 +557,8 @@ func (r *Router) routeCommon(ctx context.Context, req *logical.Request, existenc return logical.ErrorResponse(`cubbyhole operations are only supported by "service" type tokens`), false, false, nil } - switch te.NamespaceID { - case namespace.RootNamespaceID: + switch { + case te.NamespaceID == namespace.RootNamespaceID && !strings.HasPrefix(req.ClientToken, "s."): // In order for the token store to revoke later, we need to have the same // salted ID, so we double-salt what's going to the cubbyhole backend salt, err := r.tokenStoreSaltFunc(ctx) diff --git a/vault/token_store.go b/vault/token_store.go index 49c1f4c1d..1b12f8eaf 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -672,11 +672,16 @@ func (ts *TokenStore) SaltID(ctx context.Context, id string) (string, error) { return "", err } - if ns.ID != namespace.RootNamespaceID { - return "h" + s.GetHMAC(id), nil + // For tokens of older format and belonging to the root namespace, use SHA1 + // hash for salting. + if ns.ID == namespace.RootNamespaceID && !strings.Contains(id, ".") { + return s.SaltID(id), nil } - return s.SaltID(id), nil + // For all other tokens, use SHA2-256 HMAC for salting. This includes + // tokens of older format, but belonging to a namespace other than the root + // namespace. + return "h" + s.GetHMAC(id), nil } // rootToken is used to generate a new token with root privileges and no parent @@ -814,8 +819,21 @@ func (ts *TokenStore) create(ctx context.Context, entry *logical.TokenEntry) err } } + if userSelectedID && strings.HasPrefix(entry.ID, "s.") { + return fmt.Errorf("custom token ID cannot have the 's.' prefix") + } + + if !userSelectedID { + entry.ID = fmt.Sprintf("s.%s", entry.ID) + } + + // Attach namespace ID for tokens that are not belonging to the root + // namespace if tokenNS.ID != namespace.RootNamespaceID { entry.ID = fmt.Sprintf("%s.%s", entry.ID, tokenNS.ID) + } + + if tokenNS.ID != namespace.RootNamespaceID || strings.HasPrefix(entry.ID, "s.") { if entry.CubbyholeID == "" { cubbyholeID, err := base62.Random(TokenLength, true) if err != nil { @@ -1052,8 +1070,8 @@ func (ts *TokenStore) Lookup(ctx context.Context, id string) (*logical.TokenEntr return nil, fmt.Errorf("cannot lookup blank token") } - // If it starts with "b-" it's a batch token - if len(id) > 2 && id[0:2] == "b." { + // If it starts with "b." it's a batch token + if len(id) > 2 && strings.HasPrefix(id, "b.") { return ts.lookupBatchToken(ctx, id) } @@ -1131,7 +1149,7 @@ func (ts *TokenStore) lookupInternal(ctx context.Context, id string, salted, tai } // If it starts with "b." it's a batch token - if len(id) > 2 && id[0:2] == "b." { + if len(id) > 2 && strings.HasPrefix(id, "b.") { return ts.lookupBatchToken(ctx, id) } @@ -2527,6 +2545,10 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque te.BoundCIDRs = nil } + if te.ID != "" { + resp.AddWarning("Supplying a custom ID for the token uses the weaker SHA1 hashing instead of the more secure SHA2-256 HMAC for token obfuscation. SHA1 hashed tokens on the wire leads to less secure lookups.") + } + // Create the token if err := ts.create(ctx, &te); err != nil { return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 693a4f02d..f08227e69 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -22,6 +22,95 @@ import ( "github.com/hashicorp/vault/logical" ) +func TestTokenStore_Salting(t *testing.T) { + c, _, _ := TestCoreUnsealed(t) + ts := c.tokenStore + + saltedID, err := ts.SaltID(namespace.RootContext(nil), "foo") + if err != nil { + t.Fatal(err) + } + if strings.HasPrefix(saltedID, "h") { + t.Fatalf("expected sha1 hash; got sha2-256 hmac") + } + + saltedID, err = ts.SaltID(namespace.RootContext(nil), "s.foo") + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(saltedID, "h") { + t.Fatalf("expected sha2-256 hmac; got sha1 hash") + } + + nsCtx := namespace.ContextWithNamespace(context.Background(), &namespace.Namespace{"testid", "ns1"}) + saltedID, err = ts.SaltID(nsCtx, "foo") + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(saltedID, "h") { + t.Fatalf("expected sha2-256 hmac; got sha1 hash") + } + + saltedID, err = ts.SaltID(nsCtx, "s.foo") + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(saltedID, "h") { + t.Fatalf("expected sha2-256 hmac; got sha1 hash") + } +} + +func TestTokenStore_ServiceTokenPrefix(t *testing.T) { + c, _, initToken := TestCoreUnsealed(t) + ts := c.tokenStore + + // Ensure that a regular service token has a "s." prefix + resp, err := ts.HandleRequest(namespace.RootContext(nil), &logical.Request{ + ClientToken: initToken, + Path: "create", + Operation: logical.UpdateOperation, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + if !strings.HasPrefix(resp.Auth.ClientToken, "s.") { + t.Fatalf("token %q does not have a 's.' prefix", resp.Auth.ClientToken) + } + + // Ensure that using a custon token ID results in a warning + resp, err = ts.HandleRequest(namespace.RootContext(nil), &logical.Request{ + ClientToken: initToken, + Path: "create", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "id": "foobar", + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: resp: %#v\nerr: %v", resp, err) + } + expectedWarning := "Supplying a custom ID for the token uses the weaker SHA1 hashing instead of the more secure SHA2-256 HMAC for token obfuscation. SHA1 hashed tokens on the wire leads to less secure lookups." + if resp.Warnings[0] != expectedWarning { + t.Fatalf("expected warning not present") + } + + // Ensure that custom token ID having a "s." prefix fails + resp, err = ts.HandleRequest(namespace.RootContext(nil), &logical.Request{ + ClientToken: initToken, + Path: "create", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "id": "s.foobar", + }, + }) + if err == nil { + t.Fatalf("expected an error") + } + if resp.Error().Error() != "custom token ID cannot have the 's.' prefix" { + t.Fatalf("expected input error not present in error response") + } +} + type TokenEntryOld struct { ID string Accessor string @@ -1357,6 +1446,7 @@ func TestTokenStore_HandleRequest_CreateToken_DisplayName(t *testing.T) { t.Fatalf("err: %v", err) } expected.CreationTime = out.CreationTime + expected.CubbyholeID = out.CubbyholeID if !reflect.DeepEqual(out, expected) { t.Fatalf("bad: expected:%#v\nactual:%#v", expected, out) } @@ -1400,6 +1490,7 @@ func TestTokenStore_HandleRequest_CreateToken_NumUses(t *testing.T) { t.Fatalf("err: %v", err) } expected.CreationTime = out.CreationTime + expected.CubbyholeID = out.CubbyholeID if !reflect.DeepEqual(out, expected) { t.Fatalf("bad: expected:%#v\nactual:%#v", expected, out) } @@ -1476,6 +1567,7 @@ func TestTokenStore_HandleRequest_CreateToken_NoPolicy(t *testing.T) { t.Fatalf("err: %v", err) } expected.CreationTime = out.CreationTime + expected.CubbyholeID = out.CubbyholeID if !reflect.DeepEqual(out, expected) { t.Fatalf("bad: expected:%#v\nactual:%#v", expected, out) }