h2o / picotls

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

Add support for chacha20 cipher prioritization #476

Closed robguima closed 1 year ago

robguima commented 1 year ago

Add a new boolean flag to determine if chacha20 should be reprioritized to the top of the server cipher list in case it happens to appear at the top of the client cipher list. This is meant to cater to clients which may lack AES HW acceleration, and/or may prefer chacha20.

It should work similarly to OpenSSL's SSL_OP_PRIORITIZE_CHACHA CTX option.

robguima commented 1 year ago

Thank you for the PR. Looks good, I only wonder if could simplify the logic by considering the server_chacha_priority a mode that switches to client-driven mode when the first client-side entry is chachapoly. Please see b74e8a8.

Ah you are right, I realized later (when writing tests) that the chacha switch was essentially just client preference when chacha it top (and chacha is in the server list), but did not think of combining the two 😞 . https://github.com/h2o/picotls/commit/b74e8a8641a7fd13289ae3aa6be2e18d2dedcb20 looks much better, thanks! 👍🏻 ~Mind pushing it to the branch or should I~ ~I suspect it will fail when the server does not have chacha in its list tho. Let me push a new test for that.~ incorrect--works just fine in that case.

robguima commented 1 year ago

Pushed the new test 9918937 which ensures server preference is honored when chacha isn't present in the server list. https://github.com/h2o/picotls/commit/b74e8a8641a7fd13289ae3aa6be2e18d2dedcb20 still looks good 👍🏻 mind pushing it? 🙇🏻

kazuho commented 1 year ago

Thank you! Merged.