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

JCE: Implements RSA key gen benchmark #95

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jackctj117
Copy link

Added RSA key gen for WolfJCE and BC benchmarks.

Copy link
Member

@cconlon cconlon left a comment

Choose a reason for hiding this comment

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

Please see comments. Let's also add SunJCE RSA benchmarks as part of this PR.

examples/provider/CryptoBenchmark.java Outdated Show resolved Hide resolved
examples/provider/CryptoBenchmark.java Outdated Show resolved Hide resolved
examples/provider/CryptoBenchmark.java Outdated Show resolved Hide resolved
examples/provider/CryptoBenchmark.java Outdated Show resolved Hide resolved
examples/provider/CryptoBenchmark.java Outdated Show resolved Hide resolved
examples/provider/CryptoBenchmark.java Outdated Show resolved Hide resolved
@cconlon cconlon removed their assignment Jan 24, 2025
System.out.println("-----------------------------------------------------------------------------");

for (Provider provider : providers) {
if (!provider.getName().equals("SunJCE")) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we are not benchmarking SunJCE "RSA/ECB/PKCS1Padding"?

Copy link
Author

Choose a reason for hiding this comment

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

When I look online it look like SunJCE does not have completed RSA implementation. Since JDK 5 the RSA implementation is split between different providers where SunRSASign does key pair generation and SunJCE handles RSA encryption/decryption. When lines 410 and 419 are commented out lines (which is an if statement that excludes SunJSE from the RSA test) I get the following:

java.security.NoSuchAlgorithmException: no such algorithm: RSA for provider SunJCE
	at sun.security.jca.GetInstance.getService(GetInstance.java:87)
	at sun.security.jca.GetInstance.getInstance(GetInstance.java:206)
	at java.security.KeyPairGenerator.getInstance(KeyPairGenerator.java:279)
	at CryptoBenchmark.runRSABenchmark(Unknown Source)
	at CryptoBenchmark.main(Unknown Source)

Copy link
Member

Choose a reason for hiding this comment

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

You are correct that Sun/Oracle RSA support is split between different providers depending on the operation.

The SunJCE provider does have support for RSA/ECB/PKCS1Padding. But, you will need to use the SunRsaSign provider for KeyPairGenerator("RSA") support, ie:

keyGen = KeyPairGenerator.getInstance("RSA", "SunRsaSign");

That's ok for us to do here. SunJCE will probably never have KeyPairGenerator support for RSA, since it will likely always be in SunRsaSign. As such, we should group SunJCE/SunRsaSign together as one group when we are testing the "Sun / Java" provider. Go ahead and use SunRsaSign for the KeyPairGenerator part and SunJCE for the other RSA operation benchmarks. Then, in your output we should probably list the SunJCE section like:

SunJCE / SunRsaSign:
RSA 2048 key gen (RSA/ECB/PKCS1Padding)                 12 ops took 1.061 sec, avg 88.424 ms, 11.309 ops/sec
RSA 2048 public (RSA/ECB/PKCS1Padding)              22031 ops took 1.000 sec, avg 0.045 ms, 22030.882 ops/sec

And in the comparison table, call out SunRsaSign for the RSA key gen op, ie:

| RSA/ECB/PKCS1Padding RSA 2048 key gen    | SunRsaSign   |    -2.36 |    -20.9 |

@cconlon cconlon removed their assignment Jan 31, 2025
@jackctj117 jackctj117 requested a review from cconlon January 31, 2025 23:18
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