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

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

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

woodruffw
Copy link
Contributor

This adds hash and mask generation algorithm ID definitions for the three RSA-PSS configurations allowed under the CABF BRs, mirroring the existing definitions for RSA-PSS with SHA-1.

Breakout from #9405.

See 7.1.3.2.1 RSA in the CABF BRs for this: https://cabforum.org/wp-content/uploads/CA-Browser-Forum-BR-v2.0.0.pdf (page 97).

Breakout from pyca#9405.

Signed-off-by: William Woodruff <[email protected]>
Comment on lines +268 to +282
// 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,
};
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.

@@ -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

@alex alex merged commit c6d7bdf into pyca:main Aug 14, 2023
@woodruffw woodruffw deleted the tob-webpki-pss branch August 14, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants