[VAULT-3472] Cap Client id_token_ttl field to associated Key's verification_ttl (#12677)

This commit is contained in:
vinay-gopalan 2021-10-01 10:47:40 -07:00 committed by GitHub
parent 2844dfac9b
commit 836d7f4ca9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 115 additions and 11 deletions

View File

@ -494,6 +494,23 @@ func (i *IdentityStore) pathOIDCCreateUpdateKey(ctx context.Context, req *logica
return logical.ErrorResponse(errorMessage), nil
}
}
// ensure any clients referencing this key do not already have a id_token_ttl
// greater than the key's verification_ttl
clients, err := i.clientsReferencingTargetKeyName(ctx, req, name)
if err != nil {
return nil, err
}
for _, client := range clients {
if client.IDTokenTTL > key.VerificationTTL {
errorMessage := fmt.Sprintf(
"unable to update key %q because it is currently referenced by one or more clients with an id_token_ttl greater than %d seconds",
name,
key.VerificationTTL/time.Second,
)
return logical.ErrorResponse(errorMessage), nil
}
}
}
if allowedClientIDsRaw, ok := d.GetOk("allowed_client_ids"); ok {

View File

@ -850,6 +850,7 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log
return logical.ErrorResponse("the key parameter is required"), nil
}
var key namedKey
// enforce key existence on client creation
entry, err := req.Storage.Get(ctx, namedKeyConfigPath+client.Key)
if err != nil {
@ -858,6 +859,9 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log
if entry == nil {
return logical.ErrorResponse("key %q does not exist", client.Key), nil
}
if err := entry.DecodeJSON(&key); err != nil {
return nil, err
}
if idTokenTTLRaw, ok := d.GetOk("id_token_ttl"); ok {
client.IDTokenTTL = time.Duration(idTokenTTLRaw.(int)) * time.Second
@ -865,6 +869,10 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log
client.IDTokenTTL = time.Duration(d.Get("id_token_ttl").(int)) * time.Second
}
if client.IDTokenTTL > key.VerificationTTL {
return logical.ErrorResponse("a client's id_token_ttl cannot be greater than the verification_ttl of the key it references"), nil
}
if accessTokenTTLRaw, ok := d.GetOk("access_token_ttl"); ok {
client.AccessTokenTTL = time.Duration(accessTokenTTLRaw.(int)) * time.Second
} else if req.Operation == logical.CreateOperation {

View File

@ -828,7 +828,8 @@ func TestOIDC_Path_OIDC_ProviderReadPublicKey(t *testing.T) {
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key-1",
"key": "test-key-1",
"id_token_ttl": "1m",
},
})
@ -883,7 +884,8 @@ func TestOIDC_Path_OIDC_ProviderReadPublicKey(t *testing.T) {
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key-2",
"key": "test-key-2",
"id_token_ttl": "1m",
},
})
@ -972,6 +974,53 @@ func TestOIDC_Path_OIDC_ProviderClient_NilKeyEntry(t *testing.T) {
expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings)
}
// TestOIDC_Path_OIDC_ProviderClient_InvalidTokenTTL tests the TokenTTL validation
func TestOIDC_Path_OIDC_ProviderClient_InvalidTokenTTL(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
ctx := namespace.RootContext(nil)
storage := &logical.InmemStorage{}
// Create a test key "test-key"
c.identityStore.HandleRequest(ctx, &logical.Request{
Path: "oidc/key/test-key",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"verification_ttl": int64(60),
},
Storage: storage,
})
// Create a test client "test-client" with an id_token_ttl longer than the
// verification_ttl -- should fail with error
resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{
Path: "oidc/client/test-client",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"key": "test-key",
"id_token_ttl": int64(3600),
},
Storage: storage,
})
expectError(t, resp, err)
// validate error message
expectedStrings := map[string]interface{}{
"a client's id_token_ttl cannot be greater than the verification_ttl of the key it references": true,
}
expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings)
// Read "test-client"
respReadTestClient, err := c.identityStore.HandleRequest(ctx, &logical.Request{
Path: "oidc/client/test-client",
Operation: logical.ReadOperation,
Storage: storage,
})
// Ensure that "test-client" was not created
expectSuccess(t, respReadTestClient, err)
if respReadTestClient != nil {
t.Fatalf("Expected a nil response but instead got:\n%#v", respReadTestClient)
}
}
// TestOIDC_Path_OIDC_ProviderClient_UpdateKey tests that a client
// does not allow key modification on Update operations
func TestOIDC_Path_OIDC_ProviderClient_UpdateKey(t *testing.T) {
@ -1007,7 +1056,8 @@ func TestOIDC_Path_OIDC_ProviderClient_UpdateKey(t *testing.T) {
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key1",
"key": "test-key1",
"id_token_ttl": "1m",
},
})
expectSuccess(t, resp, err)
@ -1018,7 +1068,8 @@ func TestOIDC_Path_OIDC_ProviderClient_UpdateKey(t *testing.T) {
Operation: logical.UpdateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key2",
"key": "test-key2",
"id_token_ttl": "1m",
},
})
expectError(t, resp, err)
@ -1088,7 +1139,8 @@ func TestOIDC_Path_OIDC_ProviderClient(t *testing.T) {
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key",
"key": "test-key",
"id_token_ttl": "1m",
},
})
expectSuccess(t, resp, err)
@ -1104,7 +1156,7 @@ func TestOIDC_Path_OIDC_ProviderClient(t *testing.T) {
"redirect_uris": []string{},
"assignments": []string{},
"key": "test-key",
"id_token_ttl": int64(86400),
"id_token_ttl": int64(60),
"access_token_ttl": int64(86400),
"client_id": resp.Data["client_id"],
"client_secret": resp.Data["client_secret"],
@ -1207,6 +1259,7 @@ func TestOIDC_Path_OIDC_ProviderClient_Deduplication(t *testing.T) {
Storage: storage,
Data: map[string]interface{}{
"key": "test-key",
"id_token_ttl": "1m",
"assignments": []string{"test-assignment1", "test-assignment1"},
"redirect_uris": []string{"http://example.com", "http://notduplicate.com", "http://example.com"},
},
@ -1224,7 +1277,7 @@ func TestOIDC_Path_OIDC_ProviderClient_Deduplication(t *testing.T) {
"redirect_uris": []string{"http://example.com", "http://notduplicate.com"},
"assignments": []string{"test-assignment1"},
"key": "test-key",
"id_token_ttl": int64(86400),
"id_token_ttl": int64(60),
"access_token_ttl": int64(86400),
"client_id": resp.Data["client_id"],
"client_secret": resp.Data["client_secret"],
@ -1351,7 +1404,8 @@ func TestOIDC_Path_OIDC_ProviderClient_List(t *testing.T) {
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key",
"key": "test-key",
"id_token_ttl": "1m",
},
})
@ -1360,7 +1414,8 @@ func TestOIDC_Path_OIDC_ProviderClient_List(t *testing.T) {
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key",
"key": "test-key",
"id_token_ttl": "1m",
},
})
@ -1948,8 +2003,9 @@ func TestOIDC_Path_OIDC_ProviderAssignment_DeleteWithExistingClient(t *testing.T
Operation: logical.CreateOperation,
Storage: storage,
Data: map[string]interface{}{
"key": "test-key",
"assignments": []string{"test-assignment"},
"key": "test-key",
"assignments": []string{"test-assignment"},
"id_token_ttl": "1m",
},
})
expectSuccess(t, resp, err)

View File

@ -527,6 +527,29 @@ func TestOIDC_Path_OIDCKey_InvalidTokenTTL(t *testing.T) {
Storage: storage,
})
expectError(t, resp, err)
// Create a client that depends on test key
resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
Path: "oidc/client/test-client",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"key": "test-key",
"id_token_ttl": "4m",
},
Storage: storage,
})
expectSuccess(t, resp, err)
// Update test key "test-key" -- should fail since id_token_ttl is greater than 2m
resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
Path: "oidc/key/test-key",
Operation: logical.UpdateOperation,
Data: map[string]interface{}{
"verification_ttl": "2m",
},
Storage: storage,
})
expectError(t, resp, err)
}
// TestOIDC_Path_OIDCKey tests the List operation for keys