Add PSK bundle validation (#72)

* Made PskBundle require a constructor that now validates inputs

* Updated CHANGELOG

* Oops forgot to update KAT and example

* Fixed typo

Author: rozbb

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+* Made `PskBundle` require an explicit constructor that performs validation on inputs
+
 ## [0.12.0] - 2024-07-03
 
 ### Additions

examples/agility.rs

@@ -702,10 +702,7 @@ fn main() {
                 let psk_id = b"preshared key attempt #5, take 2. action";
                 let psk_bundle = {
                     csprng.fill_bytes(&mut psk_bytes);
-                    AgilePskBundle(PskBundle {
-                        psk: &psk_bytes,
-                        psk_id,
-                    })
+                    AgilePskBundle(PskBundle::new(&psk_bytes, psk_id).unwrap())
                 };
 
                 // Make two agreeing OpModes (AuthPsk is the most complicated, so we're just using

src/kat_tests.rs

@@ -222,10 +222,7 @@ fn make_op_mode_r<'a, Kem: KemTrait>(
     psk_id: Option<&'a [u8]>,
 ) -> OpModeR<'a, Kem> {
     // Deserialize the optional bundle
-    let bundle = psk.map(|bytes| PskBundle {
-        psk: bytes,
-        psk_id: psk_id.unwrap(),
-    });
+    let bundle = psk.map(|bytes| PskBundle::new(bytes, psk_id.unwrap()).unwrap());
 
     // These better be set if the mode ID calls for them
     match mode_id {

src/lib.rs

@@ -164,6 +164,8 @@ pub enum HpkeError {
     /// An input isn't the right length. First value is the expected length, second is the given
     /// length.
     IncorrectInputLength(usize, usize),
+    /// A preshared key bundle was constructed incorrectly
+    InvalidPskBundle,
 }
 
 impl core::fmt::Display for HpkeError {
@@ -181,6 +183,9 @@ impl core::fmt::Display for HpkeError {
                 "Incorrect input length. Expected {} bytes. Got {}.",
                 expected, given
             ),
+            HpkeError::InvalidPskBundle => {
+                write!(f, "Preshared key bundle is missing a key or key ID")
+            }
         }
     }
 }

src/op_mode.rs

@@ -1,19 +1,36 @@
-use crate::kem::Kem as KemTrait;
+use crate::{kem::Kem as KemTrait, HpkeError};
 
 /// Contains preshared key bytes and an identifier. This is intended to go inside an `OpModeR` or
 /// `OpModeS` struct.
-///
-/// Requirements
-/// ============
-/// `psk` MUST contain at least 32 bytes of entropy. Further, `psk.len()` SHOULD be at least as
-/// long as an extracted key from the KDF you use with `setup_sender`/`setup_receiver`, i.e., at
-/// least `Kdf::extracted_key_size()`.
 #[derive(Clone, Copy)]
 pub struct PskBundle<'a> {
     /// The preshared key
-    pub psk: &'a [u8],
+    psk: &'a [u8],
     /// A bytestring that uniquely identifies this PSK
-    pub psk_id: &'a [u8],
+    psk_id: &'a [u8],
+}
+
+impl<'a> PskBundle<'a> {
+    /// Creates a new preshared key bundle from the given preshared key and its ID
+    ///
+    /// Errors
+    /// ======
+    /// `psk` and `psk_id` must either both be empty or both be nonempty. If one is empty while
+    /// the other is not, then this returns [`HpkeError::InvalidPskBundle`].
+    ///
+    /// Other requirements
+    /// ==================
+    /// Other requirements from the HPKE spec: `psk` MUST contain at least 32 bytes of entropy.
+    /// Further, `psk.len()` SHOULD be at least as long as an extracted key from the KDF you use
+    /// with `setup_sender`/`setup_receiver`, i.e., at least `Kdf::extracted_key_size()`.
+    pub fn new(psk: &'a [u8], psk_id: &'a [u8]) -> Result<Self, HpkeError> {
+        // RFC 9180 §5.1: The psk and psk_id fields MUST appear together or not at all
+        if (psk.is_empty() && psk_id.is_empty()) || (!psk.is_empty() && !psk_id.is_empty()) {
+            Ok(PskBundle { psk, psk_id })
+        } else {
+            Err(HpkeError::InvalidPskBundle)
+        }
+    }
 }
 
 /// The operation mode of the HPKE session (receiver's view). This is how the sender authenticates
@@ -23,7 +40,8 @@ pub struct PskBundle<'a> {
 pub enum OpModeR<'a, Kem: KemTrait> {
     /// No extra information included
     Base,
-    /// A preshared key known to the sender and receiver
+    /// A preshared key known to the sender and receiver. If the bundle contents is empty strings,
+    /// then this is equivalent to `Base`.
     Psk(PskBundle<'a>),
     /// The identity public key of the sender
     Auth(Kem::PublicKey),
@@ -50,7 +68,8 @@ impl<Kem: KemTrait> OpModeR<'_, Kem> {
 pub enum OpModeS<'a, Kem: KemTrait> {
     /// No extra information included
     Base,
-    /// A preshared key known to the sender and receiver
+    /// A preshared key known to the sender and receiver. If the bundle contents is empty strings,
+    /// then this is equivalent to `Base`.
     Psk(PskBundle<'a>),
     /// The identity keypair of the sender
     Auth((Kem::PrivateKey, Kem::PublicKey)),
@@ -148,3 +167,12 @@ impl<Kem: KemTrait> OpMode<Kem> for OpModeS<'_, Kem> {
         }
     }
 }
+
+// Test that you can only make a PskBundle if both fields are empty or both fields are nonempty
+#[test]
+fn psk_bundle_validation() {
+    assert!(PskBundle::new(b"hello", b"world").is_ok());
+    assert!(PskBundle::new(b"", b"").is_ok());
+    assert!(PskBundle::new(b"hello", b"").is_err());
+    assert!(PskBundle::new(b"", b"world").is_err());
+}

src/single_shot.rs

@@ -177,10 +177,7 @@ mod test {
                 // Set up an arbitrary info string, a random PSK, and an arbitrary PSK ID
                 let info = b"why would you think in a million years that that would actually work";
                 let (psk, psk_id) = (gen_rand_buf(), gen_rand_buf());
-                let psk_bundle = PskBundle {
-                    psk: &psk,
-                    psk_id: &psk_id,
-                };
+                let psk_bundle = PskBundle::new(&psk, &psk_id).unwrap();
 
                 // Generate the sender's and receiver's long-term keypairs
                 let (sk_sender_id, pk_sender_id) = Kem::gen_keypair(&mut csprng);

src/test_util.rs

@@ -80,7 +80,7 @@ pub(crate) fn new_op_mode_pair<'a, Kdf: KdfTrait, Kem: KemTrait>(
 ) -> (OpModeS<'a, Kem>, OpModeR<'a, Kem>) {
     let mut csprng = StdRng::from_os_rng();
     let (sk_sender, pk_sender) = Kem::gen_keypair(&mut csprng);
-    let psk_bundle = PskBundle { psk, psk_id };
+    let psk_bundle = PskBundle::new(psk, psk_id).unwrap();
 
     match kind {
         OpModeKind::Base => {