Stop advertising the unimplementable AES-GCM suites

The AES-GCM symmetric suites were advertised but could never be used.
bitcoin-hpke removed its AES-GCM schemes in 0.13.0, and `dispatch_hpkes_new!`
only ever mapped ChaCha20Poly1305, so a peer that honoured the advertisement
and selected AES-GCM got `Err(InvalidKeyType)`. `Config::supported()` returned
true for it anyway, and the test constants listed it first, which is why 11 of
this crate's own tests failed. Correcting `supported()` makes
`strip_unsupported` prune the advertised KeyConfig automatically. The `Aead`
enum keeps its GCM variants so other peers' configs still parse.

`decode` built its probe config with `Aes128Gcm` on the grounds that "the KDF
and AEAD doesn't matter here"; it does now, since the probe is checked against
`supported()`, so it uses ChaCha20Poly1305.

`decode` also rejects a config whose suite list is empty after pruning, with
`Error::Unsupported`. Without that, a client handed a GCM-only config reached
`ClientRequest::from_config`, which selects `symmetric[0]`, and panicked on the
empty list where `main` returned `InvalidKeyType`. `decode_list` already skips
`Unsupported` entries, so a list containing such a config decodes to the usable
remainder. A test covers both paths. The example server no longer requests
AES-128-GCM, which it would only have had stripped.

Two tests were stale from before the secp256k1 port and never passed:
`derive_key_pair`'s expected config encoded a 32-byte X25519 key under KEM
0x0020, regenerated here for KEM 0x0016 with a 65-byte key; and
`truncate_kdf_aead_list` hard-coded an offset that assumed the X25519 key size,
so it now derives the offset from the encoding.

Also fixes two lints current clippy rejects under this crate's
`deny(warnings, clippy::pedantic)`: a redundant `continue` and non-inlined
format args.

Author: DanGould

ohttp-server/src/main.rs

@@ -108,10 +108,7 @@ async fn main() -> Res<()> {
     let config = KeyConfig::new(
         0,
         Kem::K256Sha256,
-        vec![
-            SymmetricSuite::new(Kdf::HkdfSha256, Aead::Aes128Gcm),
-            SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305),
-        ],
+        vec![SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)],
     )?;
     let ohttp = OhttpServer::new(config)?;
     println!(

ohttp/src/config.rs

@@ -181,8 +181,9 @@ impl KeyConfig {
         let key_id = r.read_u8()?;
         let kem = Kem::try_from(r.read_u16::<NetworkEndian>()?)?;
 
-        // Note that the KDF and AEAD doesn't matter here.
-        let kem_config = HpkeConfig::new(kem, Kdf::HkdfSha256, AeadId::Aes128Gcm);
+        // Only the KEM is being validated here. The KDF and AEAD are filled with a
+        // pair this backend implements so the check cannot fail on them.
+        let kem_config = HpkeConfig::new(kem, Kdf::HkdfSha256, AeadId::ChaCha20Poly1305);
         if !kem_config.supported() {
             return Err(Error::Unsupported);
         }
@@ -210,6 +211,12 @@ impl KeyConfig {
         }
 
         Self::strip_unsupported(&mut symmetric, kem);
+        // A config whose every suite is one this backend cannot perform is unusable.
+        // Report it like an unsupported KEM, so `decode_list` skips it and a client
+        // never selects from an empty suite list.
+        if symmetric.is_empty() {
+            return Err(Error::Unsupported);
+        }
         let pk = HpkeR::decode_public_key(kem_config.kem(), &pk_buf)?;
 
         Ok(Self {
@@ -241,7 +248,7 @@ impl KeyConfig {
             r.consume(len);
             match res {
                 Ok(config) => configs.push(config),
-                Err(Error::Unsupported) => continue,
+                Err(Error::Unsupported) => {}
                 Err(e) => return Err(e),
             }
         }
@@ -278,10 +285,8 @@ mod test {
 
     const KEY_ID: KeyId = 1;
     const KEM: Kem = Kem::K256Sha256;
-    const SYMMETRIC: &[SymmetricSuite] = &[
-        SymmetricSuite::new(Kdf::HkdfSha256, Aead::Aes128Gcm),
-        SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305),
-    ];
+    const SYMMETRIC: &[SymmetricSuite] =
+        &[SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)];
 
     #[test]
     fn encode_decode_config_list() {
@@ -364,13 +369,41 @@ mod test {
     fn truncate_kdf_aead_list() {
         init();
 
-        let mut x25519 = KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC))
+        let mut encoded = KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC))
             .unwrap()
             .encode()
             .unwrap();
-        x25519.truncate(38);
-        assert_eq!(usize::from(x25519[36]), SYMMETRIC.len() * 4);
-        x25519[36] = 1;
-        assert!(matches!(KeyConfig::decode(&x25519), Err(Error::Format)));
+        // The suite list sits at the end, preceded by its u16 length; derive the offset
+        // rather than hard-coding one, since the public key size depends on the KEM.
+        let len_lo = encoded.len() - SYMMETRIC.len() * 4 - 1;
+        assert_eq!(usize::from(encoded[len_lo]), SYMMETRIC.len() * 4);
+        // A length that isn't a whole number of suites must be rejected.
+        encoded[len_lo] = 1;
+        assert!(matches!(KeyConfig::decode(&encoded), Err(Error::Format)));
+    }
+
+    /// A config that offers only suites this backend cannot perform is unsupported,
+    /// not a config with nothing in it.
+    #[test]
+    fn decode_rejects_config_with_no_supported_suite() {
+        init();
+
+        let mut encoded = KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC))
+            .unwrap()
+            .encode()
+            .unwrap();
+        // The AEAD id is the last field of the last suite; swap it for AES-128-GCM.
+        let aead_at = encoded.len() - 2;
+        encoded[aead_at..].copy_from_slice(&u16::from(Aead::Aes128Gcm).to_be_bytes());
+        assert!(matches!(
+            KeyConfig::decode(&encoded),
+            Err(Error::Unsupported)
+        ));
+
+        // In a list, such a config is skipped rather than failing the whole list.
+        let mut list = Vec::new();
+        list.extend_from_slice(&u16::try_from(encoded.len()).unwrap().to_be_bytes());
+        list.extend_from_slice(&encoded);
+        assert!(KeyConfig::decode_list(&list).unwrap().is_empty());
     }
 }

