Add support CA bundle in memory with CURLOPT_CAINFO_BLOB and CURLOPT_PROXY_CAINFO_BLOB

[gvollant]

Adds CURLOPT_CAINFO_BLOB and CURLOPT_PROXY_CAINFO_BLOB that let applications provide the CA bundle to libcurl as a PEM formatted in-memory buffer.

This is an alternative of pr #6109 , which uses blob (introduced with #5357 on curl 7.71) instead char* string to hold PEM certificates.

On #6109 , @bagder and @jay discuss about using blob possible advantage

[gvollant]

@JCMais and @nadrolinux your test are also welcomes

[gvollant]

I don't understand the two check problem, probably not related to my code.
I don't plan other modification.
If you suggest documentation modification, can you give me exact text (as you see, my english is not perfect).

regards

[jay]

I have made some suggested changes. One of them is that the blob override regular cainfo. I don't understand why some ssl primary options are accessed by SSL_CONN_CONFIG and the others are accessed by SSL_SET_OPTION(primary.foo, for example SSL_CONN_CONFIG(CAfile) but for the blob it's SSL_SET_OPTION(primary.ca_info_blob). Why is it not SSL_CONN_CONFIG(ca_info_blob);, if a blob is copied to the connection shouldn't we use that copy or do I not understand this?

[gvollant]

SSL_CONN_CONFIG

@jay Thank you for your modification.
I switched from SSL_SET_OPTION to SSL_CONN_CONFIG for ca_info_blob, like CAfile, and merged your modification

[nadrolinux]

Glad to see progress on this stuff. Later this week I'll try to test this version on both Linux and Android.

[gvollant]

I added support on Secure Transport and Schannel backends, like others blobs
Next step is probably all blob on toher popular vtls ...

[jay]

I added support on Secure Transport and Schannel backends, like others blobs

That's great. I still don't know whether it's proper to access the blobs with SSL_SET_OPTION(primary.foo or SSL_CONN_CONFIG(foo). My understanding of the latter is it carries with the connection, so I guess is copied from the transfer. I wonder why though CAfile or ca blob needs to go with the connection. In the case of blob it just seems like an extra unnecessary copy.

[gvollant]

That's great. I still don't know whether it's proper to access the blobs with SSL_SET_OPTION(primary.foo or SSL_CONN_CONFIG(foo). My understanding of the latter is it carries with the connection, so I guess is copied from the transfer. I wonder why though CAfile or ca blob needs to go with the connection. In the case of blob it just seems like an extra unnecessary copy.

I've no opinion, but I suppose the method for CAINFO (filename) and CAINFO_BLOB must be the same. @bagder probably have an idea...

[gvollant]

I wrote on #6820 a comment about blob test.

[jay]

I wrote on #6820 a comment about blob test.

I've copied the relevant part of your comment below.

--cert "loadmem=F\:\\demosync\\cert_4\\iosad.p12:vfdffrefre" 

will load the file and use CURLOPT_SSLCERT_BLOB

That seems unnecessary for a blob test. Rather than use the curl tool to test you can use just libcurl, refer to libtest directory

[gvollant]

Do you think there are more work needed on this PR ?

[jay]

Do you think there are more work needed on this PR ?

I will review it again when the feature window opens

[gvollant]

Do you think there are more work needed on this PR ?

I will review it again when the feature window opens

Thank you. So feature window is only 2 week I believe.

[jay]

Thank you. So feature window is only 2 week I believe.

yes if there are no serious problems in 7.76.1 then it runs from next week until may 5. release calendar

[gvollant]

I rebase to master each time openssl.c is modifed.

