Skip to content

Commit 8f8e5b9

Browse files
jhoylajhoyla
committed
Address comments
1 parent a056cd3 commit 8f8e5b9

File tree

13 files changed

+457
-118
lines changed

13 files changed

+457
-118
lines changed

‎crates/dapf/src/acceptance/load_testing.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ pub async fn execute_single_combination_from_env(
448448
&measurment,
449449
VERSION,
450450
system_now.0,
451+
Some(vec![]),
451452
vec![messages::Extension::Taskprov],
452453
t.replay_reports,
453454
)

‎crates/dapf/src/acceptance/mod.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,10 @@ impl Test {
655655
measurement.as_ref(),
656656
version,
657657
now.0,
658+
match version {
659+
DapVersion::Draft09 => None,
660+
DapVersion::Latest => Some(vec![]),
661+
},
658662
vec![messages::Extension::Taskprov],
659663
self.replay_reports,
660664
)

‎crates/daphne-server/tests/e2e/e2e.rs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,10 @@ async fn leader_upload_taskprov() {
428428
t.now,
429429
&task_id,
430430
DapMeasurement::U32Vec(vec![1; 10]),
431+
match version {
432+
DapVersion::Draft09 => None,
433+
DapVersion::Latest => Some(vec![]),
434+
},
431435
vec![Extension::Taskprov],
432436
version,
433437
)
@@ -455,6 +459,10 @@ async fn leader_upload_taskprov() {
455459
t.now,
456460
&task_id,
457461
DapMeasurement::U32Vec(vec![1; 10]),
462+
match version {
463+
DapVersion::Draft09 => None,
464+
DapVersion::Latest => Some(vec![]),
465+
},
458466
vec![Extension::Taskprov],
459467
version,
460468
)
@@ -520,6 +528,10 @@ async fn leader_upload_taskprov_wrong_version(version: DapVersion) {
520528
t.now,
521529
&task_id,
522530
DapMeasurement::U32Vec(vec![1; 10]),
531+
match version {
532+
DapVersion::Draft09 => None,
533+
DapVersion::Latest => Some(vec![]),
534+
},
523535
vec![Extension::Taskprov],
524536
version,
525537
)
@@ -1548,18 +1560,18 @@ async fn leader_collect_taskprov_repeated_abort() {
15481560
.unwrap(),
15491561
),
15501562
{
1551-
let mut report = task_config
1563+
let report = task_config
15521564
.vdaf
15531565
.produce_report_with_extensions(
15541566
&hpke_config_list,
15551567
now,
15561568
&task_id,
15571569
DapMeasurement::U32Vec(vec![1; 10]),
1570+
Some(vec![Extension::Taskprov]),
15581571
extensions,
15591572
version,
15601573
)
15611574
.unwrap();
1562-
report.report_metadata.public_extensions = Some(vec![Extension::Taskprov]);
15631575
report.get_encoded_with_param(&version).unwrap()
15641576
},
15651577
)
@@ -1666,6 +1678,10 @@ async fn leader_collect_taskprov_ok(version: DapVersion) {
16661678
now,
16671679
&task_id,
16681680
DapMeasurement::U32Vec(vec![1; 10]),
1681+
match version {
1682+
DapVersion::Draft09 => None,
1683+
DapVersion::Latest => Some(vec![]),
1684+
},
16691685
extensions,
16701686
version,
16711687
)

‎crates/daphne/src/error/aborts.rs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -268,14 +268,10 @@ impl DapAbort {
268268
task_id: &TaskId,
269269
unknown_extensions: &[u16],
270270
) -> Result<Self, DapError> {
271-
let detail = serde_json::to_string(&unknown_extensions);
272-
match detail {
273-
Ok(s) => Ok(Self::UnsupportedExtension {
274-
detail: s,
275-
task_id: *task_id,
276-
}),
277-
Err(x) => Err(fatal_error!(err = %x,)),
278-
}
271+
Ok(Self::UnsupportedExtension {
272+
detail: format!("{unknown_extensions:?}"),
273+
task_id: *task_id,
274+
})
279275
}
280276

