Fix the casing problem in approle (#3665)

builtin/credential/approle/path_role.go

@@ -57,6 +57,10 @@ type roleStorageEntry struct {
 	// value is not modified on the role. If the `Period` in the role is modified,
 	// a token will pick up the new value during its next renewal.
 	Period time.Duration `json:"period" mapstructure:"period" structs:"period"`
+
+	// LowerCaseRoleName enforces the lower casing of role names for all the
+	// roles that get created since this field was introduced.
+	LowerCaseRoleName bool `json:"lower_case_role_name" mapstructure:"lower_case_role_name" structs:"lower_case_role_name"`
 }
 
 // roleIDStorageEntry represents the reverse mapping from RoleID to Role
@@ -560,6 +564,10 @@ func (b *backend) pathRoleSecretIDList(req *logical.Request, data *framework.Fie
 		return logical.ErrorResponse(fmt.Sprintf("role %q does not exist", roleName)), nil
 	}
 
+	if role.LowerCaseRoleName {
+		roleName = strings.ToLower(roleName)
+	}
+
 	// Guard the list operation with an outer lock
 	b.secretIDListingLock.RLock()
 	defer b.secretIDListingLock.RUnlock()
@@ -737,7 +745,8 @@ func (b *backend) pathRoleCreateUpdate(req *logical.Request, data *framework.Fie
 			return nil, fmt.Errorf("failed to create role_id: %v\n", err)
 		}
 		role = &roleStorageEntry{
-			HMACKey: hmacKey,
+			HMACKey:           hmacKey,
+			LowerCaseRoleName: true,
 		}
 	} else if role == nil {
 		return logical.ErrorResponse(fmt.Sprintf("invalid role name")), nil
@@ -991,6 +1000,10 @@ func (b *backend) pathRoleSecretIDLookupUpdate(req *logical.Request, data *frame
 		return nil, fmt.Errorf("role %q does not exist", roleName)
 	}
 
+	if role.LowerCaseRoleName {
+		roleName = strings.ToLower(roleName)
+	}
+
 	// Create the HMAC of the secret ID using the per-role HMAC key
 	secretIDHMAC, err := createHMAC(role.HMACKey, secretID)
 	if err != nil {
@@ -2008,6 +2021,10 @@ func (b *backend) handleRoleSecretIDCommon(req *logical.Request, data *framework
 		return logical.ErrorResponse(fmt.Sprintf("failed to parse metadata: %v", err)), nil
 	}
 
+	if role.LowerCaseRoleName {
+		roleName = strings.ToLower(roleName)
+	}
+
 	if secretIDStorage, err = b.registerSecretIDEntry(req.Storage, roleName, secretID, role.HMACKey, secretIDStorage); err != nil {
 		return nil, fmt.Errorf("failed to store secret_id: %v", err)
 	}

builtin/credential/approle/path_role_test.go

@@ -11,6 +11,167 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
+func TestApprole_RoleNameLowerCasing(t *testing.T) {
+	var resp *logical.Response
+	var err error
+	var roleID, secretID string
+
+	b, storage := createBackendWithStorage(t)
+
+	// Save a role with out LowerCaseRoleName set
+	role := &roleStorageEntry{
+		RoleID:       "testroleid",
+		HMACKey:      "testhmackey",
+		Policies:     []string{"default"},
+		BindSecretID: true,
+	}
+	err = b.setRoleEntry(storage, "testRoleName", role, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	secretIDReq := &logical.Request{
+		Path:      "role/testRoleName/secret-id",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+	}
+	resp, err = b.HandleRequest(secretIDReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	secretID = resp.Data["secret_id"].(string)
+	roleID = "testroleid"
+
+	// Regular login flow. This should succeed.
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "login",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"role_id":   roleID,
+			"secret_id": secretID,
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+
+	// Lower case the role name when generating the secret id
+	secretIDReq.Path = "role/testrolename/secret-id"
+	resp, err = b.HandleRequest(secretIDReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	secretID = resp.Data["secret_id"].(string)
+
+	// Login should fail
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "login",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"role_id":   roleID,
+			"secret_id": secretID,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp == nil || !resp.IsError() {
+		t.Fatalf("expected an error")
+	}
+
+	// Delete the role and create it again. This time don't directly persist
+	// it, but route the request to the creation handler so that it sets the
+	// LowerCaseRoleName to true.
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "role/testRoleName",
+		Operation: logical.DeleteOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+
+	roleReq := &logical.Request{
+		Path:      "role/testRoleName",
+		Operation: logical.CreateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"bind_secret_id": true,
+		},
+	}
+	resp, err = b.HandleRequest(roleReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+
+	// Create secret id with lower cased role name
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "role/testrolename/secret-id",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	secretID = resp.Data["secret_id"].(string)
+
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "role/testrolename/role-id",
+		Operation: logical.ReadOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	roleID = resp.Data["role_id"].(string)
+
+	// Login should pass
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "login",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"role_id":   roleID,
+			"secret_id": secretID,
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr:%v", resp, err)
+	}
+
+	// Lookup of secret ID should work in case-insensitive manner
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "role/testrolename/secret-id/lookup",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"secret_id": secretID,
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	if resp == nil {
+		t.Fatalf("failed to lookup secret IDs")
+	}
+
+	// Listing of secret IDs should work in case-insensitive manner
+	resp, err = b.HandleRequest(&logical.Request{
+		Path:      "role/testrolename/secret-id",
+		Operation: logical.ListOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+
+	if len(resp.Data["keys"].([]string)) != 1 {
+		t.Fatalf("failed to list secret IDs")
+	}
+}
+
 func TestAppRole_RoleReadSetIndex(t *testing.T) {
 	var resp *logical.Response
 	var err error

builtin/credential/approle/validation.go

@@ -125,6 +125,10 @@ func (b *backend) validateCredentials(req *logical.Request, data *framework.Fiel
 			return nil, "", metadata, "", fmt.Errorf("missing secret_id")
 		}
 
+		if role.LowerCaseRoleName {
+			roleName = strings.ToLower(roleName)
+		}
+
 		// Check if the SecretID supplied is valid. If use limit was specified
 		// on the SecretID, it will be decremented in this call.
 		var valid bool