Repository: cloudflare/daphne
Pull request #169 (2022-11-17 00:59:21)

taskprov: Move draft-wang-ppm-dap-taskprov-01 to 02
The only difference is in the serialization of QueryConfig: The type byte has been moved to the top of the struct.

While at it, handle unknown taskprov version number gracefully when attempting to compute the task ID.


Patch:
diff --git a/daphne/src/messages/taskprov.rs b/daphne/src/messages/taskprov.rs
index daf8321..b3a1864 100644
--- a/daphne/src/messages/taskprov.rs
+++ b/daphne/src/messages/taskprov.rs
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: BSD-3-Clause
 
 //! Messages in the taskprov extension to the DAP protocol, as
-//! defined in draft-wang-ppm-dap-taskprov-01.
+//! defined in draft-wang-ppm-dap-taskprov-02.
 
 use crate::messages::{
     decode_u16_bytes, encode_u16_bytes, Duration, Time, QUERY_TYPE_FIXED_SIZE,
@@ -217,10 +217,10 @@ impl QueryConfig {
 
 impl ParameterizedEncode<TaskprovVersion> for QueryConfig {
     fn encode_with_param(&self, _encoding_parameter: &TaskprovVersion, bytes: &mut Vec<u8>) {
+        self.encode_query_type(bytes);
         self.time_precision.encode(bytes);
         self.max_batch_query_count.encode(bytes);
         self.min_batch_size.encode(bytes);
-        self.encode_query_type(bytes);
         match &self.var {
             QueryConfigVar::TimeInterval => (),
             QueryConfigVar::FixedSize { max_batch_size } => {
@@ -235,10 +235,10 @@ impl ParameterizedDecode<TaskprovVersion> for QueryConfig {
         _decoding_parameter: &TaskprovVersion,
         bytes: &mut Cursor<&[u8]>,
     ) -> Result<Self, CodecError> {
+        let query_type = u8::decode(bytes)?;
         let time_precision = Duration::decode(bytes)?;
         let max_batch_query_count = u16::decode(bytes)?;
         let min_batch_size = u32::decode(bytes)?;
-        let query_type = u8::decode(bytes)?;
         let var = match query_type {
             QUERY_TYPE_TIME_INTERVAL => Ok(QueryConfigVar::TimeInterval),
             QUERY_TYPE_FIXED_SIZE => Ok(QueryConfigVar::FixedSize {
diff --git a/daphne/src/roles.rs b/daphne/src/roles.rs
index 27068b7..4856571 100644
--- a/daphne/src/roles.rs
+++ b/daphne/src/roles.rs
@@ -610,7 +610,7 @@ where
                 let mut first_metadata: Option<&ReportMetadata> = None;
 
                 // If taskprov is allowed, ensure that either all of the shares have it or none of them
-                // do (section 6 of draft-wang-ppm-dap-taskprov-01).
+                // do (section 6 of draft-wang-ppm-dap-taskprov-02).
                 let global_config = self.get_global_config();
                 if global_config.allow_taskprov {
                     let task_id = req.task_id()?;
diff --git a/daphne/src/taskprov.rs b/daphne/src/taskprov.rs
index 2fb0b23..743863c 100644
--- a/daphne/src/taskprov.rs
+++ b/daphne/src/taskprov.rs
@@ -21,8 +21,8 @@ use url::Url;
 /// DAP taskprov version.
 #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)]
 pub enum TaskprovVersion {
-    #[serde(rename = "v01")]
-    Draft01,
+    #[serde(rename = "v02")]
+    Draft02,
 
     #[serde(other)]
     Unknown,
@@ -30,12 +30,12 @@ pub enum TaskprovVersion {
 
 /// SHA-256 of "dap-taskprov"
 #[allow(dead_code)]
-pub(crate) const TASK_PROV_SALT_DRAFT01: [u8; 32] = [
+pub(crate) const TASK_PROV_SALT_DRAFT02: [u8; 32] = [
     0x28, 0xb9, 0xbb, 0x4f, 0x62, 0x4f, 0x67, 0x9a, 0xc1, 0x98, 0xd9, 0x68, 0xf4, 0xb0, 0x9e, 0xec,
     0x74, 0x01, 0x7a, 0x52, 0xcb, 0x4c, 0xf6, 0x39, 0xfb, 0x83, 0xe0, 0x47, 0x72, 0x3a, 0x0f, 0xfe,
 ];
 
-fn compute_task_id_draft01(serialized: &[u8]) -> Id {
+fn compute_task_id_draft02(serialized: &[u8]) -> Id {
     let d = digest::digest(&digest::SHA256, serialized);
     let dref = d.as_ref();
     let mut b: [u8; 32] = [0; 32];
@@ -44,10 +44,12 @@ fn compute_task_id_draft01(serialized: &[u8]) -> Id {
 }
 
 /// Compute the task id of a serialized task config.
-pub fn compute_task_id(version: TaskprovVersion, serialized: &[u8]) -> Id {
+pub fn compute_task_id(version: TaskprovVersion, serialized: &[u8]) -> Result<Id, DapError> {
     match version {
-        TaskprovVersion::Draft01 => compute_task_id_draft01(serialized),
-        _ => panic!("unimplemented taskprov version"),
+        TaskprovVersion::Draft02 => Ok(compute_task_id_draft02(serialized)),
+        TaskprovVersion::Unknown => Err(DapError::fatal(
+            "attempted to resolve taskprov task with unknown version",
+        )),
     }
 }
 
@@ -63,7 +65,7 @@ pub(crate) fn extract_prk_from_verify_key_init(
     // The documentation says computing the Salt is expensive, and we use the same PRK all the
     // time, so we compute it once.
     let value = match version {
-        TaskprovVersion::Draft01 => &TASK_PROV_SALT_DRAFT01,
+        TaskprovVersion::Draft02 => &TASK_PROV_SALT_DRAFT02,
         _ => panic!("unimplemented taskprov version"),
     };
     Salt::new(HKDF_SHA256, value).extract(verify_key_init)
@@ -129,7 +131,7 @@ pub fn get_taskprov_task_config(
         0 => Ok(None),
         1 => match &taskprovs[0] {
             Extension::Taskprov { payload } => {
-                if compute_task_id(version, &payload[..]) != *task_id {
+                if compute_task_id(version, &payload[..])? != *task_id {
                     // Return unrecognizedTask following section 5.1 of the taskprov draft.
                     return Err(DapError::Abort(DapAbort::UnrecognizedTask));
                 }
@@ -214,8 +216,15 @@ impl DapTaskConfig {
 impl ReportMetadata {
     /// Does this metatdata have a taskprov extension and does it match the specified id?
     pub fn is_taskprov(&self, version: TaskprovVersion, task_id: &Id) -> bool {
+        // Don't check for taskprov usage if we don't know the version.
+        if matches!(version, TaskprovVersion::Unknown) {
+            return false;
+        }
+
         return self.extensions.iter().any(|x| match x {
-            Extension::Taskprov { payload } => *task_id == compute_task_id(version, payload),
+            Extension::Taskprov { payload } => {
+                *task_id == compute_task_id(version, payload).unwrap()
+            }
             _ => false,
         });
     }
diff --git a/daphne_worker/src/config.rs b/daphne_worker/src/config.rs
index 623ce91..752eba4 100644
--- a/daphne_worker/src/config.rs
+++ b/daphne_worker/src/config.rs
@@ -43,7 +43,7 @@ pub(crate) const KV_KEY_PREFIX_BEARER_TOKEN_COLLECTOR: &str = "bearer_token/coll
 pub(crate) const KV_KEY_PREFIX_TASK_CONFIG: &str = "config/task";
 pub(crate) const KV_BINDING_DAP_CONFIG: &str = "DAP_CONFIG";
 
-/// Long-lived parameters for tasks using draft-wang-ppm-dap-taskprov-01 ("taskprov").
+/// Long-lived parameters for tasks using draft-wang-ppm-dap-taskprov-02 ("taskprov").
 pub(crate) struct TaskprovConfig {
     /// HPKE collector configuration for all taskprov tasks.
     pub(crate) hpke_collector_config: HpkeConfig,
@@ -100,7 +100,7 @@ pub(crate) struct DaphneWorkerConfig<D> {
     /// Indicates if DaphneWorker is used as the Leader.
     is_leader: bool,
 
-    /// draft-wang-ppm-dap-taskprov-01 configuration (optional).
+    /// draft-wang-ppm-dap-taskprov-02 configuration (optional).
     pub(crate) taskprov_config: Option<TaskprovConfig>,
 }
 


Test patch:
diff --git a/daphne/src/messages/mod_test.rs b/daphne/src/messages/mod_test.rs
index e348a1f..fbd5229 100644
--- a/daphne/src/messages/mod_test.rs
+++ b/daphne/src/messages/mod_test.rs
@@ -192,18 +192,18 @@ fn read_vdaf_config() {
 }
 
 #[test]
-fn read_task_config_taskprov_draft01() {
+fn read_task_config_taskprov_draft02() {
     let data = [
         0x02, 0x48, 0x69, 0x00, 0x0e, 0x00, 0x0c, 0x68, 0x74, 0x74, 0x70, 0x73, 0x3a, 0x2f, 0x2f,
-        0x74, 0x65, 0x73, 0x74, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x80, 0x00,
-        0x00, 0x04, 0x00, 0x02, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x63, 0x52, 0xf9,
+        0x74, 0x65, 0x73, 0x74, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x80,
+        0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x63, 0x52, 0xf9,
         0xa5, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x18, 0x01, 0x02, 0x03, 0x04, 0x04, 0x03,
         0x02, 0x01, 0x02, 0x02, 0x03, 0x04, 0x04, 0x03, 0x02, 0x02, 0x03, 0x02, 0x03, 0x04, 0x04,
         0x03, 0x02, 0x03,
     ];
 
     let buckets = vec![0x0102030404030201, 0x0202030404030202, 0x0302030404030203];
-    let task_config = TaskConfig::get_decoded_with_param(&TaskprovVersion::Draft01, &data).unwrap();
+    let task_config = TaskConfig::get_decoded_with_param(&TaskprovVersion::Draft02, &data).unwrap();
     assert_eq!(
         task_config,
         TaskConfig {
@@ -229,15 +229,16 @@ fn read_task_config_taskprov_draft01() {
 
     assert_eq!(
         compute_task_id(
-            TaskprovVersion::Draft01,
-            &task_config.get_encoded_with_param(&TaskprovVersion::Draft01)
+            TaskprovVersion::Draft02,
+            &task_config.get_encoded_with_param(&TaskprovVersion::Draft02)
         )
+        .unwrap()
         .to_hex(),
-        "2e5eb17cb12faaf122fca963b8e4cca3f11737f31fc83a02cf9d72019d2b2081"
+        "2b585fcbb48c21fb5f05221a241fdd8cb9ebe99bd183d66326fcecd85fe06fd5",
     );
 
     assert_eq!(
-        task_config.get_encoded_with_param(&TaskprovVersion::Draft01),
+        task_config.get_encoded_with_param(&TaskprovVersion::Draft02),
         &data
     );
 }
diff --git a/daphne/src/roles_test.rs b/daphne/src/roles_test.rs
index 3e2ae68..fdde569 100644
--- a/daphne/src/roles_test.rs
+++ b/daphne/src/roles_test.rs
@@ -72,7 +72,7 @@ impl Test {
             max_batch_interval_end: 259200,
             supported_hpke_kems: vec![HpkeKemId::X25519HkdfSha256],
             allow_taskprov: true,
-            taskprov_version: TaskprovVersion::Draft01,
+            taskprov_version: TaskprovVersion::Draft02,
         };
 
         // Task Parameters that the Leader and Helper must agree on.
diff --git a/daphne/src/taskprov_test.rs b/daphne/src/taskprov_test.rs
index eda7891..e715acd 100644
--- a/daphne/src/taskprov_test.rs
+++ b/daphne/src/taskprov_test.rs
@@ -21,7 +21,7 @@ fn check_vdaf_key_computation() {
         0x76, 0x1d,
     ];
     let vk = compute_vdaf_verify_key(
-        TaskprovVersion::Draft01,
+        TaskprovVersion::Draft02,
         &verify_key_init,
         &task_id,
         VdafType::Prio3Aes128Count,
diff --git a/daphne_worker_test/tests/e2e.rs b/daphne_worker_test/tests/e2e.rs
index 7e3e0b4..925192e 100644
--- a/daphne_worker_test/tests/e2e.rs
+++ b/daphne_worker_test/tests/e2e.rs
@@ -309,8 +309,8 @@ async fn e2e_leader_upload() {
             var: VdafTypeVar::Prio3Aes128Count,
         },
     };
-    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft01);
-    let task_id = compute_task_id(TaskprovVersion::Draft01, &payload);
+    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft02);
+    let task_id = compute_task_id(TaskprovVersion::Draft02, &payload).unwrap();
     let extensions = vec![Extension::Taskprov { payload }];
     let report = t
         .task_config
@@ -332,10 +332,10 @@ async fn e2e_leader_upload() {
     .await;
 
     // Generate and upload a report with taskprov but with the wrong id
-    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft01);
+    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft02);
     let mut bad_payload = payload.clone();
     bad_payload[0] = u8::wrapping_add(bad_payload[0], 1);
-    let task_id = compute_task_id(TaskprovVersion::Draft01, &bad_payload);
+    let task_id = compute_task_id(TaskprovVersion::Draft02, &bad_payload).unwrap();
     let extensions = vec![Extension::Taskprov { payload }];
     let report = t
         .task_config
@@ -360,8 +360,8 @@ async fn e2e_leader_upload() {
     .await;
 
     // Generate and upload a report with two copies of the taskprov extension
-    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft01);
-    let task_id = compute_task_id(TaskprovVersion::Draft01, &payload);
+    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft02);
+    let task_id = compute_task_id(TaskprovVersion::Draft02, &payload).unwrap();
     let extensions = vec![
         Extension::Taskprov {
             payload: payload.clone(),
@@ -413,8 +413,8 @@ async fn e2e_leader_upload() {
             var: VdafTypeVar::Prio3Aes128Count,
         },
     };
-    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft01);
-    let task_id = compute_task_id(TaskprovVersion::Draft01, &payload);
+    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft02);
+    let task_id = compute_task_id(TaskprovVersion::Draft02, &payload).unwrap();
     let extensions = vec![Extension::Taskprov { payload }];
     let report = t
         .task_config
@@ -1134,11 +1134,11 @@ async fn e2e_leader_collect_taskprov_ok() {
             var: VdafTypeVar::Prio3Aes128Sum { bit_length: 10 },
         },
     };
-    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft01);
-    let task_id = compute_task_id(TaskprovVersion::Draft01, &payload);
+    let payload = taskprov_task_config.get_encoded_with_param(&TaskprovVersion::Draft02);
+    let task_id = compute_task_id(TaskprovVersion::Draft02, &payload).unwrap();
     let task_config = DapTaskConfig::try_from_taskprov(
         DapVersion::Draft02,
-        TaskprovVersion::Draft01,
+        TaskprovVersion::Draft02,
         &task_id.clone(),
         taskprov_task_config.clone(),
         &t.taskprov_vdaf_verify_key_init,
diff --git a/daphne_worker_test/tests/test_runner.rs b/daphne_worker_test/tests/test_runner.rs
index 86962f3..03286c1 100644
--- a/daphne_worker_test/tests/test_runner.rs
+++ b/daphne_worker_test/tests/test_runner.rs
@@ -112,7 +112,7 @@ impl TestRunner {
             max_batch_interval_end: 259200,
             supported_hpke_kems: vec![HpkeKemId::X25519HkdfSha256],
             allow_taskprov: true,
-            taskprov_version: TaskprovVersion::Draft01,
+            taskprov_version: TaskprovVersion::Draft02,
         };
         let taskprov_vdaf_verify_key_init =
             hex::decode("0074a5dd6e9dac501f73f7a961193b2b").unwrap();
diff --git a/daphne_worker_test/wrangler.toml b/daphne_worker_test/wrangler.toml
index c1bbb25..73554c8 100644
--- a/daphne_worker_test/wrangler.toml
+++ b/daphne_worker_test/wrangler.toml
@@ -33,7 +33,7 @@ DAP_GLOBAL_CONFIG = """{
      "max_batch_interval_end": 259200,
      "supported_hpke_kems": ["x25519_hkdf_sha256"],
      "allow_taskprov": true,
-     "taskprov_version": "v01"
+     "taskprov_version": "v02"
 }"""
 DAP_TASKPROV_HPKE_COLLECTOR_CONFIG = """{
   "id": 23,
@@ -89,7 +89,7 @@ DAP_GLOBAL_CONFIG = """{
   "max_batch_interval_end": 259200,
   "supported_hpke_kems": ["x25519_hkdf_sha256"],
   "allow_taskprov": true,
-  "taskprov_version": "v01"
+  "taskprov_version": "v02"
 }"""
 DAP_TASKPROV_HPKE_COLLECTOR_CONFIG = """{
   "id": 23,