Skip to content

Commit

Permalink
ARO-9712 disallow shared access keys for managed identity clusters
Browse files Browse the repository at this point in the history
  • Loading branch information
rajdeepc2792 committed Oct 18, 2024
1 parent 50cecdf commit c11d8e3
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
4 changes: 2 additions & 2 deletions pkg/cluster/deploybaseresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ func (m *manager) deployBaseResourceTemplate(ctx context.Context) error {
}

resources := []*arm.Resource{
m.storageAccount(clusterStorageAccountName, azureRegion, ocpSubnets, true),
m.storageAccount(clusterStorageAccountName, azureRegion, ocpSubnets, true, true),
m.storageAccountBlobContainer(clusterStorageAccountName, graph.IgnitionContainer),
m.storageAccountBlobContainer(clusterStorageAccountName, graph.GraphContainer),
m.storageAccount(m.doc.OpenShiftCluster.Properties.ImageRegistryStorageAccountName, azureRegion, ocpSubnets, true),
m.storageAccount(m.doc.OpenShiftCluster.Properties.ImageRegistryStorageAccountName, azureRegion, ocpSubnets, true, false),
m.storageAccountBlobContainer(m.doc.OpenShiftCluster.Properties.ImageRegistryStorageAccountName, "image-registry"),
m.clusterNSG(infraID, azureRegion),
m.networkPrivateLinkService(azureRegion),
Expand Down
11 changes: 10 additions & 1 deletion pkg/cluster/deploybaseresources_additional.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (m *manager) fpspStorageBlobContributorRBAC(storageAccountName, principalID
// storageAccount will return storage account resource.
// Legacy storage accounts (public) are not encrypted and cannot be retrofitted.
// The flag controls this behavior in update/create.
func (m *manager) storageAccount(name, region string, ocpSubnets []string, encrypted bool) *arm.Resource {
func (m *manager) storageAccount(name, region string, ocpSubnets []string, encrypted bool, setSasPolicy bool) *arm.Resource {
virtualNetworkRules := []mgmtstorage.VirtualNetworkRule{
{
VirtualNetworkResourceID: to.StringPtr("/subscriptions/" + m.env.SubscriptionID() + "/resourceGroups/" + m.env.ResourceGroup() + "/providers/Microsoft.Network/virtualNetworks/rp-pe-vnet-001/subnets/rp-pe-subnet"),
Expand Down Expand Up @@ -207,6 +207,15 @@ func (m *manager) storageAccount(name, region string, ocpSubnets []string, encry
Type: to.StringPtr("Microsoft.Storage/storageAccounts"),
}

// For Workload Identity Cluster disable shared access keys, only User Delegated SAS are allowed
if m.doc.OpenShiftCluster.UsesWorkloadIdentity() && setSasPolicy {
sa.AllowSharedKeyAccess = to.BoolPtr(false)
sa.SasPolicy = &mgmtstorage.SasPolicy{
SasExpirationPeriod: to.StringPtr("0.01:00:00"),
ExpirationAction: to.StringPtr("Log"),
}
}

// In development API calls originates from user laptop so we allow all.
// TODO: Move to development on VPN so we can make this IPRule. Will be done as part of Simply secure v2 work
if m.env.IsLocalDevelopmentMode() {
Expand Down
4 changes: 2 additions & 2 deletions pkg/cluster/storageaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ func (m *manager) migrateStorageAccounts(ctx context.Context) error {
Schema: "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
ContentVersion: "1.0.0.0",
Resources: []*arm.Resource{
m.storageAccount(clusterStorageAccountName, m.doc.OpenShiftCluster.Location, ocpSubnets, false),
m.storageAccount(registryStorageAccountName, m.doc.OpenShiftCluster.Location, ocpSubnets, false),
m.storageAccount(clusterStorageAccountName, m.doc.OpenShiftCluster.Location, ocpSubnets, false, true),
m.storageAccount(registryStorageAccountName, m.doc.OpenShiftCluster.Location, ocpSubnets, false, false),
},
}

Expand Down

0 comments on commit c11d8e3

Please sign in to comment.