From 06d33917483dc38942ac38685b0d5f1312108187 Mon Sep 17 00:00:00 2001 From: Adam Wick Date: Wed, 3 Apr 2019 19:48:33 -0700 Subject: [PATCH] Cleaner number parsing, padding check. --- cryptonum | 2 +- src/dsa/params.rs | 2 +- src/ssh/errors.rs | 3 +- src/ssh/frame.rs | 57 ++++++++++++++++++++++++++++ src/ssh/mod.rs | 97 ++++++++++------------------------------------- 5 files changed, 80 insertions(+), 81 deletions(-) diff --git a/cryptonum b/cryptonum index 037413a..cac39b0 160000 --- a/cryptonum +++ b/cryptonum @@ -1 +1 @@ -Subproject commit 037413ad15c9abc6159f41b64c2dab9660390278 +Subproject commit cac39b0e5025d1b03c4502309697e480a3afc08d diff --git a/src/dsa/params.rs b/src/dsa/params.rs index c33ece1..107cae8 100644 --- a/src/dsa/params.rs +++ b/src/dsa/params.rs @@ -23,7 +23,7 @@ pub trait DSAParameters : ToASN1 macro_rules! generate_parameters { ($name: ident, $ltype: ident, $ntype: ident, $l: expr, $n: expr) => { - #[derive(Clone)] + #[derive(Clone,PartialEq)] pub struct $name { pub p: $ltype, pub g: $ltype, diff --git a/src/ssh/errors.rs b/src/ssh/errors.rs index bd6b986..52d6b67 100644 --- a/src/ssh/errors.rs +++ b/src/ssh/errors.rs @@ -19,7 +19,8 @@ pub enum SSHKeyParseError PrivateKeyCorruption, InconsistentKeyTypes(String,String), InconsistentPublicKeyValue, - InvalidPrivateKeyValue + InvalidPrivateKeyValue, + InvalidPadding } impl From for SSHKeyParseError { diff --git a/src/ssh/frame.rs b/src/ssh/frame.rs index 25be772..7e1796f 100644 --- a/src/ssh/frame.rs +++ b/src/ssh/frame.rs @@ -1,5 +1,6 @@ use base64::{decode,encode}; use byteorder::{BigEndian,ReadBytesExt,WriteBytesExt}; +use cryptonum::unsigned::{Decoder,Encoder}; use ssh::errors::{SSHKeyParseError,SSHKeyRenderError}; use std::io::Cursor; #[cfg(test)] @@ -140,6 +141,30 @@ pub fn render_openssh_buffer(output: &mut O, b: &[u8]) -> Result<(),SS //------------------------------------------------------------------------------ +pub fn parse_openssh_number(input: &mut I) -> Result + where + I: Read, + D: Decoder +{ + let mut buffer = parse_openssh_buffer(input)?; + while buffer[0] == 0 { buffer.remove(0); } + Ok(D::from_bytes(&buffer)) +} + +pub fn render_openssh_number(output: &mut O, n: &D) -> Result<(),SSHKeyRenderError> + where + O: Write, + D: Encoder +{ + let mut bytes = n.to_bytes(); + render_openssh_buffer(output, &bytes) +} + +//------------------------------------------------------------------------------ + +#[cfg(test)] +use cryptonum::unsigned::{U192,U1024,U2048,U4096}; + #[cfg(test)] quickcheck! { fn bytes_roundtrip(x: Vec) -> bool { @@ -194,6 +219,38 @@ quickcheck! { let check = parse_openssh_buffer(&mut cursor).unwrap(); os == check } + + fn u192(x: U192) -> bool { + let mut buffer = vec![]; + render_openssh_number(&mut buffer, &x).unwrap(); + let mut cursor = Cursor::new(buffer); + let check: U192 = parse_openssh_number(&mut cursor).unwrap(); + check == x + } + + fn u1024(x: U1024) -> bool { + let mut buffer = vec![]; + render_openssh_number(&mut buffer, &x).unwrap(); + let mut cursor = Cursor::new(buffer); + let check: U1024 = parse_openssh_number(&mut cursor).unwrap(); + check == x + } + + fn u2048(x: U2048) -> bool { + let mut buffer = vec![]; + render_openssh_number(&mut buffer, &x).unwrap(); + let mut cursor = Cursor::new(buffer); + let check: U2048 = parse_openssh_number(&mut cursor).unwrap(); + check == x + } + + fn u4096(x: U4096) -> bool { + let mut buffer = vec![]; + render_openssh_number(&mut buffer, &x).unwrap(); + let mut cursor = Cursor::new(buffer); + let check: U4096 = parse_openssh_number(&mut cursor).unwrap(); + check == x + } } #[cfg(test)] diff --git a/src/ssh/mod.rs b/src/ssh/mod.rs index 0dc684d..3f2b508 100644 --- a/src/ssh/mod.rs +++ b/src/ssh/mod.rs @@ -70,42 +70,12 @@ impl SSHKey for DSAKeyPair { return Err(SSHKeyParseError::UnknownKeyType(pubkey_type)); } - let mut pbytes = parse_openssh_buffer(&mut pubkey_cursor)?; - while pbytes[0] == 0 { pbytes.remove(0); } - if pbytes.len() > (1024 / 8) { - println!("pbytes.len() = {}", pbytes.len()); - println!("pbytes = {:?}", pbytes); - return Err(SSHKeyParseError::InvalidPublicKeyMaterial); - } - - let mut qbytes = parse_openssh_buffer(&mut pubkey_cursor)?; - while qbytes[0] == 0 { qbytes.remove(0); } - if qbytes.len() > (160 / 8) { - println!("qbytes.len() = {}", qbytes.len()); - return Err(SSHKeyParseError::InvalidPublicKeyMaterial); - } - - let mut gbytes = parse_openssh_buffer(&mut pubkey_cursor)?; - while gbytes[0] == 0 { gbytes.remove(0); } - if gbytes.len() > (1024 / 8) { - println!("gbytes.len() = {}", gbytes.len()); - return Err(SSHKeyParseError::InvalidPublicKeyMaterial); - } - - let mut ybytes = parse_openssh_buffer(&mut pubkey_cursor)?; - while ybytes[0] == 0 { ybytes.remove(0); } - if ybytes.len() > (1024 / 8) { - println!("ybytes.len() = {}", ybytes.len()); - return Err(SSHKeyParseError::InvalidPublicKeyMaterial); - } - - // Finally, we can turn this into a key - let p = U1024::from_bytes(&pbytes); - let g = U1024::from_bytes(&gbytes); - let q = U192::from_bytes(&qbytes); - let params = L1024N160::new(p, g, q); - let y = U1024::from_bytes(&ybytes); - let pubkey = DSAPubKey::::new(params, y); + let pubp = parse_openssh_number(&mut pubkey_cursor)?; + let pubq = parse_openssh_number(&mut pubkey_cursor)?; + let pubg = parse_openssh_number(&mut pubkey_cursor)?; + let pubparams = L1024N160::new(pubp, pubg, pubq); + let puby: U1024 = parse_openssh_number(&mut pubkey_cursor)?; + let pubkey = DSAPubKey::::new(pubparams.clone(), puby.clone()); // And now we can look at the private key! let mut privkey_cursor = Cursor::new(privkeys); @@ -120,52 +90,23 @@ impl SSHKey for DSAKeyPair { return Err(SSHKeyParseError::InconsistentKeyTypes(pubkey_type, privkey_type)); } - let mut pdata = parse_openssh_buffer(&mut privkey_cursor)?; - while pdata[0] == 0 { pdata.remove(0); } - if pdata != pbytes { + let privp = parse_openssh_number(&mut privkey_cursor)?; + let privq = parse_openssh_number(&mut privkey_cursor)?; + let privg = parse_openssh_number(&mut privkey_cursor)?; + let privparams = L1024N160::new(privp, privg, privq); + let privy = parse_openssh_number(&mut privkey_cursor)?; + let privx = parse_openssh_number(&mut privkey_cursor)?; + if (pubparams != privparams) || (puby != privy) { return Err(SSHKeyParseError::InconsistentPublicKeyValue); } - println!("p consistent"); - let p = U1024::from_bytes(&pdata); - let mut qdata = parse_openssh_buffer(&mut privkey_cursor)?; - while qdata[0] == 0 { qdata.remove(0); } - if qdata != qbytes { - return Err(SSHKeyParseError::InconsistentPublicKeyValue); - } - let q = U192::from_bytes(&qdata); - println!("q consistent"); - - let mut gdata = parse_openssh_buffer(&mut privkey_cursor)?; - while gdata[0] == 0 { gdata.remove(0); } - if gdata != gbytes { - return Err(SSHKeyParseError::InconsistentPublicKeyValue); - } - let g = U1024::from_bytes(&gdata); - println!("g consistent"); - - let params = L1024N160::new(p, g, q); - - // We don't need this, but it's good cross-validation - let mut ydata = parse_openssh_buffer(&mut privkey_cursor)?; - while ydata[0] == 0 { ydata.remove(0); } - if ydata != ybytes { - return Err(SSHKeyParseError::InconsistentPublicKeyValue); - } - println!("y consistent"); - - // Finally, what we actually - let mut xdata = parse_openssh_buffer(&mut privkey_cursor)?; - while xdata[0] == 0 { xdata.remove(0); } - if xdata.len() > (192 / 8) { - // FIXME: Should this test for too small a value? - return Err(SSHKeyParseError::InvalidPrivateKeyValue); - } - let x = U192::from_bytes(&xdata); - println!("x: {:X}", x); - let privkey = DSAPrivKey::::new(params, x); + let privkey = DSAPrivKey::::new(pubparams, privx); let comment = parse_openssh_string(&mut privkey_cursor)?; - println!("comment[{}] = {:?}", comment.len(), comment); + for (idx,byte) in privkey_cursor.bytes().enumerate() { + if ((idx+1) as u8) != byte? { + return Err(SSHKeyParseError::InvalidPadding); + } + } let result = DSAKeyPair{ public: pubkey, private: privkey }; Ok(result)