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

support ring and aws-lc-rs (default = ring) #402

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ base64 = "0.22"
# For PEM decoding
pem = { version = "3", optional = true }
simple_asn1 = { version = "0.6", optional = true }
aws-lc-rs = { version = "1.8.1", optional = true }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
ring = { version = "0.17.4", features = ["std"] }
Expand All @@ -50,6 +51,7 @@ criterion = { version = "0.4", default-features = false }
[features]
default = ["use_pem"]
use_pem = ["pem", "simple_asn1"]
aws_lc_rs = ["dep:aws-lc-rs"]

[[bench]]
name = "jwt"
Expand Down
47 changes: 47 additions & 0 deletions src/crypto/core.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#[cfg(all(feature = "aws_lc_rs", not(any(target_arch = "wasm32", target_os = "windows"))))]
pub(crate) mod dep {
pub(crate) use ::aws_lc_rs::{constant_time, error, hmac, rand};

pub(crate) mod signature {
pub(crate) use ::aws_lc_rs::signature::*;

#[inline]
pub(crate) fn ecdsa_key_pair_from_pkcs8(
alg: &'static EcdsaSigningAlgorithm,
pkcs8: &[u8],
_rng: &dyn ::aws_lc_rs::rand::SecureRandom,
) -> Result<EcdsaKeyPair, ::aws_lc_rs::error::KeyRejected> {
EcdsaKeyPair::from_pkcs8(alg, pkcs8)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other functions used by jsonwebtoken have the rng param which is ignored. Sadly this one seems to be forgotten. We could also try to upstream this to aws-lc-rs, but would require a major release, not sure how eager they are for that :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! -- I stumbled across this thread, and thought I would provide a little context around this difference.

In ring v0.16.x, EcdsaKeyPair::from_pkcs8 only takes two parameters. But in ring 0.17, this function takes three parameters. Because aws-lc-rs v1.x maintains compatibility with ring 0.16.x, our function (likewise) only takes two parameters.

Unfortunately, I don't see a way we could support a 3rd parameter w/o breaking backwards compatibility. But feel free to drop us an issue if you have questions or would like to suggest an improvement to our API. Thanks!

}

#[inline]
pub(crate) fn rsa_key_pair_public_modulus_len(key_pair: &RsaKeyPair) -> usize {
key_pair.public_modulus_len()
}
}
}

#[cfg(not(all(feature = "aws_lc_rs", not(any(target_arch = "wasm32", target_os = "windows")))))]
pub(crate) mod dep {
pub(crate) use ::ring::{constant_time, error, hmac, rand};

pub(crate) mod signature {
pub(crate) use ::ring::signature::*;

#[inline]
pub(crate) fn ecdsa_key_pair_from_pkcs8(
alg: &'static EcdsaSigningAlgorithm,
pkcs8: &[u8],
rng: &dyn ::ring::rand::SecureRandom,
) -> Result<EcdsaKeyPair, ::ring::error::KeyRejected> {
EcdsaKeyPair::from_pkcs8(alg, pkcs8, rng)
}

#[inline]
pub(crate) fn rsa_key_pair_public_modulus_len(key_pair: &RsaKeyPair) -> usize {
key_pair.public().modulus_len()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ring does have key_pair.public_modulus_len() but it's deprecated.
So this can be removed if we upstream a patch to aws-lc-rs to have public() also available.

}
}
}

pub(crate) use dep::*;
5 changes: 2 additions & 3 deletions src/crypto/ecdsa.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use ring::{rand, signature};

use crate::algorithms::Algorithm;
use crate::crypto::core::{rand, signature};
use crate::errors::Result;
use crate::serialization::b64_encode;

Expand Down Expand Up @@ -32,7 +31,7 @@ pub fn sign(
message: &[u8],
) -> Result<String> {
let rng = rand::SystemRandom::new();
let signing_key = signature::EcdsaKeyPair::from_pkcs8(alg, key, &rng)?;
let signing_key = signature::ecdsa_key_pair_from_pkcs8(alg, key, &rng)?;
let out = signing_key.sign(&rng, message)?;
Ok(b64_encode(out))
}
3 changes: 1 addition & 2 deletions src/crypto/eddsa.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use ring::signature;

use crate::algorithms::Algorithm;
use crate::crypto::core::signature;
use crate::errors::Result;
use crate::serialization::b64_encode;

Expand Down
6 changes: 3 additions & 3 deletions src/crypto/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use ring::constant_time::verify_slices_are_equal;
use ring::{hmac, signature};

use crate::algorithms::Algorithm;
use crate::crypto::core::constant_time::verify_slices_are_equal;
use crate::crypto::core::{hmac, signature};
use crate::decoding::{DecodingKey, DecodingKeyKind};
use crate::encoding::EncodingKey;
use crate::errors::Result;
use crate::serialization::{b64_decode, b64_encode};

pub(crate) mod core;
pub(crate) mod ecdsa;
pub(crate) mod eddsa;
pub(crate) mod rsa;
Expand Down
5 changes: 2 additions & 3 deletions src/crypto/rsa.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use ring::{rand, signature};

use crate::algorithms::Algorithm;
use crate::crypto::core::{rand, signature};
use crate::errors::{ErrorKind, Result};
use crate::serialization::{b64_decode, b64_encode};

Expand Down Expand Up @@ -41,7 +40,7 @@ pub(crate) fn sign(
let key_pair = signature::RsaKeyPair::from_der(key)
.map_err(|e| ErrorKind::InvalidRsaKey(e.to_string()))?;

let mut signature = vec![0; key_pair.public().modulus_len()];
let mut signature = vec![0; signature::rsa_key_pair_public_modulus_len(&key_pair)];
let rng = rand::SystemRandom::new();
key_pair.sign(alg, &rng, message, &mut signature).map_err(|_| ErrorKind::RsaFailedSigning)?;

Expand Down
10 changes: 5 additions & 5 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub enum ErrorKind {
/// Some of the text was invalid UTF-8
Utf8(::std::string::FromUtf8Error),
/// Something unspecified went wrong with crypto
Crypto(::ring::error::Unspecified),
Crypto(crate::crypto::core::error::Unspecified),
}

impl StdError for Error {
Expand Down Expand Up @@ -159,14 +159,14 @@ impl From<::std::string::FromUtf8Error> for Error {
}
}

impl From<::ring::error::Unspecified> for Error {
fn from(err: ::ring::error::Unspecified) -> Error {
impl From<crate::crypto::core::error::Unspecified> for Error {
fn from(err: crate::crypto::core::error::Unspecified) -> Error {
new_error(ErrorKind::Crypto(err))
}
}

impl From<::ring::error::KeyRejected> for Error {
fn from(_err: ::ring::error::KeyRejected) -> Error {
impl From<crate::crypto::core::error::KeyRejected> for Error {
fn from(_err: crate::crypto::core::error::KeyRejected) -> Error {
new_error(ErrorKind::InvalidEcdsaKey)
}
}
Expand Down