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

Support MutualAuthentication in HTTPS-Server (IDFGH-2004) #4184

Closed
wants to merge 1 commit into from
Closed

Support MutualAuthentication in HTTPS-Server (IDFGH-2004) #4184

wants to merge 1 commit into from

Conversation

schmidma
Copy link
Contributor

The ESP-TLS-Server supports client verification in mutual authentication. This adds functionality to the esp_https_server for setting the required certificates.
In this way the https server is able to verify a client's certificate using the given ca certificate.

@mahavirj mahavirj requested a review from anurag-kar October 10, 2019 09:45
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2019

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title Support MutualAuthentication in HTTPS-Server Support MutualAuthentication in HTTPS-Server (IDFGH-2004) Oct 15, 2019
/** Private key */
const uint8_t *prvtkey_pem;
Copy link
Member

Choose a reason for hiding this comment

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

This change will break public API and hence build for examples like https_server, esp_local_control. Can you please revert this?

Copy link
Collaborator

@AdityaHPatwardhan AdityaHPatwardhan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I have tested these changes , they LGTM. Along with this, it requires appropriate changes in the https_server example in the ESP-IDF to make the PR complete. It seems in https_server example
i.e.
change the name of server cert from ca_cert to server_cert,so that when user wants to turn on mutual auth he will just add a client cert or (rootCA of clients) in ca_cert in httpd_ssl_config_t(which should be empty otherwise),
the nomenclature there was a bit confusing which should be fixed with this PR.Thank You

}
memcpy((char *)cfg->cacert_buf, config->cacert_pem, config->cacert_len);
cfg->cacert_bytes = config->cacert_len;

cfg->servercert_buf = (unsigned char *)malloc(config->cacert_len);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be
cfg->servercert_buf = (unsigned char *)malloc(config->servercert_len);

free(cfg);
return NULL;
}
memcpy((char *)cfg->servercert_buf, config->cacert_pem, config->cacert_len);
memcpy((char *)cfg->servercert_buf, config->servercert_pem, config->servercert_len);
cfg->servercert_bytes = config->cacert_len;
Copy link
Collaborator

Choose a reason for hiding this comment

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

it should be
cfg->servercert_bytes = config->servercert_len;

@AdityaHPatwardhan
Copy link
Collaborator

@schmidma Please make the required changes and help merge the PR. Thank You!

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