private-octopus / picoquic

Minimal implementation of the QUIC protocol
MIT License
523 stars 153 forks source link

picoquic_set_tls_key is missing implementation #1585

Open mludha opened 7 months ago

mludha commented 7 months ago

I believe there might be an oversight in the following commit: https://github.com/private-octopus/picoquic/commit/3ef5498fa1cc41be2ef5d7c344f52334e5602020#diff-af950564dc228efe7ad7171ffa32d1ea56ed4090cbf57daa24ddf44a76613376L558 It removes the function picoquic_set_tls_key from tls_api.c, probably because it has been used only in a test that is also being removed there. But the function is also declared in tls_api.h, am I assuming correctly that this means it is a part of public interface? My use case is that I'd like to set the key without having to store it in a file first, and this used to be the only way that I know of.

huitema commented 7 months ago

You are right, we need a way to set the TLS server key from memory. But no, "tls_api.h" is not the public API. The public APIs are defined in "picoquic.h":

/* Set the TLS private key(DER format) for the QUIC context. The caller is responsible for cleaning up the pointer. */
int picoquic_set_tls_key(picoquic_quic_t* quic, const uint8_t* data, size_t len);

/* Set the verify certificate callback and context. */
typedef struct st_ptls_verify_certificate_t ptls_verify_certificate_t;
int picoquic_set_verify_certificate_callback(picoquic_quic_t* quic, 
    ptls_verify_certificate_t * cb, picoquic_free_verify_certificate_ctx free_fn);

Except, as you noticed, these two APIs do not work. I have to finish fixing two other issues before working on that, so it will take some time, unless of course someone can propose a PR.

davidk-ad8 commented 1 week ago

Do you mean the previous implementation of picoquic_set_tls_key was removed because it didn't work?

huitema commented 1 week ago

The problem is that the proposed API passes the key in clear text. We need a version of the API in which we can either pass an encrypted key, or better an implementation that can pass an opaque handle to a key, as defined by the backend. That would be more secure.

davidk-ad8 commented 1 week ago

Right so for OpenSSL we would be passing an opaque pointer to an EVP_PKEY. I may try implement this for the OpenSSL backend soon.