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.

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

common: add more RSA-PSS algorithm id definitions #9412

Merged
merged 1 commit into from
Aug 14, 2023
Merged

common: add more RSA-PSS algorithm id definitions #9412

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 32 additions & 0 deletions src/rust/cryptography-x509/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,22 @@ pub const PSS_SHA1_HASH_ALG: AlgorithmIdentifier<'_> = AlgorithmIdentifier {
params: AlgorithmParameters::Sha1(Some(())),
};

// RSA-PSS ASN.1 hash algorithm definitions specified under the CA/B Forum BRs.
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment really correct? It's not really defined by CABF, these come from the RFCs, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's right, although CABF doesn't make reference to the RFCs for their definitions of these. Want me to dig them up regardless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.rfc-editor.org/rfc/rfc8017 looks like the right RFC, although it doesn't explicitly parametrize these the way CABF does.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, really? I just assumed that these were all natural consequences of how it worked, and didn't need to be explicitly specified (same as SHA1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, CABF specifies the precise set that can be used (and provides their exact DER encodings as well) -- both are on page 97 of the PDF I linked in the top.

Copy link
Member

Choose a reason for hiding this comment

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

Ok

pub const PSS_SHA256_HASH_ALG: AlgorithmIdentifier<'_> = AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: AlgorithmParameters::Sha256(Some(())),
};

pub const PSS_SHA384_HASH_ALG: AlgorithmIdentifier<'_> = AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: AlgorithmParameters::Sha384(Some(())),
};

pub const PSS_SHA512_HASH_ALG: AlgorithmIdentifier<'_> = AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: AlgorithmParameters::Sha512(Some(())),
};

// This is defined as an AlgorithmIdentifier in RFC 4055,
// but the mask generation algorithm **must** contain an AlgorithmIdentifier
// in its params, so we define it this way.
Expand All @@ -249,6 +265,22 @@ pub const PSS_SHA1_MASK_GEN_ALG: MaskGenAlgorithm<'_> = MaskGenAlgorithm {
params: PSS_SHA1_HASH_ALG,
};

// RSA-PSS ASN.1 mask gen algorithms defined under the CA/B Forum BRs.
pub const PSS_SHA256_MASK_GEN_ALG: MaskGenAlgorithm<'_> = MaskGenAlgorithm {
oid: oid::MGF1_OID,
params: PSS_SHA256_HASH_ALG,
};

pub const PSS_SHA384_MASK_GEN_ALG: MaskGenAlgorithm<'_> = MaskGenAlgorithm {
oid: oid::MGF1_OID,
params: PSS_SHA384_HASH_ALG,
};

pub const PSS_SHA512_MASK_GEN_ALG: MaskGenAlgorithm<'_> = MaskGenAlgorithm {
oid: oid::MGF1_OID,
params: PSS_SHA512_HASH_ALG,
};
Comment on lines +268 to +282
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NB: No coverage needed since these are just data, but for maximum correctness I could also test these against the exact expected DER encodings supplied by the CABF rules.


// From RFC 4055 section 3.1:
// RSASSA-PSS-params ::= SEQUENCE {
// hashAlgorithm [0] HashAlgorithm DEFAULT
Expand Down