Skip to content

Commit 28ff330

Browse files
jhoylajhoyla
committed
Address comments
1 parent 5078b99 commit 28ff330

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
@@ -420,6 +420,10 @@ async fn leader_upload_taskprov() {
420420
t.now,
421421
&task_id,
422422
DapMeasurement::U32Vec(vec![1; 10]),
423+
match version {
424+
DapVersion::Draft09 => None,
425+
DapVersion::Latest => Some(vec![]),
426+
},
423427
vec![Extension::Taskprov],
424428
version,
425429
)
@@ -447,6 +451,10 @@ async fn leader_upload_taskprov() {
447451
t.now,
448452
&task_id,
449453
DapMeasurement::U32Vec(vec![1; 10]),
454+
match version {
455+
DapVersion::Draft09 => None,
456+
DapVersion::Latest => Some(vec![]),
457+
},
450458
vec![Extension::Taskprov],
451459
version,
452460
)
@@ -512,6 +520,10 @@ async fn leader_upload_taskprov_wrong_version(version: DapVersion) {
512520
t.now,
513521
&task_id,
514522
DapMeasurement::U32Vec(vec![1; 10]),
523+
match version {
524+
DapVersion::Draft09 => None,
525+
DapVersion::Latest => Some(vec![]),
526+
},
515527
vec![Extension::Taskprov],
516528
version,
517529
)
@@ -1542,18 +1554,18 @@ async fn leader_collect_taskprov_repeated_abort() {
15421554
.unwrap(),
15431555
),
15441556
{
1545-
let mut report = task_config
1557+
let report = task_config
15461558
.vdaf
15471559
.produce_report_with_extensions(
15481560
&hpke_config_list,
15491561
now,
15501562
&task_id,
15511563
DapMeasurement::U32Vec(vec![1; 10]),
1564+
Some(vec![Extension::Taskprov]),
15521565
extensions,
15531566
version,
15541567
)
15551568
.unwrap();
1556-
report.report_metadata.public_extensions = Some(vec![Extension::Taskprov]);
15571569
report.get_encoded_with_param(&version).unwrap()
15581570
},
15591571
)
@@ -1660,6 +1672,10 @@ async fn leader_collect_taskprov_ok(version: DapVersion) {
16601672
now,
16611673
&task_id,
16621674
DapMeasurement::U32Vec(vec![1; 10]),
1675+
match version {
1676+
DapVersion::Draft09 => None,
1677+
DapVersion::Latest => Some(vec![]),
1678+
},
16631679
extensions,
16641680
version,
16651681
)

‎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
@@ -1418,7 +1418,7 @@ impl Decode for HpkeCiphertext {
14181418
/// A plaintext input share.
14191419
#[derive(Clone, Debug, PartialEq, Eq)]
14201420
pub struct PlaintextInputShare {
1421-
pub extensions: Vec<Extension>,
1421+
pub private_extensions: Vec<Extension>,
14221422
pub payload: Vec<u8>,
14231423
}
14241424

@@ -1428,7 +1428,7 @@ impl ParameterizedEncode<DapVersion> for PlaintextInputShare {
14281428
version: &DapVersion,
14291429
bytes: &mut Vec<u8>,
14301430
) -> Result<(), CodecError> {
1431-
encode_u16_items(bytes, version, &self.extensions)?;
1431+
encode_u16_items(bytes, version, &self.private_extensions)?;
14321432
encode_u32_bytes(bytes, &self.payload)?;
14331433
Ok(())
14341434
}
@@ -1440,7 +1440,7 @@ impl ParameterizedDecode<DapVersion> for PlaintextInputShare {
14401440
bytes: &mut Cursor<&[u8]>,
14411441
) -> Result<Self, CodecError> {
14421442
Ok(Self {
1443-
extensions: decode_u16_items(version, bytes)?,
1443+
private_extensions: decode_u16_items(version, bytes)?,
14441444
payload: decode_u32_bytes(bytes)?,
14451445
})
14461446
}
@@ -1648,6 +1648,24 @@ mod test {
16481648

16491649
test_versions! {report_metadata_encode_decode}
16501650

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