Ensure DSA signatures are encoded according to spec

ssh-key/src/signature.rs

@@ -12,6 +12,7 @@ use crate::{private::Ed25519Keypair, public::Ed25519PublicKey};
 #[cfg(feature = "dsa")]
 use {
     crate::{private::DsaKeypair, public::DsaPublicKey},
+    bigint::BigUint,
     sha1::Sha1,
     signature::{DigestSigner, DigestVerifier},
 };
@@ -320,9 +321,20 @@ impl Signer<Signature> for DsaKeypair {
             .try_sign_digest(Sha1::new_with_prefix(message))
             .map_err(|_| signature::Error::new())?;
 
+        // Note that we need to roll our own signature encoding, as [RFC4253 section 6.6]
+        // specifies two raw 80 bit integer but the dsa::SigningKey serialization
+        // encodes to a der format.
+        let mut buf: Vec<u8> = Vec::new();
+        buf.append(&mut signature.r().to_bytes_be());
+        buf.append(&mut signature.s().to_bytes_be());
+
+        if buf.len() != DSA_SIGNATURE_SIZE {
+            return Err(signature::Error::new());
+        }
+
         Ok(Signature {
             algorithm: Algorithm::Dsa,
-            data: signature.to_vec(),
+            data: buf,
         })
     }
 }
@@ -332,8 +344,15 @@ impl Verifier<Signature> for DsaPublicKey {
     fn verify(&self, message: &[u8], signature: &Signature) -> signature::Result<()> {
         match signature.algorithm {
             Algorithm::Dsa => {
-                let signature = dsa::Signature::try_from(signature.data.as_slice())?;
-
+                let data = signature.data.as_slice();
+                if data.len() != DSA_SIGNATURE_SIZE {
+                    return Err(signature::Error::new());
+                }
+                let (r, s) = data.split_at(DSA_SIGNATURE_SIZE / 2);
+                let signature = dsa::Signature::from_components(
+                    BigUint::from_bytes_be(r),
+                    BigUint::from_bytes_be(s),
+                )?;
                 dsa::VerifyingKey::try_from(self)?
                     .verify_digest(Sha1::new_with_prefix(message), &signature)
                     .map_err(|_| signature::Error::new())

ssh-key/tests/sshsig.rs

@@ -16,6 +16,9 @@ use ssh_key::PrivateKey;
 #[cfg(feature = "ed25519")]
 use ssh_key::Error;
 
+#[cfg(feature = "dsa")]
+use {encoding::Decode, signature::Verifier, ssh_key::Signature};
+
 /// DSA OpenSSH-formatted private key.
 #[cfg(feature = "dsa")]
 const DSA_PRIVATE_KEY: &str = include_str!("examples/id_dsa_1024");
@@ -76,6 +79,13 @@ const MSG_EXAMPLE: &[u8] = b"testing";
 /// Example domain/namespace used for the message.
 const NAMESPACE_EXAMPLE: &str = "example";
 
+/// An ssh-agent signature response signing MSG_EXAMPLE with DSA_PRIVATE_KEY
+#[cfg(feature = "dsa")]
+const DSA_SIGNATURE_BYTES: [u8; 55] = hex!(
+    "000000077373682d647373000000282d0c9613d9745c6088ae4d9e8dbf35a557"
+    "7bd0e6796acccb22ab4809d569e86ec619510ec48b6950"
+);
+
 #[test]
 fn decode_ed25519() {
     let sshsig = ED25519_SIGNATURE.parse::<SshSig>().unwrap();
@@ -134,6 +144,17 @@ fn sign_dsa() {
     );
 }
 
+#[test]
+#[cfg(feature = "dsa")]
+fn verify_dsa() {
+    let signature = Signature::decode(&mut DSA_SIGNATURE_BYTES.as_ref()).unwrap();
+    let verifying_key = DSA_PUBLIC_KEY.parse::<PublicKey>().unwrap();
+    verifying_key
+        .key_data()
+        .verify(MSG_EXAMPLE, &signature)
+        .unwrap();
+}
+
 #[test]
 #[cfg(feature = "p256")]
 fn sign_ecdsa_p256() {