Skip to content

Make PSS/OAEP params use generic AlgorithmIdentifier #799

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.

Sign up for GitHub

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

Merged
tarcieri merged 2 commits into from
Dec 17, 2022
Merged

Make PSS/OAEP params use generic AlgorithmIdentifier #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30dacc8
AlgorithmIdentifier: move assert_algorithm_oid to generic impl
lumag Dec 17, 2022
cd37b03
feat: params: switch to generic AlgorithmIdentifier implementation
lumag Dec 17, 2022
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
18 changes: 6 additions & 12 deletions pkcs1/src/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,12 @@ use der::{
asn1::ContextSpecificRef, Decode, DecodeValue, Encode, EncodeValue, FixedTag, Reader, Sequence,
Tag, TagMode, TagNumber, Writer,
};
use spki::AlgorithmIdentifierRef;
use spki::{AlgorithmIdentifier, AlgorithmIdentifierRef};

const OID_SHA_1: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.3.14.3.2.26");
const OID_MGF_1: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.8");
const OID_PSPECIFIED: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.9");

// TODO(tarcieri): make `AlgorithmIdentifier` generic around params; use `OID_SHA_1`
const SEQ_OID_SHA_1_DER: &[u8] = &[0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02, 0x1a];

const SHA_1_AI: AlgorithmIdentifierRef<'_> = AlgorithmIdentifierRef {
oid: OID_SHA_1,
parameters: None,
Expand Down Expand Up @@ -84,7 +81,7 @@ pub struct RsaPssParams<'a> {
pub hash: AlgorithmIdentifierRef<'a>,

/// Mask Generation Function (MGF)
pub mask_gen: AlgorithmIdentifierRef<'a>,
pub mask_gen: AlgorithmIdentifier<AlgorithmIdentifierRef<'a>>,
Copy link
Member

@baloo baloo May 23, 2024

Choose a reason for hiding this comment

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

Context: I've just submitted #1415 that rewrites the AlgorithmIdentifier to make sure the parameter is either Any or AnyRef.

I'm confused about this change. This doesn't seem to line up with https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.1.2 (spki::AlgorithmIdentifier)

   AlgorithmIdentifier  ::=  SEQUENCE  {
        algorithm               OBJECT IDENTIFIER,
        parameters              ANY DEFINED BY algorithm OPTIONAL  }

Which, from my understand, ask that the parameters are encoded as an Any.

This change moves the implementation to remove the type prefix that Any would provide.

That said, this AlgorithmIdentifier would be defined by https://datatracker.ietf.org/doc/html/rfc8017#page-70:

   -- ================
   --   Useful types
   -- ================

   ALGORITHM-IDENTIFIER ::= CLASS {
       &id    OBJECT IDENTIFIER  UNIQUE,
       &Type  OPTIONAL
   }
       WITH SYNTAX { OID &id [PARAMETERS &Type] }

Which one is right here?
Could it be we need another definition of AlgorithmIdentifier altogether?

I think I would otherwise revert that change and get back the SEQ_OID_SHA_1_DER that was removed.


/// Salt length
pub salt_len: u8,
Expand Down Expand Up @@ -180,13 +177,10 @@ impl<'a> TryFrom<&'a [u8]> for RsaPssParams<'a> {
}

