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

feat: add support for SHA-256 RSA PSS signatures #9

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

madztheo
Copy link
Contributor

Description

Currently, the library only supports the PKCS#1 v1.5 scheme for RSA signatures. However, there is also another common scheme, namely RSA PSS. As it is more recent and considered more secure than PKCS#1 v1.5, adding support for it would be a great addition to the library.

In the context of electronic passports, some countries are using RSA PSS, nearly exclusively with SHA-256. Thus, this PR aims to add support for the RSA PSS signature scheme using SHA-256 for the hash function and MGF1 for the mask generation function.

Problem*

Lack of support for RSA PSS signatures.

Summary*

This PR adds a new verify_sha256_pss function that can take in a signature generated with the PSS scheme and verify its validity against the provided message hash. It also requires to pass it the modulus size in bits as this value is required to decode the signature correctly. The exponent is assumed to be 65537.

pub fn verify_sha256_pss(_: Self, instance: BNInstance, msg_hash: [u8; 32], sig: BN, key_size: u32) -> bool

A private function (not exposed to outside projects referencing the library) was also added to compute the MGF1 mask of a given seed, as it is required by the RSA PSS.

fn mgf1_sha256<let SEED_LEN: u32, let MASK_LEN: u32>(seed: [u8; SEED_LEN]) -> [u8; MASK_LEN]

As this is not specific to RSA PSS, it might be worth exporting this function to a separate location and exposing it for other use cases.

Three test cases were added to make sure the verification logic works correctly.

#[test]
fn test_mgf1_sha256()
#[test]
fn test_verify_sha256_pss_2048()
#[test]
fn test_verify_sha256_pss_1964()

The first verifies that the mask generation function MGF1 works as expected.
The second verifies a regular 2048-bit RSA signature.
And the last verifies a more unusual 1964-bit RSA signature to test an edge case for key size not divisible by 8.

An argument was added to the signature_gen Rust package to generate an RSA PSS signature. As such, running the following command in /signature_gen will generate a PSS signature for the message "Hello World! This is Noir-RSA":

cargo run -- --msg "Hello World! This is Noir-RSA" --pss

Additional Context

RSA PSS is more complex than PKCS#1 v1.5 and as such the gate count will be higher for verifying this type of signature. But the precise gate count has not been estimated yet.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

lib/src/rsa.nr Show resolved Hide resolved
lib/src/rsa.nr Show resolved Hide resolved
lib/src/rsa.nr Outdated Show resolved Hide resolved
lib/src/rsa.nr Outdated Show resolved Hide resolved
lib/src/rsa.nr Outdated
Comment on lines 177 to 182
let bits_to_mask = 8 * em_len - em_bits;
let mask = 0xFF >> bits_to_mask as u8;
let first_byte = masked_db[offset];
// Make sure the 8 * em_len - em_bits leftmost bits are 0
// c.f. https://github.com/RustCrypto/RSA/blob/aeedb5adf5297892fcb9e11f7c0f6c0157005c58/src/algorithms/pss.rs#L205
assert((first_byte & !mask) == 0);
Copy link
Member

Choose a reason for hiding this comment

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

This can be done more cheaply by just performing a division rather than bitwise operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I switched to using powers of 2 now

lib/src/rsa.nr Outdated
Comment on lines 183 to 186
let bits_to_mask = 8 * em_len - em_bits;
let mask_value = pow(2, bits_to_mask as u32);
let max_allowed_value = 255 / mask_value;
assert(masked_db[0] as u32 <= max_allowed_value);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let bits_to_mask = 8 * em_len - em_bits;
let mask_value = pow(2, bits_to_mask as u32);
let max_allowed_value = 255 / mask_value;
assert(masked_db[0] as u32 <= max_allowed_value);
let bits_to_mask = 8 * em_len - em_bits;
let mask_value = pow(2, bits_to_mask as u32);
assert_eq(masked_db[0] as u32 / mask_value, 0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually not the desired behavior. What we want to check is that all the bits_to_mask leftmost bits are all equal to 0. Here, it's checking that the 8 - bits_to_mask leftmost bits are zero. But following your logic, I'll replace my code with this:

let bits_to_mask = 8 - (8 * em_len - em_bits);
let mask_value = pow(2, bits_to_mask as u32);
assert_eq(masked_db[0] as u32 / mask_value, 0);

@TomAFrench
Copy link
Member

This has merge conflicts with master.

@madztheo
Copy link
Contributor Author

This has merge conflicts with master.

I've resolved the conflicts. It should be good to go now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants