babelouest / rhonabwy

Javascript Object Signing and Encryption (JOSE) library - JWK, JWKS, JWS, JWE and JWT
https://babelouest.github.io/rhonabwy/
GNU Lesser General Public License v2.1
45 stars 21 forks source link

specify non-default path to CA cert when importing JWK(S) from remote server #21

Closed metalalive closed 2 years ago

metalalive commented 2 years ago

Hi , rhonabwy requires libcurl when importing JWK(S) from remote server , and libcurl uses /etc/ssl/cert as the default path to CA cert (according to the command curl-config --ca).

In my case, I need to specify different CA certs (including Let'sencrypt or self-signed) to different test cases in my project, so would it be good to have an option to let application callers specify CA path in each HTTP request (e.g. made by curl_easy_perform()) ? AFAIK _r_get_http_content(...) loads serialized JWK(S) from remote server then following functions calling _r_get_http_content(...) decodes the response body :

r_jwk_key_type(...) r_jwk_import_from_x5u(...) r_jwk_export_to_gnutls_pubkey(...) r_jwk_export_to_gnutls_crt(...) r_jwks_import_from_uri(...)

The functions above could probably have extra argument for CA path , but I am not sure if you have better design option and plan to do it in future release ? thanks for reading.

babelouest commented 2 years ago

Hi @metalalive ,

That's a tricky question.

The option x5u_flags passed to multiple functions was designed to set these kind of options, but as flags, no more. Therefore I don't think I can add x5u_flags values to meet this specific needs.

A workaround, which is not a solution, is to use the value R_FLAG_IGNORE_SERVER_CERTIFICATE as value for x5u_flags. So you can ignore invalid remote server certificate.

For now, I see 2 different ways to fit your needs: 1- add clone functions, like

int r_jwk_key_type_advanced(jwk_t * jwk, unsigned int * bits, int x5u_flags, ...)

2- set some curl options as global variables that would be read and parsed by the _r_get_http_content

But I don't find those solutions valid, and it could add side bugs difficult to fix.

metalalive commented 2 years ago

@babelouest , I know that it is also good to keep function interface simple , and yes , since x5u_flags is just a flag, it doesn't provide more detailed parameters to set up a secure connection (e.g. CA path in my case).

My current workaround is to clone the functions as mentioned in your comment, except I replaced the argument x5u_flags with a new type (say struct x5u_t) in order to provide more detail, such as:

typedef struct {
    const char *ca_path;
    int  flags; // original x5u_flags is moved here
   // ... extend to more parameters if necessary ... 
} x5u_t;

char * _r_get_http_content_advanced(const char * url,  x5u_t  *x5u, const char * expected_content_type) {
     // .... everything else is the same ....
         if (curl_easy_setopt(curl, CURLOPT_CAPATH, x5u->ca_path) != CURLE_OK) {
              break;
         }
     // .... everything else is the same ....
}

int r_jwks_import_from_uri_advanced(jwks_t * jwks, const char * uri, x5u_t  *x5u) {
     // .... everything else is the same ....
     if (jwks != NULL && uri != NULL) {
         if ((x5u_content = _r_get_http_content_advanced(uri, x5u, "application/json")) != NULL) {
              // .... everything else is the same ....
         }
     }
     // .... everything else is the same ....
}

Currently I couldn't think of any potential issues / side bugs caused by the workaround, I would probably continue going with that before I can figure out any better design . Anyway thanks again.

babelouest commented 2 years ago

Hello @metalalive ,

I was thinking about your feature request and I came with an idea that might help you without breaking the structures and the compatibility: environment variables.

I can add a secure_getenv call with the value RHN_X5U_CA_PATH in _r_get_http_content. Like that, you should have to set an environment variable before running your program:

$ export RHN_X5U_CA_PATH=/path/to/my/ca
$ ./my_program_that_gets_remote_jwks_with_rhonabwy
babelouest commented 2 years ago

@metalalive , any update on the proposed improvement?

metalalive commented 2 years ago

@babelouest the environment variable approach also works well to my current project, thank you for the help