ssl: Allow empty psk identity

[ausimian]

Relax the psk_identity validation to allow the client to provide an empty string.

[github-actions]

CT Test Results

    2 files     66 suites   25m 50s ⏱️
  815 tests   772 ✅  43 💤 0 ❌
4 184 runs  3 384 ✅ 800 💤 0 ❌

Results for commit 9a5f7d3.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• No Windows Installer found

// Erlang/OTP Github Action Bot

[ausimian]

For background, I'm attempting to connect to 'developer services' on iPhones running 17.0 <= iOS < 17.4. An SRP handshake establishes a session key which acts as the PSK for a TLSv1.2 service running on such devices. However the service expects that an empty PSK identity is passed, and rejects the connection otherwise.

To confirm, this still requires the client to explicitly provide the option:

{psk_identity, ""}

The default remains undefined if no such option is provided

[ausimian]

Dunno if i did the right thing, but i noticed there were merge conflicts against maint, i assume 28 or something got merged in there - anyway I redid the change to reflect some of the ssl re-org in 28. I'd personally like to see this change back-ported to 27 etc, as 27 is what we're running on rn and happy to submit fixes for that if it's a thing.

[IngelaAndin]

@ausimian When a new major OTP release is produced what will happen is also that the maint branch will changed to point to the new release and master will become the development branch for the next-major release. We have not had time to handle your PR yet hope to have time to take a closer look soonish. If we decide to include it you can make a branch based on the tag of the latest OTP-27 patch and we could piggyback it on some other OTP-27 patch build.

[IngelaAndin]

Defenition from RFC 4279 seems to allow it to be empty so I think this should be ok.

opaque psk_identity_hint<0..2^16-1>;
          };

[IngelaAndin]

I think it would make sense to 0 <= PSKSize andalso PSKSize < 65536 when Role == client
for interop reasons that you have, but not allow this for the server as this I think it does not make sense from a security perspective.

[ausimian]

Thx for the feedback.

Obvs your call, but if the rfc actually allows it (thx for checking) I would allow it on the server. It's more a judgement call that a zero-length identity is not useful in the sense that a 1-byte identity is also not very useful, but is allowed.

If I wanted to write a server that acted identically to an existing server that was correct (but not wise!) per the rfc, restricting the server validation would prevent that.

[IngelaAndin]

At least I think we should check that it is greater than or equal to zero. Maybe we can allow it even if it was not allowed before as it sort of equally stupid as using only one byte, and all this is sort of legacy way to do things anyway. What do you think @dgud, @u3s ?

[ausimian]

re: checking >= 0, I did actually write the first version like this but dropped the clause because it appeared redundant given that byte_size/1 will always return non_neg_integer().

But happy to put it back if you think its clearer.

EDIT: I put the explicit check back in the latest commit

[IngelaAndin]

We should use client in test as I think we do not want to promote this usage in servers although it might not be forbidden by old PSK spec.

[IngelaAndin]

@ausimian I think we should tweak the implementation a little see code comments

[ausimian]

@ausimian I think we should tweak the implementation a little see code comments

I added the explicit check for a length of at least zero.

[IngelaAndin]

I suppose we can allow it as it must be configured, and if the user want to make poor configuration choices that is on them.