Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing non_exhaustive to enums #1957

Merged
merged 5 commits into from
Sep 19, 2023
Merged

Add missing non_exhaustive to enums #1957

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b37708
Add non_exhaustive to ExtensionType
inahga Sep 18, 2023
da93597
Add non_exhaustive to query type
inahga Sep 18, 2023
ca1d412
Add non_exhaustive to HPKE types
inahga Sep 18, 2023
b7f1a95
Errant unwrap in aggregator API, should propagate error
inahga Sep 18, 2023
224ff08
Missing unwrap
inahga Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions aggregator_api/src/routes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,14 @@ pub(super) async fn post_task<C: Clock>(
_ => unreachable!(),
};

// Unwrap safety: we always use a supported KEM.
let hpke_keys = Vec::from([generate_hpke_config_and_private_key(
random(),
HpkeKemId::X25519HkdfSha256,
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)]);
)
.unwrap()]);

let task = Arc::new(
Task::new(
Expand Down Expand Up @@ -321,7 +323,8 @@ pub(super) async fn put_global_hpke_config<C: Clock>(
req.kem_id.unwrap_or(HpkeKemId::X25519HkdfSha256),
req.kdf_id.unwrap_or(HpkeKdfId::HkdfSha256),
req.aead_id.unwrap_or(HpkeAeadId::Aes128Gcm),
);
)
.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should propagate this error instead of unwrapping it, since the algorithms could be unsupported if the operator makes a mistake.


let inserted_keypair = ds
.run_tx_with_name("put_global_hpke_config", |tx| {
Expand Down
13 changes: 11 additions & 2 deletions aggregator_api/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ async fn post_task_bad_role() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: Some(aggregator_auth_token),
Expand Down Expand Up @@ -246,6 +247,7 @@ async fn post_task_unauthorized() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: Some(aggregator_auth_token),
Expand Down Expand Up @@ -287,6 +289,7 @@ async fn post_task_helper_no_optional_fields() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: None,
Expand Down Expand Up @@ -366,6 +369,7 @@ async fn post_task_helper_with_aggregator_auth_token() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: Some(aggregator_auth_token),
Expand Down Expand Up @@ -408,6 +412,7 @@ async fn post_task_idempotence() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: Some(aggregator_auth_token.clone()),
Expand Down Expand Up @@ -488,6 +493,7 @@ async fn post_task_leader_all_optional_fields() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: Some(aggregator_auth_token.clone()),
Expand Down Expand Up @@ -577,6 +583,7 @@ async fn post_task_leader_no_aggregator_auth_token() {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
.config()
.clone(),
aggregator_auth_token: None,
Expand Down Expand Up @@ -861,7 +868,8 @@ async fn get_global_hpke_configs() {
HpkeKemId::P256HkdfSha256,
HpkeKdfId::HkdfSha384,
HpkeAeadId::Aes128Gcm,
);
)
.unwrap();
ds.run_tx(|tx| {
let keypair1 = keypair1.clone();
let keypair2 = keypair2.clone();
Expand Down Expand Up @@ -962,7 +970,8 @@ async fn get_global_hpke_config() {
HpkeKemId::P256HkdfSha256,
HpkeKdfId::HkdfSha384,
HpkeAeadId::Aes128Gcm,
);
)
.unwrap();
ds.run_tx(|tx| {
let keypair1 = keypair1.clone();
let keypair2 = keypair2.clone();
Expand Down
4 changes: 3 additions & 1 deletion aggregator_core/src/task.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,14 @@ impl SerializedTask {
}

if self.hpke_keys.is_empty() {
// Unwrap safety: we always use a supported KEM.
let hpke_keypair = generate_hpke_config_and_private_key(
random(),
HpkeKemId::X25519HkdfSha256,
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
);
)
.unwrap();

self.hpke_keys = Vec::from([hpke_keypair]);
}
Expand Down
14 changes: 10 additions & 4 deletions core/src/hpke.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ pub enum Error {
Hpke(#[from] HpkeError),
#[error("invalid HPKE configuration: {0}")]
InvalidConfiguration(&'static str),
#[error("unsupported KEM")]
UnsupportedKem,
}

fn hpke_dispatch_config_from_hpke_config(
Expand Down Expand Up @@ -200,21 +202,23 @@ pub fn open(
}

/// Generate a new HPKE keypair and return it as an HpkeConfig (public portion) and
/// HpkePrivateKey (private portion).
/// HpkePrivateKey (private portion). This function errors if the supplied key
/// encapsulated mechanism is not supported by the underlying HPKE library.
pub fn generate_hpke_config_and_private_key(
hpke_config_id: HpkeConfigId,
kem_id: HpkeKemId,
kdf_id: HpkeKdfId,
aead_id: HpkeAeadId,
) -> HpkeKeypair {
) -> Result<HpkeKeypair, Error> {
let Keypair {
private_key,
public_key,
} = match kem_id {
HpkeKemId::X25519HkdfSha256 => Kem::X25519HkdfSha256.gen_keypair(),
HpkeKemId::P256HkdfSha256 => Kem::DhP256HkdfSha256.gen_keypair(),
_ => return Err(Error::UnsupportedKem),
};
HpkeKeypair::new(
Ok(HpkeKeypair::new(
HpkeConfig::new(
hpke_config_id,
kem_id,
Expand All @@ -223,7 +227,7 @@ pub fn generate_hpke_config_and_private_key(
HpkePublicKey::from(public_key),
),
HpkePrivateKey::new(private_key),
)
))
}

/// An HPKE configuration and its corresponding private key.
Expand Down Expand Up @@ -313,6 +317,7 @@ pub mod test_util {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
}

pub fn generate_test_hpke_config_and_private_key_with_id(id: u8) -> HpkeKeypair {
Expand All @@ -322,6 +327,7 @@ pub mod test_util {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
}
}

Expand Down
2 changes: 2 additions & 0 deletions interop_binaries/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ impl HpkeConfigRegistry {
self.keypairs
.entry(id)
.or_insert_with(|| {
// Unwrap safety: we always use a supported KEM.
generate_hpke_config_and_private_key(
id,
// These algorithms should be broadly compatible with other DAP implementations, since they
Expand All @@ -360,6 +361,7 @@ impl HpkeConfigRegistry {
HpkeKdfId::HkdfSha256,
HpkeAeadId::Aes128Gcm,
)
.unwrap()
})
.clone()
}
Expand Down
5 changes: 5 additions & 0 deletions messages/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,7 @@ impl<'de> Deserialize<'de> for TaskId {
/// DAP protocol message representing an HPKE key encapsulation mechanism.
#[derive(Clone, Copy, Debug, PartialEq, Eq, TryFromPrimitive, Serialize, Deserialize)]
#[repr(u16)]
#[non_exhaustive]
pub enum HpkeKemId {
/// NIST P-256 keys and HKDF-SHA256.
P256HkdfSha256 = 0x0010,
Expand Down Expand Up @@ -773,6 +774,7 @@ impl Decode for HpkeKemId {
/// DAP protocol message representing an HPKE key derivation function.
#[derive(Clone, Copy, Debug, PartialEq, Eq, TryFromPrimitive, Serialize, Deserialize)]
#[repr(u16)]
#[non_exhaustive]
pub enum HpkeKdfId {
/// HMAC Key Derivation Function SHA256.
HkdfSha256 = 0x0001,
Expand Down Expand Up @@ -804,6 +806,7 @@ impl Decode for HpkeKdfId {
/// DAP protocol message representing an HPKE AEAD.
#[derive(Clone, Copy, Debug, PartialEq, Eq, TryFromPrimitive, Serialize, Deserialize)]
#[repr(u16)]
#[non_exhaustive]
pub enum HpkeAeadId {
/// AES-128-GCM.
Aes128Gcm = 0x0001,
Expand Down Expand Up @@ -886,6 +889,7 @@ impl Decode for Extension {
/// DAP protocol message representing the type of an extension included in a client report.
#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, TryFromPrimitive)]
#[repr(u16)]
#[non_exhaustive]
pub enum ExtensionType {
Tbd = 0,
}
Expand Down Expand Up @@ -2021,6 +2025,7 @@ pub mod query_type {
/// DAP protocol message representing the type of a query.
#[derive(Copy, Clone, Debug, PartialEq, Eq, TryFromPrimitive, Serialize, Deserialize)]
#[repr(u8)]
#[non_exhaustive]
pub enum Code {
Reserved = 0,
TimeInterval = 1,
Expand Down
2 changes: 1 addition & 1 deletion tools/src/bin/hpke_keygen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn main() -> Result<()> {
options.kem.into(),
options.kdf.into(),
options.aead.into(),
);
)?;

let mut writer = stdout().lock();

Expand Down