@bagder : what is you opinion about the @jay question about SSL_SET_OPTION(primary.foo vs SSL_CONN_CONFIG(foo). ?
I just think blob and filename must use smae method for same information

[gvollant]

@bagder I wrote a simple code and uploaded it to http://gvollant.free.fr/demo_curl_portion.c

You can probably use it with a test based on test 310 , for schannel, openssl and sectransp

[gvollant]

static int test_loadfile(const char *filename, void **filedata, size_t *filesize)
{
  size_t datasize = 0;
  void *data = NULL;
  if(filename) {
    FILE *fInCert = fopen(filename, "rb");

    if(fInCert) {
      long cert_tell = 0;
      bool continue_reading = fseek(fInCert, 0, SEEK_END) == 0;
      if(continue_reading)
        cert_tell = ftell(fInCert);
      if(cert_tell < 0)
        continue_reading = FALSE;
      else
        datasize = (size_t)cert_tell;
      if(continue_reading)
         continue_reading = fseek(fInCert, 0, SEEK_SET) == 0;
      if(continue_reading)
          data = malloc(datasize + 1);
      if((!data) ||
          ((int)fread(data, datasize, 1, fInCert) != 1))
          continue_reading = FALSE;
      fclose(fInCert);
      if(!continue_reading) {
        free(data);
        datasize = 0;
        data = NULL;
      }
   }
  }
  *filesize = datasize;
  *filedata = data;
  return data ? 1 : 0;
}

CURLcode test_cert_file(const char *url, const char *cafile)
{
  CURLcode code;
  CURL *curl;

  curl = curl_easy_init();

  curl_easy_setopt(curl, CURLOPT_VERBOSE,    1L);
  curl_easy_setopt(curl, CURLOPT_URL,        url);
  curl_easy_setopt(curl, CURLOPT_CAINFO,     cafile);
  if(!curl) {
    fprintf(stderr, "curl_easy_init() failed\n");
    return CURLE_FAILED_INIT;
  }

  printf("PERFORM CURLOPT_CAINFO\n");
  code = curl_easy_perform(curl);

test_cleanup:
  printf("CLEANUP\n");
  curl_easy_cleanup(curl);
  curl_global_cleanup();

  return code;
}


CURLcode test_cert_blob(const char *url, const char *cafile)
{
  CURLcode code;
  CURL *curl;
  struct curl_blob blob;
  size_t certsize;
  void *certdata;

  curl = curl_easy_init();
  if(!curl) {
    fprintf(stderr, "curl_easy_init() failed\n");
    return CURLE_FAILED_INIT;
  }

  if(!test_loadfile(cafile, &certdata, &certsize)) {
    return CURLE_READ_ERROR;
  }

  curl_easy_setopt(curl, CURLOPT_VERBOSE,     1L);
  curl_easy_setopt(curl, CURLOPT_URL,         url);

  blob.data = certdata;
  blob.len = certsize;
  blob.flags = CURL_BLOB_COPY;
  curl_easy_setopt(curl, CURLOPT_CAINFO_BLOB, &blob);
  free(certdata);


  printf("PERFORM CURLOPT_CAINFO_BLOB\n");
  code = curl_easy_perform(curl);

test_cleanup:
  printf("CLEANUP\n");
  curl_easy_cleanup(curl);
  curl_global_cleanup();

  return code;
}


CURLcode test_cert_both(const char *url, const char *cafile)
{
    CURLcode res = test_cert_file(url, cafile);
    if(!res)
      res = test_cert_blob(url, cafile);
    return res;
}

[bagder]

I wrote this commit that adds a check for memmem so that the native function can be used on platforms that have it.

[bagder]

Here's a patch that adds test 678 that verifies CURLOPT_CAINFO_BLOB. It is based on #6662 (comment) and it worked when I try it here in my local test of this branch. It might need some adjustments to also work with torture tests (./runtests.pl -t 678) but I didn't check.

0001-test678-verify-CURLOPT_CAINFO_BLOB.patch.txt

[gvollant]

Here's a patch that adds test 678 that verifies CURLOPT_CAINFO_BLOB. It is based on #6662 (comment) and it worked when I try it here in my local test of this branch. It might need some adjustments to also work with torture tests (./runtests.pl -t 678) but I didn't check.

0001-test678-verify-CURLOPT_CAINFO_BLOB.patch.txt

I integrated your test. What about adding a section to test with openssl, schannel and sectransport only?

[gvollant]

On you file, this portion of file test678 have CRLF ending. Is it needed?

GET /%TESTNUMBER HTTP/1.1
Host: localhost:%HTTPSPORT
User-Agent: CURLOPT_CAINFO_BLOB
Accept: */*

[bagder]

this portion of file test678 have CRLF ending. Is it needed?

It is needed, and you probably see that the test fails without them. They are present in my local version, but I forgot that they don't survive a git format-patch.

[bagder]

What about adding a section to test with openssl, schannel and sectransport only?

Ah right. That needs to be handled too somehow. I'll work on it.

[gvollant]

There is fail on https://dev.azure.com/daniel0244/5571c33c-81e1-43d7-93ee-d3d4152f27b2/_apis/build/builds/5612/logs/100 and I don't have knowledge to fix them.

test 678 seem ok at https://cirrus-ci.com/task/5675663530131456

Note: You can commit on my github fork.I invited you, and I beleive as maintainer of original repo, you have modification right on my repo

[bagder]

Okay, I added a commit that makes test 678 only run if it can use the setopt. So I made the setopt fail properly if libcurl doesn't support the option in that build setup.

[gvollant]

Do you want I rebase and merge all commit ?

[gvollant]

test 678 fail at https://dev.azure.com/daniel0244/5571c33c-81e1-43d7-93ee-d3d4152f27b2/_apis/build/builds/5616/logs/163

[gvollant]

They are some schannel error

https://dev.azure.com/daniel0244/5571c33c-81e1-43d7-93ee-d3d4152f27b2/_apis/build/builds/5616/logs/137

2021-05-04T14:44:18.6522953Z  * schannel: added 1 certificate(s) from CA file '(memory blob)'
2021-05-04T14:44:18.6523738Z  * schannel: CertGetCertificateChain trust error CERT_TRUST_REVOCATION_STATUS_UNKNOWN

Pehaps solution is using CURLSSLOPT_NO_REVOKE or CURLSSLOPT_REVOKE_BEST_EFFORT on test 678

[gvollant]

It seem test failure are now not related with these PR modification. I do a rebase

[jay]

Thanks guys

[mback2k]

Actually lib678_SOURCES is missing $(MULTIBYTE) in tests\libtest\Makefile.inc since you are using fopen which is defined to curlx_win32_fopen in Windows Unicode builds. Should I do a follow up or will one of you do it? @jay @gvollant @bagder

Error can be seen here: https://cirrus-ci.com/task/4924099123216384?logs=compile#L1095

[jay]

Should I do a follow up or will one of you do it?

My fault. Thanks, landed in 5a1ec19.

[JCMais]

Awesome job you all on getting this PR merged. This feature will be very useful.