Include public key in signature (#224)

Author: casey

Cargo.lock

@@ -42,14 +42,8 @@ walkdir = "2.5.0"
 
 [dev-dependencies]
 pretty_assertions = "1.4.1"
-ssh-key = { version = "0.6.0", features = ["ed25519"] }
 tempfile = "3.24.0"
 
-[dev-dependencies.sequoia-openpgp]
-version = "2.1.0"
-default-features = false
-features = ["allow-experimental-crypto", "allow-variable-time-crypto", "crypto-rust"]
-
 [lib]
 doctest = false
 

Cargo.toml

@@ -196,8 +196,7 @@ such as `C:`.
 ### `notes`
 
 The value of the mandatory `notes` key is an array of signed notes. Notes are
-objects containing a single mandatory key `signatures`, an object mapping
-public keys to signatures.
+objects containing a single mandatory key `signatures`, an array of signatures.
 
 Notes may optionally contain a `time` field whose value is a timestamp given as
 the number of nanoseconds after the UNIX epoch.
@@ -228,16 +227,17 @@ signed by the public key
   },
   "notes": [
     {
-      "signatures": {
-        "public1a67dndhhmae7p6fsfnj0z37zf78cde6mwqgtms0y87h8ldlvvflyqcxnd63": "…"
-      },
+      "signatures": [
+        "…"
+      ],
       "time": 1768531681809767000
     }
   ]
 }
 ```
 
-The signature is elided for brevity.
+The signature is elided for brevity. Signatures are bech32m-encoded strings
+containing both a public key and an Ed25519 signature.
 
 Keys, Signatures, Fingerprints, and Hashes
 ------------------------------------------
@@ -431,7 +431,7 @@ filepack sign
 ```
 
 Which signs the manifest in the current directory with your master key and adds
-the signature to the manifest's `signatures` map. Signatures are made over a
+the signature to the manifest's `signatures` array. Signatures are made over a
 fingerprint hash, recursively calculated from the contents of the manifest.
 
 ### Signature Verification

README.md

