Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(azure): check for minimal TLS version for Azure SQL server #5745

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johannes-engler-mw
Copy link
Contributor

Context

Verify a secure configuration of SQL servers in Azure

Description

New check for minimum required TLS version of Azure SQL servers.

Checklist

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the provider/azure Issues/PRs related with the Azure provider label Nov 13, 2024
@johannes-engler-mw johannes-engler-mw changed the title added check for minimal TLS version for Azure SQL server feat(check): check for minimal TLS version for Azure SQL server Nov 13, 2024
@johannes-engler-mw johannes-engler-mw changed the title feat(check): check for minimal TLS version for Azure SQL server feat(azure): check for minimal TLS version for Azure SQL server Nov 13, 2024
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.86%. Comparing base (5641160) to head (f85a9ce).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5745      +/-   ##
==========================================
+ Coverage   89.83%   89.86%   +0.02%     
==========================================
  Files        1128     1129       +1     
  Lines       35154    35173      +19     
==========================================
+ Hits        31582    31609      +27     
+ Misses       3572     3564       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@puchy22 puchy22 self-assigned this Nov 14, 2024
@@ -0,0 +1,30 @@
{
"Provider": "azure",
"CheckID": "sqlserver_minimal_tls_version",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"CheckID": "sqlserver_minimal_tls_version",
"CheckID": "sqlserver_recommended_minimal_tls_version",

from prowler.providers.azure.services.sqlserver.sqlserver_client import sqlserver_client


class sqlserver_minimal_tls_version(Check):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class sqlserver_minimal_tls_version(Check):
class sqlserver_recommended_minimal_tls_version(Check):

report.status = "FAIL"
report.location = sql_server.location
report.status_extended = f"SQL Server {sql_server.name} from subscription {subscription} has no or an deprecated minimal TLS version set."
if sql_server.minimal_tls_version in ("1.2", "1.3"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this check configurable? So if in the future version 1.2 is not recommended, you only have to change the configuration file and not the logic of the check. Here are examples of other checks that are configurable: https://docs.prowler.com/projects/prowler-open-source/en/latest/tutorials/configuration_file/#azure

report.resource_id = sql_server.id
report.status = "FAIL"
report.location = sql_server.location
report.status_extended = f"SQL Server {sql_server.name} from subscription {subscription} has no or an deprecated minimal TLS version set."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.status_extended = f"SQL Server {sql_server.name} from subscription {subscription} has no or an deprecated minimal TLS version set."
report.status_extended = f"SQL Server {sql_server.name} from subscription {subscription} is using TLS version {sql_server.minimal_tls_version} as minimal accepted which is not recommended. Please use one of the recommended versions: {<Here add the minimal recommended or the versions accepted>}"



class sqlserver_minimal_tls_version(Check):
def execute(self) -> Check_Report_Azure:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def execute(self) -> Check_Report_Azure:
def execute(self) -> List[Check_Report_Azure]:

@@ -0,0 +1,22 @@
from prowler.lib.check.models import Check, Check_Report_Azure
from prowler.providers.azure.services.sqlserver.sqlserver_client import sqlserver_client

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from typing import List

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider/azure Issues/PRs related with the Azure provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants