search

h2o / picotls

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

External PSK auth may fail with missing_extension (109) #534

Open sshock opened 3 months ago

sshock commented 3 months ago

Hi, this is not too serious for me, but I noticed a slight behavior change with external PSK auth.

I tested with the following 3 strategies (in ptls_context_t):

Earlier when I was using rev d4987ca (on kazuho/psk2 branch) I saw this behavior:

client NO_KEY_SHARE client DEFAULT client DHE_REQUIRED
server NO_KEY_SHARE PSK-only PSK-only handshake_failure
server DEFAULT PSK-only PSK-only PSK+DHE
server DHE_REQUIRED handshake_failure PSK+DHE PSK+DHE

Now after upgrading to latest master, I see this behavior (two scenarios changed, marked with a ✱):

client NO_KEY_SHARE client DEFAULT client DHE_REQUIRED
server NO_KEY_SHARE PSK-only PSK-only handshake_failure
server DEFAULT missing_extension ✱ PSK-only PSK+DHE
server DHE_REQUIRED missing_extension ✱ PSK+DHE PSK+DHE

I think the change from handshake_failure to missing_extension in the scenario where client has NO_KEY_SHARE and server has DHE_REQUIRED is fine and good. We're still failing, just with a different and arguably better error. I think the new error makes a lot of sense, given that the key_share is missing but the server requires it in this case.

The one I am concerned about is the change where client has NO_KEY_SHARE and server is DEFAULT. This was succeeding before and using PSK-only mode, but now it fails with missing_extension. Is this an intentional change? Why should this not succeed?

Thanks!

sshock commented 3 months ago

The one I am concerned about is the change where client has NO_KEY_SHARE and server is DEFAULT. This was succeeding before and using PSK-only mode, but now it fails with missing_extension. Is this an intentional change? Why should this not succeed?

Debugging the code I can see it hits PTLS_ALERT_MISSING_EXTENSION in picotls.c on line 4522 due to no negotiated groups.

Part of why it enters this block is because ch->key_shares.base != NULL. I can see why this happens because the client actually does send a key_share, although it's empty (no entries).

Perhaps if I were able to make the client omit the key_share completely, then I would see the old behavior; but picotls doesn't provide a way to do this.

So my two takeaways so far are:

  1. Perhaps picotls should provide a way for client to actually omit the key_share. E.g., maybe when key_exchanges is NULL picotls should omit it completely. And when key_exchanges is set to { NULL } that is when it could send a key_share with no entries.
  2. Even if the client includes an empty key_share, as long as the key exchange modes includes psk_ke, the server ought to be able to succeed (with PSK-only mode).