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

mbedTLS: CURLOPT_SSL_CTX_FUNCTION support added #1272

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@o7alesmlakar
Contributor

o7alesmlakar commented Feb 21, 2017

I need to add mbedTLS ca certs from memory, so I added the same code as used in OpenSSL implementation. It's done at the end of step1 and it works out great.

Example (based on OpenSSL example from https://curl.haxx.se/libcurl/c/CURLOPT_SSL_CTX_FUNCTION.html):

/* mbedTLS specific */
 
#include <mbedtls/net.h>
#include <curl/curl.h>
#include <stdio.h>
 
static CURLcode sslctx_function(CURL *curl, void *sslctx, void *parm)
{
  mbedtls_ssl_config *config=(mbedtls_ssl_config*)sslctx;
  char *mypem = /* example CA cert PEM - shortened */
    "-----BEGIN CERTIFICATE-----n"\
    "MIIHPTCCBSWgAwIBAgIBADANBgkqhkiG9w0BAQQFADB5MRAwDgYDVQQKEwdSb290n"\
    "IENBMR4wHAYDVQQLExVodHRwOi8vd3d3LmNhY2VydC5vcmcxIjAgBgNVBAMTGUNBn"\
    "IENlcnQgU2lnbmluZyBBdXRob3JpdHkxITAfBgkqhkiG9w0BCQEWEnN1cHBvcnRAn"\
    "Y2FjZXJ0Lm9yZzAeFw0wMzAzMzAxMjI5NDlaFw0zMzAzMjkxMjI5NDlaMHkxEDAOn"\
    "GCSNe9FINSkYQKyTYOGWhlC0elnYjyELn8+CkcY7v2vcB5G5l1YjqrZslMZIBjzkn"\
    "zk6q5PYvCdxTby78dOs6Y5nCpqyJvKeyRKANihDjbPIky/qbn3BHLt4Ui9SyIAmWn"\
    "omTxJBzcoTWcFbLUvFUufQb1nA5V9FrWk9p2rSVzTMVDn"\
    "-----END CERTIFICATE-----n";
  /* add our certificate to this config */
  if(mbedtls_x509_crt_parse(config->ca_chain, (const unsigned char*)mypem, strlen(mypem) + 1))
    printf("mbedtls_x509_crt_parse failed...\n");
  /* all set to go */
  return CURLE_OK;
}
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Feb 21, 2017

@o7alesmlakar, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sasq64, @bagder and @jay to be potential reviewers.

mention-bot commented Feb 21, 2017

@o7alesmlakar, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sasq64, @bagder and @jay to be potential reviewers.

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Feb 21, 2017

Member

Run make checksrc in the source tree and make sure it doesn't warn.

Member

bagder commented Feb 21, 2017

Run make checksrc in the source tree and make sure it doesn't warn.

@bagder bagder added the SSL/TLS label Feb 21, 2017

@jazzbre

This comment has been minimized.

Show comment
Hide comment
@jazzbre

jazzbre Feb 21, 2017

Apparently gcc test 2033...[NTLM connection mapping, pipelining enabled] failed, I don't think I've done anything to mess with that though.

jazzbre commented Feb 21, 2017

Apparently gcc test 2033...[NTLM connection mapping, pipelining enabled] failed, I don't think I've done anything to mess with that though.

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Feb 21, 2017

Member

Pipelining tests are notoriously flaky, there is a timing component in what order the data is received so occasionally they fail.

Member

jay commented Feb 21, 2017

Pipelining tests are notoriously flaky, there is a timing component in what order the data is received so occasionally they fail.

Show outdated Hide outdated lib/vtls/mbedtls.c
@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Feb 21, 2017

Member

Great. Now this just needs documentation. If you want to take a crack at it you would need to modify docs/libcurl/opts CURLOPT_SSL_CTX_FUNCTION.3 to say a little something about how it differs for mbedTLS, and then the AVAILABILITY of both that and CURLOPT_SSL_CTX_DATA.3 to say Added in 7.54.0 for mbedTLS. Note 7.54.0 and not 7.53.0 since this PR is too late for tomorrow's release. If you don't want to do the doc stuff that's fine, i can do it.

Member

jay commented Feb 21, 2017

Great. Now this just needs documentation. If you want to take a crack at it you would need to modify docs/libcurl/opts CURLOPT_SSL_CTX_FUNCTION.3 to say a little something about how it differs for mbedTLS, and then the AVAILABILITY of both that and CURLOPT_SSL_CTX_DATA.3 to say Added in 7.54.0 for mbedTLS. Note 7.54.0 and not 7.53.0 since this PR is too late for tomorrow's release. If you don't want to do the doc stuff that's fine, i can do it.

@o7alesmlakar

This comment has been minimized.

Show comment
Hide comment
@o7alesmlakar

o7alesmlakar Feb 21, 2017

Contributor

Ok, I can give it a shot, except if you think it'll just make more work for you to do all the checks and reviews etc.

Contributor

o7alesmlakar commented Feb 21, 2017

Ok, I can give it a shot, except if you think it'll just make more work for you to do all the checks and reviews etc.

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Feb 21, 2017

Member

no way go for it, if adjustments are needed so be it

Member

jay commented Feb 21, 2017

no way go for it, if adjustments are needed so be it

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Feb 26, 2017

Member

This may or may not need changes. During review I noticed CURLOPT_SSL_CTX_FUNCTION says re ctx pointer "pointer will be a new one every time". At the current time that is not guaranteed, even for other backends. I've submitted #1290 and will see what happens there first.

Member

jay commented Feb 26, 2017

This may or may not need changes. During review I noticed CURLOPT_SSL_CTX_FUNCTION says re ctx pointer "pointer will be a new one every time". At the current time that is not guaranteed, even for other backends. I've submitted #1290 and will see what happens there first.

@jay jay closed this in a360906 Mar 22, 2017

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Mar 22, 2017

Member

Thanks

Member

jay commented Mar 22, 2017

Thanks

@lock lock bot locked as resolved and limited conversation to collaborators May 24, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.