From 1cbdf91dd5ba30068ea09c48fedd6704ce4dc8d6 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 29 May 2024 11:08:22 +0100 Subject: [PATCH 1/2] Fix azure join when filtering by resource group Azure Resource Groups are not case sensitive. The API usually returns them in all caps. When deciding whether to accept a new node using the azure join method, we should ignore the case of the resource groups. --- lib/auth/join_azure.go | 25 +++++++++++++++++++++---- lib/auth/join_azure_test.go | 18 +++++++++--------- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/lib/auth/join_azure.go b/lib/auth/join_azure.go index 9cb41e8e76ce3..c15c7e1628e03 100644 --- a/lib/auth/join_azure.go +++ b/lib/auth/join_azure.go @@ -23,6 +23,7 @@ import ( "encoding/pem" "net/url" "slices" + "strings" "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" @@ -284,15 +285,31 @@ func checkAzureAllowRules(vm *azure.VirtualMachine, token string, allowRules []* if rule.Subscription != vm.Subscription { continue } - if len(rule.ResourceGroups) > 0 { - if !slices.Contains(rule.ResourceGroups, vm.ResourceGroup) { - continue - } + if !azureResourceGroupIsAllowed(rule.ResourceGroups, vm.ResourceGroup) { + continue } return nil } return trace.AccessDenied("instance %v did not match any allow rules in token %v", vm.Name, token) } +func azureResourceGroupIsAllowed(allowedResourceGroups []string, vmResourceGroup string) bool { + if len(allowedResourceGroups) == 0 { + return true + } + + // ResourceGroups are case insensitive. + // https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/frequently-asked-questions#are-resource-group-names-case-sensitive + // The API returns them using capital case, but docs don't mention a specific case. + // Converting everything to the same case will ensure a proper comparison. + resourceGroup := strings.ToUpper(vmResourceGroup) + for _, allowedResourceGroup := range allowedResourceGroups { + if resourceGroup == strings.ToUpper(allowedResourceGroup) { + return true + } + } + + return false +} func (a *Server) checkAzureRequest(ctx context.Context, challenge string, req *proto.RegisterUsingAzureMethodRequest, cfg *azureRegisterConfig) error { requestStart := a.clock.Now() diff --git a/lib/auth/join_azure_test.go b/lib/auth/join_azure_test.go index 28630708cf4fc..8323410ff919d 100644 --- a/lib/auth/join_azure_test.go +++ b/lib/auth/join_azure_test.go @@ -188,7 +188,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: subID, - resourceGroup: "rg", + resourceGroup: "RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -210,7 +210,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "wrong-token", subscription: subID, - resourceGroup: "rg", + resourceGroup: "RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -231,7 +231,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: subID, - resourceGroup: "rg", + resourceGroup: "RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -253,7 +253,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: "some-junk", - resourceGroup: "rg", + resourceGroup: "RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -274,7 +274,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: subID, - resourceGroup: "wrong-rg", + resourceGroup: "WRONG-RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -296,7 +296,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: subID, - resourceGroup: "rg", + resourceGroup: "RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -320,7 +320,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: subID, - resourceGroup: "rg", + resourceGroup: "RG", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, Azure: &types.ProvisionTokenSpecV2Azure{ @@ -341,7 +341,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { tokenName: "test-token", requestTokenName: "test-token", subscription: subID, - resourceGroup: "rg", + resourceGroup: "RG", vmID: "vm-id", tokenSpec: types.ProvisionTokenSpecV2{ Roles: []types.SystemRole{types.RoleNode}, @@ -356,7 +356,7 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { }, vmResult: &azure.VirtualMachine{ Subscription: subID, - ResourceGroup: "rg", + ResourceGroup: "RG", VMID: "different-id", }, verify: mockVerifyToken(nil), From 28f1a01a3298256a1108f94a8a59b7f02ffc92de Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 29 May 2024 16:42:38 +0100 Subject: [PATCH 2/2] use EqualFold and add explicit test --- lib/auth/join_azure.go | 2 +- lib/auth/join_azure_test.go | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/auth/join_azure.go b/lib/auth/join_azure.go index c15c7e1628e03..feeae723a6ac9 100644 --- a/lib/auth/join_azure.go +++ b/lib/auth/join_azure.go @@ -303,7 +303,7 @@ func azureResourceGroupIsAllowed(allowedResourceGroups []string, vmResourceGroup // Converting everything to the same case will ensure a proper comparison. resourceGroup := strings.ToUpper(vmResourceGroup) for _, allowedResourceGroup := range allowedResourceGroups { - if resourceGroup == strings.ToUpper(allowedResourceGroup) { + if strings.EqualFold(resourceGroup, allowedResourceGroup) { return true } } diff --git a/lib/auth/join_azure_test.go b/lib/auth/join_azure_test.go index 8323410ff919d..34933fd43caed 100644 --- a/lib/auth/join_azure_test.go +++ b/lib/auth/join_azure_test.go @@ -205,6 +205,28 @@ func TestAuth_RegisterUsingAzureMethod(t *testing.T) { certs: []*x509.Certificate{tlsConfig.Certificate}, assertError: require.NoError, }, + { + name: "resource group is case insensitive", + tokenName: "test-token", + requestTokenName: "test-token", + subscription: subID, + resourceGroup: "my-RESOURCE-GROUP", + tokenSpec: types.ProvisionTokenSpecV2{ + Roles: []types.SystemRole{types.RoleNode}, + Azure: &types.ProvisionTokenSpecV2Azure{ + Allow: []*types.ProvisionTokenSpecV2Azure_Rule{ + { + Subscription: subID, + ResourceGroups: []string{"MY-resource-group"}, + }, + }, + }, + JoinMethod: types.JoinMethodAzure, + }, + verify: mockVerifyToken(nil), + certs: []*x509.Certificate{tlsConfig.Certificate}, + assertError: require.NoError, + }, { name: "wrong token", tokenName: "test-token",