@@ -8,20 +8,13 @@ pub(crate) struct Bech32Decoder<'a> {
 }
 
 impl<'a> Bech32Decoder<'a> {
-  pub(crate) fn byte_array<const LEN: usize>(mut self) -> Result<[u8; LEN], Bech32Error> {
+  pub(crate) fn byte_array<const LEN: usize>(&mut self) -> Result<[u8; LEN], Bech32Error> {
     let mut array = [0; LEN];
 
     for (slot, byte) in array.iter_mut().zip(self.bytes(LEN)?) {
       *slot = byte;
     }
 
-    let excess = self.data.len() - self.i;
-
-    ensure! {
-      excess == 0,
-      bech32_error::Overlong { excess, ty: self.ty },
-    }
-
     Ok(array)
   }
 
@@ -41,6 +34,27 @@ impl<'a> Bech32Decoder<'a> {
     Ok(fes.fes_to_bytes())
   }
 
+  pub(crate) fn decode_byte_array<const LEN: usize>(
+    ty: Bech32Type,
+    s: &'a str,
+  ) -> Result<[u8; LEN], Bech32Error> {
+    let mut decoder = Self::new(ty, s)?;
+    let array = decoder.byte_array()?;
+    decoder.done()?;
+    Ok(array)
+  }
+
+  pub(crate) fn done(self) -> Result<(), Bech32Error> {
+    let excess = self.data.len() - self.i;
+
+    ensure! {
+      excess == 0,
+      bech32_error::Overlong { excess, ty: self.ty },
+    }
+
+    Ok(())
+  }
+
   fn fes(
     &mut self,
     len: usize,
@@ -105,8 +119,7 @@ mod tests {
     #[track_caller]
     fn case(s: &str, err: &str) {
       assert_eq!(
-        Bech32Decoder::new(Bech32Type::PublicKey, &checksum(s))
-          .and_then(Bech32Decoder::byte_array::<1>)
+        Bech32Decoder::decode_byte_array::<1>(Bech32Type::PublicKey, &checksum(s))
           .unwrap_err()
           .to_string(),
         err,

src/bech32_decoder.rs

@@ -23,8 +23,7 @@ impl FromStr for Fingerprint {
   type Err = Bech32Error;
 
   fn from_str(s: &str) -> Result<Self, Self::Err> {
-    let decoder = Bech32Decoder::new(Bech32Type::Fingerprint, s)?;
-    let inner = decoder.byte_array()?;
+    let inner = Bech32Decoder::decode_byte_array(Bech32Type::Fingerprint, s)?;
     Ok(Self(inner.into()))
   }
 }

src/fingerprint.rs

@@ -58,6 +58,7 @@ use {
     public_key_error::PublicKeyError,
     serialized_message::SerializedMessage,
     sign_options::SignOptions,
+    signature_error::SignatureError,
     style::Style,
     subcommand::Subcommand,
     tag::Tag,
@@ -77,7 +78,9 @@ use {
   owo_colors::Styled,
   regex::Regex,
   serde::{Deserialize, Deserializer, Serialize, Serializer},
-  serde_with::{DeserializeFromStr, MapPreventDuplicates, SerializeDisplay, serde_as},
+  serde_with::{
+    DeserializeFromStr, MapPreventDuplicates, SerializeDisplay, SetPreventDuplicates, serde_as,
+  },
   snafu::{ErrorCompat, OptionExt, ResultExt, Snafu, ensure},
   std::{
     backtrace::{Backtrace, BacktraceStatus},
@@ -199,6 +202,7 @@ mod relative_path;
 mod serialized_message;
 mod sign_options;
 mod signature;
+mod signature_error;
 mod style;
 mod subcommand;
 mod tag;

src/lib.rs

@@ -100,14 +100,15 @@ impl Manifest {
     for note in &mut self.notes {
       if note.message(message.fingerprint) == message {
         ensure! {
-          note.signatures.insert(key, signature).is_none() || options.overwrite,
+          !note.has_signature(key) || options.overwrite,
           error::SignatureAlreadyExists { key },
         }
+        note.signatures.insert(signature);
         return Ok(());
       }
     }
 
-    self.notes.push(Note::from_message(message, key, signature));
+    self.notes.push(Note::from_message(message, signature));
 
     Ok(())
   }
@@ -128,7 +129,8 @@ impl Manifest {
     let mut digests = BTreeMap::new();
     let mut signatures = BTreeMap::new();
     for (index, note) in self.notes.iter().enumerate() {
-      for &public_key in note.signatures.keys() {
+      for signature in &note.signatures {
+        let public_key = signature.public_key();
         if let Some(first) = signatures.insert(public_key, index) {
           return Err(
             error::DuplicateSignature {

src/manifest.rs

@@ -4,26 +4,25 @@ use super::*;
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 #[serde(deny_unknown_fields, rename_all = "kebab-case")]
 pub struct Note {
-  #[serde_as(as = "MapPreventDuplicates<_, _>")]
-  pub signatures: BTreeMap<PublicKey, Signature>,
+  #[serde_as(as = "SetPreventDuplicates<_>")]
+  pub signatures: BTreeSet<Signature>,
   #[serde(default, skip_serializing_if = "is_default")]
   pub time: Option<u128>,
 }
 
 impl Note {
-  pub(crate) fn from_message(
-    message: Message,
-    public_key: PublicKey,
-    signature: Signature,
-  ) -> Self {
+  pub(crate) fn from_message(message: Message, signature: Signature) -> Self {
     Self {
-      signatures: [(public_key, signature)].into(),
+      signatures: [signature].into(),
       time: message.time,
     }
   }
 
-  pub(crate) fn has_signature(&self, public_key: PublicKey) -> bool {
-    self.signatures.contains_key(&public_key)
+  pub fn has_signature(&self, public_key: PublicKey) -> bool {
+    self
+      .signatures
+      .iter()
+      .any(|signature| signature.public_key() == public_key)
   }
 
   pub(crate) fn message(&self, fingerprint: Fingerprint) -> Message {
@@ -35,8 +34,8 @@ impl Note {
 
   pub(crate) fn verify(&self, fingerprint: Fingerprint) -> Result<u64> {
     let serialized = self.message(fingerprint).serialize();
-    for (public_key, signature) in &self.signatures {
-      public_key.verify(&serialized, signature)?;
+    for signature in &self.signatures {
+      signature.verify(&serialized)?;
     }
     Ok(self.signatures.len().into_u64())
   }
@@ -49,7 +48,7 @@ mod tests {
   #[test]
   fn duplicate_fields_are_rejected() {
     assert_eq!(
-      serde_json::from_str::<Note>(r#"{"signatures":{},"signatures":{}}"#)
+      serde_json::from_str::<Note>(r#"{"signatures":[],"signatures":[]}"#)
         .unwrap_err()
         .to_string(),
       "duplicate field `signatures` at line 1 column 29",
@@ -59,28 +58,26 @@ mod tests {
   #[test]
   fn duplicate_signatures_are_rejected() {
     let json = format!(
-      r#"{{"signatures":{{"{}":"{}","{}":"{}"}}}}"#,
-      test::PUBLIC_KEY,
+      r#"{{"signatures":["{}","{}"]}}"#,
       test::SIGNATURE,
-      test::PUBLIC_KEY,
       test::SIGNATURE,
     );
 
     assert_matches_regex! {
       serde_json::from_str::<Note>(&json).unwrap_err().to_string(),
-      r"invalid entry: found duplicate key at line 1 column \d+",
+      r"invalid entry: found duplicate value at line 1 column \d+",
     }
   }
 
   #[test]
   fn optional_fields_are_not_serialized() {
     assert_eq!(
       serde_json::to_string(&Note {
-        signatures: BTreeMap::new(),
+        signatures: BTreeSet::new(),
         time: None,
       })
       .unwrap(),
-      r#"{"signatures":{}}"#,
+      r#"{"signatures":[]}"#,
     );
   }
 }

src/note.rs

@@ -49,16 +49,15 @@ impl PrivateKey {
 
   pub(crate) fn sign(&self, message: &SerializedMessage) -> Signature {
     use ed25519_dalek::Signer;
-    Signature::new(self.0.sign(message.as_bytes()))
+    Signature::new(self.0.sign(message.as_bytes()), self.public_key())
   }
 }
 
 impl FromStr for PrivateKey {
   type Err = Bech32Error;
 
   fn from_str(key: &str) -> Result<Self, Self::Err> {
-    let decoder = Bech32Decoder::new(Bech32Type::PrivateKey, key)?;
-    let inner = decoder.byte_array()?;
+    let inner = Bech32Decoder::decode_byte_array(Bech32Type::PrivateKey, key)?;
     let inner = ed25519_dalek::SigningKey::from_bytes(&inner);
     assert!(!inner.verifying_key().is_weak());
     Ok(Self(inner))

src/private_key.rs

@@ -42,10 +42,6 @@ impl PublicKey {
 
     Ok(public_key)
   }
-
-  pub(crate) fn verify(self, message: &SerializedMessage, signature: &Signature) -> Result {
-    signature.verify(message, self)
-  }
 }
 
 impl From<PrivateKey> for PublicKey {
@@ -58,8 +54,7 @@ impl FromStr for PublicKey {
   type Err = PublicKeyError;
 
   fn from_str(key: &str) -> Result<Self, Self::Err> {
-    let decoder = Bech32Decoder::new(Bech32Type::PublicKey, key)?;
-    let inner = decoder.byte_array()?;
+    let inner = Bech32Decoder::decode_byte_array(Bech32Type::PublicKey, key)?;
     Self::from_bytes(inner)
   }
 }