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

bug fix 4127. Restore RSA support for x509 public key import on PKCS11 #4128

Conversation

marianothiago
Copy link

Type of change

  • Bug fix

Description

While RSA has never been supported by Fabric for transaction signing and
verification, prior to version 2.x, MSPs could include CA certificates
that included RSA public keys. This was broken with the removal of the
unsupported RSA code.

It was fixed in SW but the error is still in PKCS11

This change modifies the key import function used by the MSP on PKCS11 to convert
an RSA public key to a bccsp.Key that can then be used as part of an MSP
identity.

This code does not add support for RSA on transaction paths.

Additional details

Related issues

#4127

@ale-linux
Copy link
Contributor

Thanks for your PR. A couple of Qs/remarks:

  1. please look at completing the DCO
  2. you say that this PR restores code that was mistakenly remove - which commit removed RSA from pkcs? Could you pls point me to it?

@denyeart
Copy link
Contributor

denyeart commented Apr 3, 2023

@ale-linux
RSA removal commit - 80a20e3
Discussion about RSA removal and decision to restore - #2069
Restore RSA for SW implementation - #2121

It looks like it was only restored for SW implementation, so it makes sense to restore for PKCS11 implementation as well.

@ale-linux
Copy link
Contributor

LGTM @marianothiago , if you follow the DCO and fix that issue, I think we're ready to merge!

…tificates that included RSA public keys

Signed-off-by: Thiago Mariano <mariano.thiago@gmail.com>
@marianothiago marianothiago force-pushed the issue-4127-PKCS11-dont-support-CA-with-RSA-public-key branch from a80c108 to 46cfd7d Compare April 17, 2023 12:52
@ale-linux ale-linux merged commit 394ef77 into hyperledger:release-2.2 Apr 17, 2023
13 checks passed
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.

None yet

3 participants