async public key operations #291
Conversation
| static int send_certificate_verify(ptls_t *tls, ptls_message_emitter_t *emitter, | ||
| struct st_ptls_signature_algorithms_t *signature_algorithms, const char *context_string) | ||
| { | ||
| size_t start_off = emitter->buf->off; |
There was a problem hiding this comment.
The second run here the start_off will be 0
There was a problem hiding this comment.
It can be either.
When calling ptls_handshake, it is up to the application if it provides a fresh output buffer, or a buffer that already has some data.
| static int sign_certificate(ptls_sign_certificate_t *_self, ptls_t *tls, uint16_t *selected_algorithm, ptls_buffer_t *outbuf, | ||
| ptls_iovec_t input, const uint16_t *algorithms, size_t num_algorithms) | ||
| static int sign_certificate(ptls_sign_certificate_t *_self, ptls_t *tls, void **sign_ctx, uint16_t *selected_algorithm, | ||
| ptls_buffer_t *outbuf, ptls_iovec_t input, const uint16_t *algorithms, size_t num_algorithms) | ||
| { | ||
| ptls_openssl_sign_certificate_t *self = (ptls_openssl_sign_certificate_t *)_self; |
There was a problem hiding this comment.
The second time runs in sign_certificate, it will fail to find the scheme_md
There was a problem hiding this comment.
This is intentional.
By the time sign_certificate callback is called for the second time, the signature using an algorithm selected by the async crypto would already be available. The callback no longer needs access to the list of algorithms that the client has offered.
I understand it causes a bit of pain when implementing a fake sign_certificate callback, but my preference goes to keeping the TLS stack simple. FWIW, the fake callback added in 5dcab74 caches the selected algorithm.
There was a problem hiding this comment.
yes, I saw it, and it works perfectly.
|
@pingyucn Thank you for the review. I have added unit test reusing your idea of having a async callback that wraps an existing implementation, and that have led me to find and fix some bugs. To be specific, the async callback that has been introduced is the |
|
Just tried the latest version the async mode, and the test works fine. I will incorporate those code to QAT to boost the performance. |
| ptls_buffer_push_block(sendbuf, 2, { | ||
| uint16_t algo; | ||
| uint8_t data[PTLS_MAX_CERTIFICATE_VERIFY_SIGNDATA_SIZE]; | ||
| size_t datalen = build_certificate_verify_signdata(data, tls->key_schedule, context_string); |
There was a problem hiding this comment.
This function will be called twice, and we may save the info for efficiency
There was a problem hiding this comment.
Indeed!
I think there is no need to generate signdata when this send_certificate_verify is called the second time. I will update the code so that the sixth argument to the sign_certificate callback would be ptls_iovec_init(NULL, 0) when send_certificate_verify is called the second time.
| assert(tls->is_server || !"async operation only supported on the server-side"); | ||
| /* Reset the output to the end of the previous handshake message. CertificateVerify will be rebuilt when the | ||
| * async operation completes. */ | ||
| emitter->buf->off = start_off; |
There was a problem hiding this comment.
if returning to the start, then it requires to push message again. Could we optimize it by keeping this offset?
There was a problem hiding this comment.
We cannot keep the offset, as the application is allowed to modify the buffer once ptls_handshake returns, or to pass a different buffer which might be empty when calling ptls_handshake the second time. Therefore, the only feasible option would be to cache the bytes that we have built for the CertificateVerify message and reuse it.
That said, I do not think it's worth the effort. There are very few amount of work done before reaching this line. The cost of redoing them is going to be much much less than encrypting the TLS record that would contain the CertificateVerify message.
This is an attempt to extend the picotls callback API to support asynchronous public key operations. I think we would be interested in supporting async operations for both key exchange and signing, but at the moment this PR implements only the latter.
In short, this PR extends the
sign_certificatecallback in the following ways:PTLS_ERROR_ASYNC_OPERATION. The callback is expected to return this error code when it starts an asynchronous public key crypto calculation. This error code is propagated back to the application.sign_certificatecallback and the application.ptls_handshake. Then, picotls would call thesign_certificatecallback with the samectxparameter. The callback then should write back the results of the public key operation, and the handshake continues.