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

Add support to disable tls verification #647

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hatmaalla
Copy link

Our product uses azure-c-shared-utility to establish communication with a web server.
In our case, we want to disable TLS server verification for certain calls to this server.
This PR aims to add an option to disable/enable TLS server verification using http_compact/mbedtls.

@hatmaalla
Copy link
Author

hatmaalla commented Oct 23, 2023 via email

@CIPop
Copy link
Member

CIPop commented Oct 23, 2023

In our case, we want to disable TLS server verification for certain calls to this server.

@hatmaalla First, I would like to ensure that the side-effect of disabling TLS server verification is well understood: if the channel is attacked by a man-in-the-middle, no security is offered: Data can be intercepted as well as modified by the MITM (i.e. the channel is similar in security to a non-encrypted channel).

Copy link
Member

@CIPop CIPop left a comment

Choose a reason for hiding this comment

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

Thank you @hatmaalla . I've left a few comments.

@@ -1028,6 +1028,20 @@ int tlsio_mbedtls_setoption(CONCRETE_IO_HANDLE tls_io, const char *optionName, c
mbedtls_ssl_conf_renegotiation(&tls_io_instance->config, set_renegotiation ? 1 : 0);
result = 0;
}
}
else if (strcmp(optionName, OPTION_DISABLE_TLS_VERIFICATION) == 0)
Copy link
Member

Choose a reason for hiding this comment

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

Please change the option name to: OPTION_DISABLE_TLS_REMOTE_CERTIFICATE_VALIDATION

@@ -57,6 +57,7 @@ typedef struct HTTP_HANDLE_DATA_TAG
unsigned int is_connected : 1;
unsigned int send_completed : 1;
bool tls_renegotiation;
bool tls_verification;
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used anywhere (only set) - please remove.

@@ -1446,6 +1447,12 @@ HTTPAPI_RESULT HTTPAPI_SetOption(HTTP_HANDLE handle, const char* optionName, con
http_instance->tls_renegotiation = tls_renegotiation;
result = HTTPAPI_OK;
}
else if (strcmp(OPTION_DISABLE_TLS_VERIFICATION, optionName) == 0)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used anywhere (only set) - please remove.

}
else
{
bool set_verification = *((bool*)(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

Add LogInfo() message if disabling TLS verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants