diff options
author | Neal H. Walfield <neal@pep.foundation> | 2019-01-29 10:22:46 +0100 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2019-01-29 10:22:46 +0100 |
commit | bff390b189ac92012a3c25e8c361013c78698fd3 (patch) | |
tree | 4460d76b3d396c48d7974ba49cad2ed08f08abf9 | |
parent | ee276a2dc55fd55f22494e9e70187f060fb2f151 (diff) |
openpgp: TPKBuilder::autocrypt should not default to an empty UID
- TPKBuilder::autocrypt created a TPK with a single User ID, as
required by the Autocrypt specification. Since no User ID was
passed, it used the empty string.
- An empty User ID is a bit surprising, and it is unclear if it is
even a reasonable default (GnuPG rejects it). But, even if the
programmer is aware of this, adding a new user ID does not replace
the empty User ID, and removing the empty User ID is a pain.
- Change the API to better match typical usage: have the constructor
take the User ID.
- Nevertheless, preserve the flexibility by making the User ID
optional to allow the caller to add a User ID later. In this
case, a non-autocrypt compliant TPK with no User ID is created
instead of an empty User ID.
- Closes #146.
-rw-r--r-- | openpgp-ffi/src/tpk.rs | 23 | ||||
-rw-r--r-- | openpgp/src/tpk/builder.rs | 28 | ||||
-rw-r--r-- | openpgp/src/tpk/mod.rs | 4 | ||||
-rw-r--r-- | openpgp/src/tsk.rs | 56 |
4 files changed, 90 insertions, 21 deletions
diff --git a/openpgp-ffi/src/tpk.rs b/openpgp-ffi/src/tpk.rs index b4026604..1649a6ae 100644 --- a/openpgp-ffi/src/tpk.rs +++ b/openpgp-ffi/src/tpk.rs @@ -574,10 +574,25 @@ pub extern "system" fn pgp_tpk_builder_default() -> *mut TPKBuilder { /// Generates a key compliant to [Autocrypt Level 1]. /// +/// Autocrypt requires a user id, however, if `uid` is NULL, a TPK is +/// created without any user ids. It is then the caller's +/// responsibility to ensure that a user id is added later. +/// +/// `uid` must contain valid UTF-8. If it does not contain valid +/// UTF-8, then the invalid code points are silently replaced with +/// `U+FFFD REPLACEMENT CHARACTER`. +/// /// [Autocrypt Level 1]: https://autocrypt.org/level1.html #[::ffi_catch_abort] #[no_mangle] -pub extern "system" fn pgp_tpk_builder_autocrypt() -> *mut TPKBuilder { - box_raw!(TPKBuilder::autocrypt(Autocrypt::V1)) +pub extern "system" fn pgp_tpk_builder_autocrypt(uid: *const c_char) + -> *mut TPKBuilder +{ + let uid = if uid.is_null() { + None + } else { + Some(ffi_param_cstr!(uid).to_string_lossy()) + }; + box_raw!(TPKBuilder::autocrypt(Autocrypt::V1, uid)) } /// Frees an `pgp_tpk_builder_t`. @@ -607,6 +622,10 @@ pub extern "system" fn pgp_tpk_builder_set_cipher_suite /// Adds a new user ID. The first user ID added replaces the default /// ID that is just the empty string. +/// +/// `uid` must contain valid UTF-8. If it does not contain valid +/// UTF-8, then the invalid code points are silently replaced with +/// `U+FFFD REPLACEMENT CHARACTER`. #[::ffi_catch_abort] #[no_mangle] pub extern "system" fn pgp_tpk_builder_add_userid (tpkb: *mut *mut TPKBuilder, uid: *const c_char) diff --git a/openpgp/src/tpk/builder.rs b/openpgp/src/tpk/builder.rs index 014ec5e4..8ab86cd4 100644 --- a/openpgp/src/tpk/builder.rs +++ b/openpgp/src/tpk/builder.rs @@ -89,12 +89,19 @@ impl Default for TPKBuilder { impl TPKBuilder { /// Generates a key compliant to - /// [Autocrypt](https://autocrypt.org/). If no version is given the latest - /// one is used. - pub fn autocrypt<V>(_: V) - -> Self where V: Into<Option<Autocrypt>> + /// [Autocrypt](https://autocrypt.org/). + /// + /// If no version is given the latest one is used. + /// + /// The autocrypt specification requires a UserID. However, + /// because it can be useful to add the UserID later, it is + /// permitted to be none. + pub fn autocrypt<'a, V, S>(_: V, userid: S) + -> Self + where V: Into<Option<Autocrypt>>, + S: Into<Option<Cow<'a, str>>> { - TPKBuilder{ + let builder = TPKBuilder{ ciphersuite: CipherSuite::RSA3k, primary: KeyBlueprint{ flags: KeyFlags::default() @@ -108,8 +115,14 @@ impl TPKBuilder { .set_encrypt_at_rest(true) } ], - userids: vec!["".into()], + userids: vec![], password: None, + }; + + if let Some(userid) = userid.into() { + builder.add_userid(userid) + } else { + builder } } @@ -474,12 +487,13 @@ mod tests { #[test] fn autocrypt() { - let (tpk1, _) = TPKBuilder::autocrypt(None) + let (tpk1, _) = TPKBuilder::autocrypt(None, Some("Foo".into())) .generate().unwrap(); assert_eq!(tpk1.primary().pk_algo(), PublicKeyAlgorithm::RSAEncryptSign); assert_eq!(tpk1.subkeys().next().unwrap().subkey().pk_algo(), PublicKeyAlgorithm::RSAEncryptSign); + assert_eq!(tpk1.userids().count(), 1); } #[test] diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs index 8e9b5f14..7ac044f6 100644 --- a/openpgp/src/tpk/mod.rs +++ b/openpgp/src/tpk/mod.rs @@ -3624,9 +3624,7 @@ mod test { // subkeys with and without a private key are merged. #[test] fn public_private_merge() { - use std::borrow::Cow; - - let (tsk, _) = TSK::new(Cow::Borrowed("foo@example.com")).unwrap(); + let (tsk, _) = TSK::new(Some("foo@example.com".into())).unwrap(); // tsk is now a tpk, but it still has its private bits. let tsk = tsk.into_tpk(); assert!(tsk.primary.secret().is_some()); diff --git a/openpgp/src/tsk.rs b/openpgp/src/tsk.rs index fff2a7ed..57a6ff2d 100644 --- a/openpgp/src/tsk.rs +++ b/openpgp/src/tsk.rs @@ -77,17 +77,12 @@ impl TSK { /// Generates a new key OpenPGP key. The key will be capable of encryption /// and signing. If no user id is given the primary self signature will be /// a direct key signature. - pub fn new<'a, O: Into<Option<Cow<'a,str>>>>(primary_uid: O) - -> Result<(TSK, Signature)> { + pub fn new<'a, O>(primary_uid: O) -> Result<(TSK, Signature)> + where O: Into<Option<Cow<'a, str>>> + { use tpk::TPKBuilder; - let mut key = TPKBuilder::autocrypt(None); - - match primary_uid.into() { - Some(uid) => { key = key.add_userid(uid); } - None => {} - } - + let key = TPKBuilder::autocrypt(None, primary_uid); let (tpk, revocation) = key.generate()?; Ok((TSK::from_tpk(tpk), revocation)) } @@ -212,6 +207,7 @@ impl Serialize for TSK { #[cfg(test)] mod tests { + use super::*; use tpk::TPKBuilder; #[test] @@ -251,4 +247,46 @@ mod tests { tpk2.userids().next().unwrap().userid()).unwrap(), true); } + + #[test] + fn user_ids() { + let (tpk, _) = TPKBuilder::default() + .add_userid("test1@example.com") + .add_userid("test2@example.com") + .generate().unwrap(); + + let userids = tpk + .userids() + .map(|binding| binding.userid().userid()) + .collect::<Vec<_>>(); + assert_eq!(userids.len(), 2); + assert!((userids[0] == b"test1@example.com" + && userids[1] == b"test2@example.com") + || (userids[0] == b"test2@example.com" + && userids[1] == b"test1@example.com"), + "User ids: {:?}", userids); + + + let (tpk, _) = TPKBuilder::autocrypt(None, Some("Foo".into())) + .generate() + .unwrap(); + + let userids = tpk + .userids() + .map(|binding| binding.userid().userid()) + .collect::<Vec<_>>(); + assert_eq!(userids.len(), 1); + assert_eq!(userids[0], b"Foo"); + + + let (tsk, _) = TSK::new(Some("test@example.com".into())).unwrap(); + let tpk = tsk.into_tpk(); + let userids = tpk + .userids() + .map(|binding| binding.userid().userid()) + .collect::<Vec<_>>(); + assert_eq!(userids.len(), 1); + assert_eq!(userids[0], b"test@example.com", + "User ids: {:?}", userids); + } } |