h2o / picotls

TLS 1.3 implementation in C (master supports RFC8446 as well as draft-26, -27, -28)
527 stars 140 forks source link

adds TLS13 support for ptls_import() and ptls_export() #513

Closed robguima closed 4 months ago

robguima commented 5 months ago

This adds the ability to transfer the traffic secrets from one TLSv1.3 tls structure to another newly created one. New helper functions are added to t/picotls.c: alloc_and_migrate_tls_context() is a function that simulates the usage of import/export in different code paths and handshake types; aead_keys_cmp() compares secrets and keys (sequence numbers as well) to determine whether the transfer was correctly done.

@kazuho was in the loop as an active contributor, but we both agreed the PoC (private branch) code needed some cleanup--some of which already included in this PR.

One new thing form the PoC was adding an explicit call to key_schedule_new() to create a new key_schedule , so that key updates would work (tests that include key updates would work as well).

robguima commented 5 months ago

@kazuho thank you, 381a154 should address your concerns plmk.

I'm not sure 61e0403 works as a fix, but my thinking is that that's a handshake time question, so the "migrated" tls may indeed not contain the information in question.

kazuho commented 5 months ago

@robguima Thank you for the changes!

I've pushed a couple of changes. Could you please take a look? I think we can merge this PR if they look fine.

I'm not sure 61e0403 works as a fix, but my thinking is that that's a handshake time question, so the "migrated" tls may indeed not contain the information in question.

I think the changes would be fine for the time being. I've added a FIXME comment to ptls_is_ech_handshake; if we end up wanting to log values specific to ECH, we can change the code to do whatever we want.

robguima commented 5 months ago

@kazuho the changes look good, thank you. The cosmetic changes also make sense -- I considered using the word "clone" in the test, but thought it would've been a loaded word like "dupe" as well. 😄 Dang we were already setting state in ptls_import() of course -- it wouldn't quite work otherwise. 👍🏻

kazuho commented 4 months ago

@robguima Thank you for checking!