/// Default Mask Generation Function (MGF): SHA-1.
fn default_mgf1_sha1<'a>() -> AlgorithmIdentifierRef<'a> {
AlgorithmIdentifierRef {
fn default_mgf1_sha1<'a>() -> AlgorithmIdentifier<AlgorithmIdentifierRef<'a>> {
AlgorithmIdentifier::<AlgorithmIdentifierRef<'a>> {
oid: OID_MGF_1,
parameters: Some(
AnyRef::new(Tag::Sequence, SEQ_OID_SHA_1_DER)
.expect("error creating default MGF1 params"),
),
parameters: Some(SHA_1_AI),
}
}

Expand All @@ -211,7 +205,7 @@ pub struct RsaOaepParams<'a> {
pub hash: AlgorithmIdentifierRef<'a>,

/// Mask Generation Function (MGF)
pub mask_gen: AlgorithmIdentifierRef<'a>,
pub mask_gen: AlgorithmIdentifier<AlgorithmIdentifierRef<'a>>,

/// The source (and possibly the value) of the label L
pub p_source: AlgorithmIdentifierRef<'a>,
Expand Down
65 changes: 25 additions & 40 deletions pkcs1/tests/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
//! PKCS#1 algorithm params tests

use const_oid::db;
use der::{
asn1::{ObjectIdentifier, OctetStringRef},
Decode, Encode,
};
use der::{asn1::OctetStringRef, Encode};
use hex_literal::hex;
use pkcs1::{RsaOaepParams, RsaPssParams, TrailerField};

Expand All @@ -31,15 +28,12 @@ fn decode_pss_param() {
.mask_gen
.assert_algorithm_oid(db::rfc5912::ID_MGF_1)
.is_ok());
assert_eq!(
param
.mask_gen
.parameters_any()
.unwrap()
.sequence(|reader| Ok(ObjectIdentifier::decode(reader)?))
.unwrap(),
db::rfc5912::ID_SHA_256
);
assert!(param
.mask_gen
.parameters
.unwrap()
.assert_algorithm_oid(db::rfc5912::ID_SHA_256)
.is_ok());
assert_eq!(param.salt_len, 32);
assert_eq!(param.trailer_field, TrailerField::BC);
}
Expand Down Expand Up @@ -67,15 +61,12 @@ fn decode_pss_param_default() {
.mask_gen
.assert_algorithm_oid(db::rfc5912::ID_MGF_1)
.is_ok());
assert_eq!(
param
.mask_gen
.parameters_any()
.unwrap()
.sequence(|reader| Ok(ObjectIdentifier::decode(reader)?))
.unwrap(),
db::rfc5912::ID_SHA_1
);
assert!(param
.mask_gen
.parameters
.unwrap()
.assert_algorithm_oid(db::rfc5912::ID_SHA_1)
.is_ok());
assert_eq!(param.salt_len, 20);
assert_eq!(param.trailer_field, TrailerField::BC);
assert_eq!(param, Default::default())
Expand Down Expand Up @@ -103,15 +94,12 @@ fn decode_oaep_param() {
.mask_gen
.assert_algorithm_oid(db::rfc5912::ID_MGF_1)
.is_ok());
assert_eq!(
param
.mask_gen
.parameters_any()
.unwrap()
.sequence(|reader| Ok(ObjectIdentifier::decode(reader)?))
.unwrap(),
db::rfc5912::ID_SHA_256
);
assert!(param
.mask_gen
.parameters
.unwrap()
.assert_algorithm_oid(db::rfc5912::ID_SHA_256)
.is_ok());
assert!(param
.p_source
.assert_algorithm_oid(db::rfc5912::ID_P_SPECIFIED)
Expand Down Expand Up @@ -145,15 +133,12 @@ fn decode_oaep_param_default() {
.mask_gen
.assert_algorithm_oid(db::rfc5912::ID_MGF_1)
.is_ok());
assert_eq!(
param
.mask_gen
.parameters_any()
.unwrap()
.sequence(|reader| Ok(ObjectIdentifier::decode(reader)?))
.unwrap(),
db::rfc5912::ID_SHA_1
);
assert!(param
.mask_gen
.parameters
.unwrap()
.assert_algorithm_oid(db::rfc5912::ID_SHA_1)
.is_ok());
assert!(param
.p_source
.assert_algorithm_oid(db::rfc5912::ID_P_SPECIFIED)
Expand Down
4 changes: 3 additions & 1 deletion spki/src/algorithm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
/// `AlgorithmIdentifier` reference which has `AnyRef` parameters.
pub type AlgorithmIdentifierRef<'a> = AlgorithmIdentifier<AnyRef<'a>>;

impl<'a> AlgorithmIdentifierRef<'a> {
impl<Params> AlgorithmIdentifier<Params> {
/// Assert the `algorithm` OID is an expected value.
pub fn assert_algorithm_oid(&self, expected_oid: ObjectIdentifier) -> Result<ObjectIdentifier> {
if self.oid == expected_oid {
Expand All @@ -87,7 +87,9 @@ impl<'a> AlgorithmIdentifierRef<'a> {
Err(Error::OidUnknown { oid: expected_oid })
}
}
}

impl<'a> AlgorithmIdentifierRef<'a> {
/// Assert `parameters` is an OID and has the expected value.
pub fn assert_parameters_oid(
&self,
Expand Down