SLK-133892/azure-plugins-draft-1 - #2205
Conversation
| users: { | ||
| list: { | ||
| url: 'https://graph.microsoft.com/v1.0/users', | ||
| url: 'https://graph.microsoft.com/v1.0/users?$select=id,displayName,userPrincipalName,mail,userType,accountEnabled', |
There was a problem hiding this comment.
Did you test the existing plugin after this change?
There was a problem hiding this comment.
Ensure No Guest User
There was a problem hiding this comment.
yes, I tested both the existing plugins and the new one
| users: { | ||
| list: { | ||
| url: 'https://graph.microsoft.com/v1.0/users', | ||
| url: 'https://graph.microsoft.com/v1.0/users?$select=id,displayName,userPrincipalName,mail,userType,accountEnabled', |
There was a problem hiding this comment.
Ensure No Guest User
| description: 'Ensures that Azure Databricks Workspace has public network access disabled.', | ||
| more_info: 'Disabling public network access ensures that the Databricks workspace is not reachable over the public internet and can only be accessed through private endpoints within trusted networks. This reduces the attack surface and the risk of unauthorized access.', | ||
| recommended_action: 'Modify Databricks workspace networking settings and set Allow Public Network Access to Disabled.', | ||
| link: 'https://learn.microsoft.com/en-us/azure/databricks/security/network/front-end/front-end-private-connect', |
There was a problem hiding this comment.
do you think this endpoint is more valid? https://learn.microsoft.com/en-us/azure/databricks/security/network/classic/secure-cluster-connectivity ?
There was a problem hiding this comment.
your endpoint is more about enableNoPublicIp which is different from Public Access which we are targeting in this plugin, so i dont think we should change it
| name: 'Subscription Owners Minimum', | ||
| description: 'Return a failing result when the number of subscription owners is below this value', | ||
| regex: '^[1-9]{1}[0-9]{0,3}$', | ||
| default: 2 |
There was a problem hiding this comment.
| default: 2 | |
| default: 1 |
There was a problem hiding this comment.
the minimum number should be 1
There was a problem hiding this comment.
but CIS explicitly states that "5.7 Ensure there are between 2 and 3 Subscription Owners", so I don't think we should change it but I can if you say so
| category: 'Storage Accounts', | ||
| domain: 'Storage', | ||
| severity: 'Medium', | ||
| description: 'Ensures that SMB file shares are configured to only allow AES-256-GCM or higher for SMB channel encryption.', |
There was a problem hiding this comment.
how do we know which encryptions are higher?
There was a problem hiding this comment.
CIS says "9.1.3 Ensure 'SMB channel encryption' is Set to 'AES-256-GCM' or Higher for SMB file shares" , but the max encryption is AES-256-GCM nothing higher is available CIS basically future proofed the title and i used same wording, should i change it to only AES-256-GCM?
There was a problem hiding this comment.
lets add/maintain the array for now for encryption and in future if there is a higher encryption we can add it in the same array
| const encryptionList = channelEncryption ? | ||
| channelEncryption.split(';').map(algorithm => algorithm.trim()).filter(algorithm => algorithm) : []; | ||
|
|
||
| if (encryptionList.length && encryptionList.every(algorithm => algorithm.toUpperCase() === 'AES-256-GCM')) { |
There was a problem hiding this comment.
this will only pass if the encryption channel is this, not higher?
There was a problem hiding this comment.
as there is no higher encryption i hardcoded AES-256-GCM, we don't know the name of higher encryption that will be released so we will have to update the plugin when it is released
| category: 'Storage Accounts', | ||
| domain: 'Storage', | ||
| severity: 'Medium', | ||
| description: 'Ensures that SMB file shares are configured to only allow SMB 3.1.1 or higher.', |
There was a problem hiding this comment.
Same concern how do we check higher
There was a problem hiding this comment.
same as above 3.1.1 is the highest version in azure we don't know what higher version will look like so i hardcoded it, i can change the description if you like
|
|
||
| if (!skuName) { | ||
| helpers.addResult(results, 3, | ||
| 'Unable to determine Storage Account redundancy setting', |
There was a problem hiding this comment.
sku tells us the storage account redundancy settings? i think it tells us about the tier of storage account
There was a problem hiding this comment.
sku.tier tells the tier but this plugin is reading sku.name which gives redundancy settings
There was a problem hiding this comment.
how will sku.name will give us redundancy setting?
There was a problem hiding this comment.
this is from official azure doc it gives what we want
| const helpers = require('../../../helpers/azure'); | ||
|
|
||
| module.exports = { | ||
| title: 'Storage Account Entra ID Authorization Default', |
There was a problem hiding this comment.
is this different than Storage Accounts Entra ID Enabled?
There was a problem hiding this comment.
yes this plugin Storage Accounts Entra ID Enabled reads enableAzureFilesAadIntegration that is about mounting Azure Files (SMB) using AD/Entra Domain Services but the pr plugin reads defaultToOAuthAuthentication to check whether the Azure Portal UI defaults to Entra ID
Co-authored-by: Meerab-Shafique <meerab.shafique.ext@aquasec.com>
Co-authored-by: Meerab-Shafique <meerab.shafique.ext@aquasec.com>
2 participants
No description provided.