From b13fc56c4deadce4cbaeaac6b7183d0e0677484b Mon Sep 17 00:00:00 2001
From: David Cook <dcook@divviup.org>
Date: Fri, 15 Sep 2023 13:13:08 -0500
Subject: [PATCH] Gracefully handle unrecognized HPKE algorithms

---
 packages/dap/src/hpkeConfig.spec.ts | 101 ++++++++++++++++++----------
 packages/dap/src/hpkeConfig.ts      |  20 ++++--
 2 files changed, 83 insertions(+), 38 deletions(-)

diff --git a/packages/dap/src/hpkeConfig.spec.ts b/packages/dap/src/hpkeConfig.spec.ts
index e8fa3c8c9..db57bfdb5 100644
--- a/packages/dap/src/hpkeConfig.spec.ts
+++ b/packages/dap/src/hpkeConfig.spec.ts
@@ -51,29 +51,35 @@ describe("DAP HpkeConfigList", () => {
   });
 
   it("selects the first config that it recognizes", () => {
+    const list = HpkeConfigList.parse(
+      Buffer.from(
+        "0022" + // Overall length prefix
+          // Unrecognized configuration first
+          "64" + // id
+          "0064" + // kem_id
+          "0064" + // kdf_id
+          "0064" + // aead_id
+          "0008" + // Length prefix for public key
+          "4141414141414141" + // public_key
+          // Valid configuration
+          "ff" + // id
+          "0010" + // kem_id, KemId.DhkemP256HkdfSha256
+          "0001" + // kdf_id, KdfId.HkdfSha256
+          "0001" + // aead_id, AeadId.Aes128Gcm
+          "0008" + // Length prefix for public_key
+          "4141414141414141", // public_key
+        "hex",
+      ),
+    );
+
     const validConfig = new HpkeConfig(
       255,
       KemId.DhkemP256HkdfSha256,
       KdfId.HkdfSha256,
       AeadId.Aes128Gcm,
-      Buffer.from("public key"),
+      Buffer.from("AAAAAAAA"),
     );
 
-    const invalidConfig = new HpkeConfig(
-      100,
-      KemId.DhkemX25519HkdfSha256,
-      KdfId.HkdfSha512,
-      AeadId.Chacha20Poly1305,
-      Buffer.from("public key"),
-    );
-
-    // none of these are known ids, so we skip the invalid config
-    invalidConfig.aeadId = 100;
-    invalidConfig.kdfId = 100;
-    invalidConfig.kemId = 100;
-
-    const list = new HpkeConfigList([invalidConfig, validConfig]);
-
     assert.deepEqual(list.selectConfig(), validConfig);
   });
 });
@@ -144,33 +150,60 @@ describe("DAP HpkeConfig", () => {
     }, /id must be an integer in \[0, 255\]/);
   });
 
-  it("cannot be built from an unrecognized kemId", () => {
+  it("cannot be built from an invalid kemId", () => {
+    for (const badKemId of [-1, 65536, 0.5, NaN]) {
+      assert.throws(() => {
+        new HpkeConfig(0, badKemId, 1, 1, Buffer.alloc(10));
+      }, /kemId must be an integer in \[0, 65535\]/);
+    }
+  });
+
+  it("cannot be buit from an invalid kdfId", () => {
+    for (const badKdfId of [-1, 65536, 0.5, NaN]) {
+      assert.throws(() => {
+        new HpkeConfig(0, 1, badKdfId, 1, Buffer.alloc(10));
+      }, /kdfId must be an integer in \[0, 65535\]/);
+    }
+  });
+
+  it("cannot be built from an invalid aeadId", () => {
+    for (const badAeadId of [-1, 65536, 0.5, NaN]) {
+      assert.throws(() => {
+        new HpkeConfig(0, 1, 1, badAeadId, Buffer.alloc(10));
+      }, /aeadId must be an integer in \[0, 65535\]/);
+    }
+  });
+
+  it("cannot be used if it contains an unrecognized kemId", () => {
+    const config = new HpkeConfig(10, 10, 1, 1, Buffer.alloc(10));
     assert.throws(() => {
-      new HpkeConfig(10, 10, 1, 1, Buffer.alloc(10));
+      config.cipherSuite();
     }, /kemId was 10 but must be one of the following:/);
   });
 
-  it("cannot be built from an unrecognized kdfId", () => {
+  it("cannot be used if it contains an unrecognized kdfId", () => {
+    const config = new HpkeConfig(
+      100,
+      KemId.DhkemP256HkdfSha256,
+      50,
+      AeadId.Chacha20Poly1305,
+      Buffer.alloc(10),
+    );
     assert.throws(() => {
-      new HpkeConfig(
-        100,
-        KemId.DhkemP256HkdfSha256,
-        50,
-        AeadId.Chacha20Poly1305,
-        Buffer.alloc(10),
-      );
+      config.cipherSuite();
     }, /kdfId was 50 but must be one of the following:/);
   });
 
-  it("cannot be built from an unrecognized aeadId", () => {
+  it("cannot be used if it contains an unrecognized aeadId", () => {
+    const config = new HpkeConfig(
+      100,
+      KemId.DhkemP256HkdfSha256,
+      KdfId.HkdfSha512,
+      5,
+      Buffer.alloc(10),
+    );
     assert.throws(() => {
-      new HpkeConfig(
-        100,
-        KemId.DhkemP256HkdfSha256,
-        KdfId.HkdfSha512,
-        5,
-        Buffer.alloc(10),
-      );
+      config.cipherSuite();
     }, /aeadId was 5 but must be one of the following:/);
   });
 
diff --git a/packages/dap/src/hpkeConfig.ts b/packages/dap/src/hpkeConfig.ts
index e82a82dba..ae3242f68 100644
--- a/packages/dap/src/hpkeConfig.ts
+++ b/packages/dap/src/hpkeConfig.ts
@@ -43,10 +43,18 @@ export class HpkeConfig implements Encodable {
     if (id !== Math.floor(id) || id < 0 || id > 255) {
       throw new Error("id must be an integer in [0, 255]");
     }
-
-    this.validate(KemId, "kemId");
-    this.validate(KdfId, "kdfId");
-    this.validate(AeadId, "aeadId");
+    if (kemId !== Math.floor(kemId) || kemId < 0 || kemId > 0xffff) {
+      throw new Error("kemId must be an integer in [0, 65535]");
+    }
+    if (kdfId !== Math.floor(kdfId) || kdfId < 0 || kdfId > 0xffff) {
+      throw new Error("kdfId must be an integer in [0, 65535]");
+    }
+    if (aeadId !== Math.floor(aeadId) || aeadId < 0 || aeadId > 0xffff) {
+      throw new Error("aeadId must be an integer in [0, 65535]");
+    }
+    if (publicKey.length > 0xffff) {
+      throw new Error("publicKey must not be longer than 65535 bytes");
+    }
   }
 
   private validate(
@@ -92,6 +100,10 @@ export class HpkeConfig implements Encodable {
 
   /** @internal */
   cipherSuite(): CipherSuite {
+    this.validate(KemId, "kemId");
+    this.validate(KdfId, "kdfId");
+    this.validate(AeadId, "aeadId");
+
     const aead: AeadId = this.aeadId as AeadId;
     const kdf: KdfId = this.kdfId as KdfId;
     const kem: KemId = this.kemId as KemId;