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

Jump to bottom

add Identity support for RSA PKCS#8 nocrypt #260

Closed
wants to merge 1 commit into from
Closed

add Identity support for RSA PKCS#8 nocrypt #260

wants to merge 1 commit into from

Conversation

dwhjames
Copy link
Contributor

Hi @str4d,

I realize that this feature is unlikely to see much use in practice, but I picked an old issue that seemed simple enough for a very new rustacean to be able to tackle (first real attempt at Rust).

fully parse unencrypted PKCS#8 RSA private key

parse encrypted PKCS#8 enough to recognize it as an unsupported key format

add test coverage for key format support

  • PEM (PKCS#1) unencrypted RSA (supported)
  • PEM (PKCS#1) encrypted RSA (never support)
  • PKCS#8 unencrypted RSA (supported)
  • PKCS#8 encrypted (not supported, yet?)

fixes #14

@dwhjames
add Identity support for RSA PKCS#8 nocrypt
931f0e7 
fully parse unencrypted PKCS#8 RSA private key

parse encrypted PKCS#8 enough to recognize it as an unsupported key format

add test coverage for key format support
- PEM (PKCS#1) unencrypted RSA (supported)
- PEM (PKCS#1) encrypted RSA (never support)
- PKCS#8 unencrypted RSA (supported)
- PKCS#8 encrypted (not supported, yet?)

fixes str4d#14
@tarcieri
Copy link

You could use the following crates for this, rather than implementing these standards yourself:

@dwhjames
Copy link
Contributor Author

Hi @tarcieri,

Thanks for the direction to prior work. I carried on in the existing style of the codebase with the use of nom, but the complexity of following in that style is ramping up.

Would a reasonable starting point be to pause on PKCS#8 and instead go back and replace what is already there (linked below) with pkcs1?

rage/age/src/ssh/identity.rs

Lines 300 to 328 in 729d9da

fn rsa_privkey(input: &str) -> IResult<&str, Identity> {
preceded(
pair(tag("-----BEGIN RSA PRIVATE KEY-----"), line_ending),
terminated(
map_opt(
pair(
opt(terminated(rsa_pem_encryption_header, line_ending)),
wrapped_str_while_encoded(base64::STANDARD),
),
|(enc_header, privkey)| {
if enc_header.is_some() {
Some(UnsupportedKey::EncryptedPem.into())
} else {
read_asn1::rsa_privkey(&privkey).ok().map(|(_, privkey)| {
let mut ssh_key = vec![];
cookie_factory::gen(
write_ssh::rsa_pubkey(&privkey.to_public_key()),
&mut ssh_key,
)
.expect("can write into a Vec");
UnencryptedKey::SshRsa(ssh_key, Box::new(privkey)).into()
})
}
},
),
pair(line_ending, tag("-----END RSA PRIVATE KEY-----")),
),
)(input)
}

rage/age/src/ssh.rs

Lines 155 to 270 in 729d9da

mod read_asn1 {
use nom::{
bytes::complete::{tag, take},
combinator::{map, map_opt},
error::{make_error, ErrorKind},
multi::{length_data, length_value},
sequence::{preceded, terminated, tuple},
IResult,
};
use rsa::BigUint;
fn der_type(class: u8, pc: u8, num: u8) -> impl Fn(&[u8]) -> IResult<&[u8], &[u8]> {
assert!(class < 4);
assert!(pc < 2);
assert!(num < 31);
move |input: &[u8]| tag(&[(class << 6) | (pc << 5) | num])(input)
}
fn der_length(input: &[u8]) -> IResult<&[u8], usize> {
let (mid, len_byte) = take(1usize)(input)?;
let len_byte = len_byte[0];
// Reject indefinite and reserved
if len_byte == 128 || len_byte == 255 {
return Err(nom::Err::Failure(make_error(input, ErrorKind::LengthValue)));
}
if (len_byte & 128) == 0 {
// Definite, short
Ok((mid, len_byte as usize))
} else {
// Definite, long
let num_len_bytes = (len_byte & 127) as usize;
let (i, len_bytes) = take(num_len_bytes)(mid)?;
len_bytes
.iter()
.fold(Ok(0usize), |acc, x| {
acc.and_then(|acc| {
acc.checked_shl(8)
.ok_or_else(|| {
nom::Err::Failure(make_error(mid, ErrorKind::LengthValue))
})
.map(|acc| acc + (*x as usize))
})
})
.map(|result| (i, result))
}
}
fn integer(input: &[u8]) -> IResult<&[u8], BigUint> {
preceded(
// Type: Universal | Primitive | INTEGER
der_type(0, 0, 2),
map(length_data(der_length), BigUint::from_bytes_be),
)(input)
}
fn tag_version(ver: u8) -> impl Fn(&[u8]) -> IResult<&[u8], &[u8]> {
move |input: &[u8]| {
preceded(
// Type: Universal | Primitive | INTEGER
der_type(0, 0, 2),
length_value(
map_opt(der_length, |ver_len| match ver_len {
1 => Some(ver_len),
_ => None,
}),
tag(&[ver]),
),
)(input)
}
}
/// A PKCS#1-encoded RSA private key.
///
/// From [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.2):
/// ```text
/// RSAPrivateKey ::= SEQUENCE {
/// version Version,
/// modulus INTEGER, -- n
/// publicExponent INTEGER, -- e
/// privateExponent INTEGER, -- d
/// prime1 INTEGER, -- p
/// prime2 INTEGER, -- q
/// exponent1 INTEGER, -- d mod (p-1)
/// exponent2 INTEGER, -- d mod (q-1)
/// coefficient INTEGER, -- (inverse of q) mod p
/// otherPrimeInfos OtherPrimeInfos OPTIONAL
/// }
/// ```
///
/// We only support the two-prime encoding, where `version = 0` and `otherPrimeInfos`
/// is omitted.
pub(super) fn rsa_privkey(input: &[u8]) -> IResult<&[u8], rsa::RsaPrivateKey> {
preceded(
// Type: Universal | Constructed | SEQUENCE
der_type(0, 1, 16),
length_value(
der_length,
preceded(
tag_version(0),
terminated(
map(
tuple((integer, integer, integer, integer, integer)),
|(n, e, d, p, q)| {
rsa::RsaPrivateKey::from_components(n, e, d, vec![p, q])
},
),
// d mod (p-1), d mod (q-1), iqmp
tuple((integer, integer, integer)),
),
),
),
)(input)
}
}

@tarcieri
Copy link

That's what I'd personally suggest.

PKCS#5 support is fully integrated into the pkcs8 crate, and there's blanket impl of pkcs8 support for anything that impls pkcs1::{FromRsaPrivateKey, FromRsaPublicKey, ToRsaPrivateKey, ToRsaPublicKey}. So if you impl the pkcs1 traits you get PKCS#8 and optionally PKCS#5 for free.

@dwhjames
Copy link
Contributor Author

@tarcieri

While I have your attention,

rage/age/src/ssh.rs

Line 260 in 729d9da

rsa::RsaPrivateKey::from_components(n, e, d, vec![p, q])

rage/age/src/ssh/identity.rs

Line 313 in 729d9da

read_asn1::rsa_privkey(&privkey).ok().map(|(_, privkey)| {

rage uses rsa::RsaPrivateKey and that implements the ToRsaPrivateKey trait (trait impl), so that simplifies the original code in rage that I linked to above down to a one-liner?

@tarcieri
Copy link

Yep, as of v0.5 the rsa crate impls the pkcs1 traits I just mentioned.

dwhjames added a commit to dwhjames/rage that referenced this pull request Sep 12, 2021
@dwhjames
use pkcs1 crate to parse RSAPrivateKey ASN.1 object
29213f8 
following from review conversation:
str4d#260 (comment)

references:

- [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.2)
- [rsa::pkcs1](https://docs.rs/rsa/0.5.0/rsa/#pkcs1-rsa-key-encoding)
@dwhjames dwhjames mentioned this pull request Sep 12, 2021
Merged
@dwhjames
Copy link
Contributor Author

I appreciate the review and the pointers @tarcieri

I followed up in PR #261

@dwhjames dwhjames closed this Sep 12, 2021
@dwhjames dwhjames mentioned this pull request Sep 13, 2021
Closed
str4d pushed a commit to dwhjames/rage that referenced this pull request Oct 26, 2022
@dwhjames @str4d
use pkcs1 crate to parse RSAPrivateKey ASN.1 object
32ad684 
following from review conversation:
str4d#260 (comment)

references:

- [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.2)
- [rsa::pkcs1](https://docs.rs/rsa/0.5.0/rsa/#pkcs1-rsa-key-encoding)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support PKCS#8 SSH keys
2 participants
@dwhjames @tarcieri