ohttp/src/lib.rs

@@ -323,10 +323,8 @@ mod test {
 
     const KEY_ID: KeyId = 1;
     const KEM: Kem = Kem::K256Sha256;
-    const SYMMETRIC: &[SymmetricSuite] = &[
-        SymmetricSuite::new(Kdf::HkdfSha256, Aead::Aes128Gcm),
-        SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305),
-    ];
+    const SYMMETRIC: &[SymmetricSuite] =
+        &[SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)];
 
     const REQUEST: &[u8] = &[
         0x00, 0x03, 0x47, 0x45, 0x54, 0x05, 0x68, 0x74, 0x74, 0x70, 0x73, 0x0b, 0x65, 0x78, 0x61,
@@ -473,11 +471,15 @@ mod test {
             0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
             0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18,
         ];
+        // key_id 0x01, KEM 0x0016 (DHKEM secp256k1, HKDF-SHA256), a 65-byte uncompressed
+        // public key, then a 4-byte suite list of HKDF-SHA256 + ChaCha20Poly1305.
         const EXPECTED_CONFIG: &[u8] = &[
-            0x01, 0x00, 0x20, 0xfc, 0x01, 0x38, 0x93, 0x64, 0x10, 0x31, 0x1a, 0x0c, 0x64, 0x1a,
-            0x5c, 0xa0, 0x86, 0x39, 0x1d, 0xe8, 0xe7, 0x03, 0x82, 0x33, 0x3f, 0x6d, 0x64, 0x49,
-            0x25, 0x21, 0xad, 0x7d, 0xc7, 0x8a, 0x5d, 0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00,
-            0x01, 0x00, 0x03,
+            0x01, 0x00, 0x16, 0x04, 0x43, 0xa4, 0xf2, 0x16, 0x79, 0xd7, 0x31, 0x3b, 0x32, 0xbf,
+            0xc9, 0x8c, 0xb7, 0x75, 0xbd, 0xa9, 0xf8, 0x76, 0xb6, 0x0c, 0xe8, 0x92, 0xd1, 0xaf,
+            0xc9, 0xf6, 0xcf, 0x74, 0xb8, 0x15, 0xd3, 0x42, 0x74, 0xe3, 0xce, 0x9d, 0x68, 0x24,
+            0xa8, 0xc8, 0xa5, 0xf5, 0x45, 0x1c, 0x2a, 0x1c, 0xda, 0xda, 0x5d, 0x2d, 0x87, 0x48,
+            0xd5, 0x40, 0xdd, 0xb6, 0xa8, 0x37, 0x70, 0xca, 0x47, 0x13, 0x62, 0x27, 0x00, 0x04,
+            0x00, 0x01, 0x00, 0x03,
         ];
 
         init();

ohttp/src/rh/hpke.rs

@@ -42,7 +42,7 @@ impl Config {
 
     pub fn supported(self) -> bool {
         // TODO support more options
-        self.kdf == Kdf::HkdfSha256 && matches!(self.aead, Aead::Aes128Gcm | Aead::ChaCha20Poly1305)
+        self.kdf == Kdf::HkdfSha256 && self.aead == Aead::ChaCha20Poly1305
     }
 }
 
@@ -401,7 +401,7 @@ pub fn generate_key_pair(kem: Kem) -> Res<(PrivateKey, PublicKey)> {
             (PrivateKey::K256(sk), PublicKey::K256(pk))
         }
     };
-    trace!("Generated key pair: sk={:?} pk={:?}", sk, pk);
+    trace!("Generated key pair: sk={sk:?} pk={pk:?}");
     Ok((sk, pk))
 }
 
@@ -413,7 +413,7 @@ pub fn derive_key_pair(kem: Kem, ikm: &[u8]) -> Res<(PrivateKey, PublicKey)> {
             (PrivateKey::K256(sk), PublicKey::K256(pk))
         }
     };
-    trace!("Derived key pair: sk={:?} pk={:?}", sk, pk);
+    trace!("Derived key pair: sk={sk:?} pk={pk:?}");
     Ok((sk, pk))
 }
 
@@ -462,11 +462,6 @@ mod test {
         assert_eq!(&pt[..], PT);
     }
 
-    #[test]
-    fn seal_open_gcm() {
-        seal_open(Aead::Aes128Gcm, Kem::K256Sha256);
-    }
-
     #[test]
     fn seal_open_chacha() {
         seal_open(Aead::ChaCha20Poly1305, Kem::K256Sha256);