Deprecate SHA1 in token store (#770)

* Deprecate SHA1 in token store

* Fallback to SHA1 for user selected IDs

* Fix existing tests

* Added warning

* Address some review feedback and remove root token prefix

* Tests for service token prefixing

* Salting utility tests

* Adjust OTP length for root token generation

* Fix tests

* Address review feedback
This commit is contained in:
Vishal Nayak 2018-10-17 13:23:04 -07:00 committed by GitHub
parent d929d83e15
commit 5818977dca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 135 additions and 21 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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