rust-hpke icon indicating copy to clipboard operation
rust-hpke copied to clipboard

Missing `VerifyPSKInputs` check in the key schedule

Open gendx opened this issue 1 year ago • 2 comments

According to RFC 9180 section 5.1, the first step of the KeySchedule() function is to run the validation function VerifyPSKInputs(), which checks that:

  • psk and psk_id are either both empty or both non-empty,
  • they are not empty in PSK mode (and they are empty in non-PSK mode).

These checks are emphasized with "MUST" language:

The psk and psk_id fields MUST appear together or not at all. That is, if a non-default value is provided for one of them, then the other MUST be set to a non-default value. This requirement is encoded in VerifyPSKInputs() below.

In the hpke crate, the RFC pseudo-code is pasted as comment for the derive_enc_ctx() function ; however I didn't see the VerifyPSKInputs() check implemented in the function body.

As far as I can tell, the hpke crate represents the PSK state as part of the PskBundle struct, which has public fields (https://github.com/rozbb/rust-hpke/blob/main/src/op_mode.rs). Given that these fields are public, it may be fine to let the user bear the responsibility of setting consistent (i.e. non-empty) values, but the documentation doesn't look strong enough (it mentions "psk MUST contain at least 32 bytes of entroy", but gives no requirement for psk_id).

In any case, adding a check within derive_enc_ctx() and either returning an error or panicking in case the PSK bundle is invalid would be good defense in depth.

gendx avatar Oct 01 '24 15:10 gendx

This is a great point, thank you! It might be a breaking change. I can draft this this weekend

rozbb avatar Nov 10 '24 00:11 rozbb

Housekeeping question -- was this issue addressed by https://github.com/rozbb/rust-hpke/pull/72?

timmc avatar Jul 04 '25 14:07 timmc