281277
fn title_and_type(&self) -> (&'static str, Option<String>) {

‎crates/daphne/src/messages/mod.rs‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,7 +1419,7 @@ impl Decode for HpkeCiphertext {
14191419
/// A plaintext input share.
14201420
#[derive(Clone, Debug, PartialEq, Eq)]
14211421
pub struct PlaintextInputShare {
1422-
pub extensions: Vec<Extension>,
1422+
pub private_extensions: Vec<Extension>,
14231423
pub payload: Vec<u8>,
14241424
}
14251425

@@ -1429,7 +1429,7 @@ impl ParameterizedEncode<DapVersion> for PlaintextInputShare {
14291429
version: &DapVersion,
14301430
bytes: &mut Vec<u8>,
14311431
) -> Result<(), CodecError> {
1432-
encode_u16_items(bytes, version, &self.extensions)?;
1432+
encode_u16_items(bytes, version, &self.private_extensions)?;
14331433
encode_u32_bytes(bytes, &self.payload)?;
14341434
Ok(())
14351435
}
@@ -1441,7 +1441,7 @@ impl ParameterizedDecode<DapVersion> for PlaintextInputShare {
14411441
bytes: &mut Cursor<&[u8]>,
14421442
) -> Result<Self, CodecError> {
14431443
Ok(Self {
1444-
extensions: decode_u16_items(version, bytes)?,
1444+
private_extensions: decode_u16_items(version, bytes)?,
14451445
payload: decode_u32_bytes(bytes)?,
14461446
})
14471447
}
@@ -1649,6 +1649,24 @@ mod test {
16491649

16501650
test_versions! {report_metadata_encode_decode}
16511651

1652+
#[test]
1653+
fn report_metadata_encode_latest_decode_draft09() {
1654+
let ext_rm = ReportMetadata {
1655+
id: ReportId([15; 16]),
1656+
time: 123_456,
1657+
public_extensions: Some(vec![Extension::NotImplemented {
1658+
typ: 0x10,
1659+
payload: vec![0x11, 0x12],
1660+
}]),
1661+
};
1662+
let bytes = ext_rm.get_encoded_with_param(&DapVersion::Latest).unwrap();
1663+
assert!(matches!(
1664+
ReportMetadata::get_decoded_with_param(&DapVersion::Draft09, bytes.as_slice())
1665+
.unwrap_err(),
1666+
CodecError::BytesLeftOver(..)
1667+
));
1668+
}
1669+
16521670
fn partial_batch_selector_encode_decode(version: DapVersion) {
16531671
const TEST_DATA_DRAFT09: &[u8] = &[1];
16541672
const TEST_DATA_LATEST: &[u8] = &[1, 0, 0];

‎crates/daphne/src/protocol/aggregator.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: BSD-3-Clause
33

44
use super::{
5-
no_duplicates,
5+
check_no_duplicates,
66
report_init::{InitializedReport, WithPeerPrepShare},
77
};
88
use crate::{
@@ -241,7 +241,7 @@ impl DapTaskConfig {
241241
DapAggregationParam::get_decoded_with_param(&self.vdaf, &agg_job_init_req.agg_param)
242242
.map_err(|e| DapAbort::from_codec_error(e, *task_id))?;
243243
if replay_protection.enabled() {
244-
no_duplicates(
244+
check_no_duplicates(
245245
agg_job_init_req
246246
.prep_inits
247247
.iter()

‎crates/daphne/src/protocol/client.rs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@ impl VdafConfig {
3030
/// * `extensions` are the extensions.
3131
///
3232
/// * `version` is the `DapVersion` to use.
33+
#[allow(clippy::too_many_arguments)]
3334
pub fn produce_report_with_extensions(
3435
&self,
3536
hpke_config_list: &[HpkeConfig; 2],
3637
time: Time,
3738
task_id: &TaskId,
3839
measurement: DapMeasurement,
39-
extensions: Vec<Extension>,
40+
public_extensions: Option<Vec<Extension>>,
41+
private_extensions: Vec<Extension>,
4042
version: DapVersion,
4143
) -> Result<Report, DapError> {
4244
let mut rng = thread_rng();
@@ -51,7 +53,8 @@ impl VdafConfig {
5153
time,
5254
task_id,
5355
&report_id,
54-
extensions,
56+
public_extensions,
57+
private_extensions,
5558
version,
5659
)
5760
}
@@ -65,21 +68,28 @@ impl VdafConfig {
6568
time: Time,
6669
task_id: &TaskId,
6770
report_id: &ReportId,
68-
extensions: Vec<Extension>,
71+
public_extensions: Option<Vec<Extension>>,
72+
private_extensions: Vec<Extension>,
6973
version: DapVersion,
7074
) -> Result<Report, DapError> {
75+
match (&public_extensions, version) {
76+
(Some(_), DapVersion::Draft09) | (None, DapVersion::Latest) => {
77+
return Err(DapError::ReportError(
78+
crate::messages::ReportError::InvalidMessage,
79+
))
80+
}
81+
_ => (),
82+
}
83+
7184
let mut plaintext_input_share = PlaintextInputShare {
72-
extensions,
85+
private_extensions,
7386
payload: Vec::default(),
7487
};
7588

7689
let metadata = ReportMetadata {
7790
id: *report_id,
7891
time,
79-
public_extensions: match version {
80-
DapVersion::Draft09 => None,
81-
DapVersion::Latest => Some(Vec::new()),
82-
},
92+
public_extensions,
8393
};
8494

8595
let encoded_input_shares = input_shares.into_iter().map(|input_share| {
@@ -147,6 +157,10 @@ impl VdafConfig {
147157
time,
148158
task_id,
149159
measurement,
160+
match version {
161+
DapVersion::Draft09 => None,
162+
DapVersion::Latest => Some(Vec::new()),
163+
},
150164
Vec::new(),
151165
version,
152166
)

‎crates/daphne/src/protocol/mod.rs‎

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub(crate) mod report_init;
1111

1212
/// checks if an iterator has no duplicate items, returns the ok if there are no dups or an error
1313
/// with the first offending item.
14-
pub(crate) fn no_duplicates<I>(iterator: I) -> Result<(), I::Item>
14+
pub(crate) fn check_no_duplicates<I>(iterator: I) -> Result<(), I::Item>
1515
where
1616
I: Iterator,
1717
I::Item: Eq + std::hash::Hash,
@@ -752,6 +752,10 @@ mod test {
752752
t.now,
753753
&t.task_id,
754754
DapMeasurement::U32Vec(vec![1; 10]),
755+
match version {
756+
DapVersion::Draft09 => None,
757+
DapVersion::Latest => Some(vec![]),
758+
},
755759
vec![Extension::NotImplemented {
756760
typ: 0xffff,
757761
payload: b"some extension data".to_vec(),
@@ -784,29 +788,32 @@ mod test {
784788

785789
test_versions! { handle_unrecognized_report_extensions }
786790

787-
fn handle_unknown_public_extensions_in_report(version: DapVersion) {
791+
#[test]
792+
fn handle_unknown_public_extensions_in_report() {
793+
let version = DapVersion::Latest;
788794
let t = AggregationJobTest::new(TEST_VDAF, HpkeKemId::X25519HkdfSha256, version);
789-
let mut report = t
795+
let report = t
790796
.task_config
791797
.vdaf
792-
.produce_report(
798+
.produce_report_with_extensions(
793799
&t.client_hpke_config_list,
794800
t.now,
795801
&t.task_id,
796802
DapMeasurement::U32Vec(vec![1; 10]),
803+
Some(vec![
804+
Extension::NotImplemented {
805+
typ: 0x01,
806+
payload: b"This is ignored".to_vec(),
807+
},
808+
Extension::NotImplemented {
809+
typ: 0x02,
810+
payload: b"This is ignored too".to_vec(),
811+
},
812+
]),
813+
vec![],
797814
version,
798815
)
799816
.unwrap();
800-
report.report_metadata.public_extensions = Some(vec![
801-
Extension::NotImplemented {
802-
typ: 0x01,
803-
payload: b"This is ignored".to_vec(),
804-
},
805-
Extension::NotImplemented {
806-
typ: 0x02,
807-
payload: b"This is ignored too".to_vec(),
808-
},
809-
]);
810817
let report_metadata = report.report_metadata.clone();
811818
let [leader_share, _] = report.encrypted_input_shares;
812819
let initialized_report = InitializedReport::from_client(
@@ -832,7 +839,6 @@ mod test {
832839
}
833840
);
834841
}
835-
test_versions! {handle_unknown_public_extensions_in_report}
836842

837843
fn handle_repeated_report_extensions(version: DapVersion) {
838844
let t = AggregationJobTest::new(TEST_VDAF, HpkeKemId::X25519HkdfSha256, version);
@@ -844,6 +850,10 @@ mod test {
844850
t.now,
845851
&t.task_id,
846852
DapMeasurement::U32Vec(vec![1; 10]),
853+
match version {
854+
DapVersion::Draft09 => None,
855+
DapVersion::Latest => Some(vec![]),
856+
},
847857
vec![
848858
Extension::NotImplemented {
849859
typ: 23,
@@ -906,6 +916,10 @@ mod test {
906916
self.now,
907917
&self.task_id,
908918
&report_id,
919+
match version {
920+
DapVersion::Draft09 => None,
921+
DapVersion::Latest => Some(vec![]),
922+
},
909923
Vec::new(), // extensions
910924
version,
911925
)
@@ -937,6 +951,10 @@ mod test {
937951
self.now,
938952
&self.task_id,
939953
&report_id,
954+
match version {
955+
DapVersion::Draft09 => None,
956+
DapVersion::Latest => Some(vec![]),
957+
},
940958
Vec::new(), // extensions
941959
version,
942960
)
@@ -969,6 +987,10 @@ mod test {
969987
self.now,
970988
&self.task_id,
971989
&report_id,
990+
match version {
991+
DapVersion::Draft09 => None,
992+
DapVersion::Latest => Some(vec![]),
993+
},
972994
Vec::new(), // extensions
973995
version,
974996
)

0 commit comments

Comments
 (0)