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.

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

New API for the headers function #51

Merged
merged 3 commits into from
Mar 18, 2021
Merged
Changes from all commits
Commits
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
35 changes: 20 additions & 15 deletions src/aesgcm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::{
crypto::{self, LocalKeyPair, RemotePublicKey},
error::*,
};
use std::collections::HashMap;

const ECE_AESGCM_PAD_SIZE: usize = 2;

Expand Down Expand Up @@ -55,35 +54,41 @@ impl AesGcmEncryptedBlock {
})
}

/// Return the headers Hash, NOTE you may need to merge Crypto-Key if there's
/// already a VAPID element present.
pub fn headers(self) -> HashMap<String, String> {
let mut result: HashMap<String, String> = HashMap::new();
/// Return the headers Hash.
/// If you're using VAPID, provide the `p256ecdsa` public key that signed the Json Web Token
/// so it can be included in the `Crypto-Key` field.
///
/// Disclaimer : You will need to manually add the Authorization field for VAPID containing the JSON Web Token
pub fn headers(&self, vapid_public_key: Option<&[u8]>) -> Vec<(&'static str, String)> {
let mut result = Vec::new();
let mut rs = "".to_owned();
result.insert(
"Crypto-Key".to_owned(),
format!(
"dh={}",
base64::encode_config(&self.dh, base64::URL_SAFE_NO_PAD)
let dh = base64::encode_config(&self.dh, base64::URL_SAFE_NO_PAD);
let crypto_key = match vapid_public_key {
Copy link
Contributor

Choose a reason for hiding this comment

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

The use of an optional vapid key here makes sense to me. I'm tempted to suggest a more open-ended approach here, e.g. allowing the caller to specify arbitrary key-value pairs that get put into the header..but that's probably going to make more opportunities for things to go wrong, so special-casing this p256ecdsa field seems fine 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea with this one is that the Crypto-Key field needs to be merged since there are several sub key-value pairs in its value (dh and p256ecdsa), one for Vapid and the other one being specific to the AESGCM scheme (this odd mix has been streamlined in AES128GCM, where the headers part is far easier to pull off). So allowing users to provide the entire key-pair would require them to effectively merge this Crypto-Key header themselves using the keys from their Vapid Signature and from the returned AESGCMEncryptedBlock, which I'm trying to avoid here.
Apart from this specific case, the function returns an owned Vec so it's pretty easy to push new key-value pairs in it if necessary, outside of the function itself.

Some(public_key) => format!(
"dh={}; p256ecdsa={}",
dh,
base64::encode_config(public_key, base64::URL_SAFE_NO_PAD)
),
);
None => format!("dh={}", dh),
};
result.push(("Crypto-Key", crypto_key));
if self.rs > 0 {
rs = format!(";rs={}", self.rs);
}
result.insert(
"Encryption".to_owned(),
result.push((
"Encryption",
format!(
"salt={}{}",
base64::encode_config(&self.salt, base64::URL_SAFE_NO_PAD),
rs
),
);
));
result
}

/// Encode the body as a String.
/// If you need the bytes, probably just call .ciphertext directly
pub fn body(self) -> String {
pub fn body(&self) -> String {
base64::encode_config(&self.ciphertext, base64::URL_SAFE_NO_PAD)
}
}
Expand Down