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

BouncyCastle FIPS/JSSE test does not work in native image #14139

Open
sberyozkin opened this issue Jan 6, 2021 · 23 comments
Open

BouncyCastle FIPS/JSSE test does not work in native image #14139

sberyozkin opened this issue Jan 6, 2021 · 23 comments
Assignees
Labels
area/security kind/bug Something isn't working

Comments

@sberyozkin
Copy link
Member

sberyozkin commented Jan 6, 2021

Describe the bug
integration-tests/bouncycastle-fips-jsse is only running in the JVM mode.

Expected behavior
Both tests pass in the native image

@sberyozkin sberyozkin added the kind/bug Something isn't working label Jan 6, 2021
@sberyozkin sberyozkin self-assigned this Jan 6, 2021
@ghost ghost added the triage/needs-triage label Jan 6, 2021
@sberyozkin
Copy link
Member Author

integration-tests/bouncycastle-fips passes in the native image for me, Java11, I might've confused it with integration-tests/bouncycastle-fips-jsse

@sberyozkin sberyozkin changed the title BouncyCastle FIPS and FIPS/JSSE tests do not work in native image BouncyCastle FIPS/JSSE tests does not work in native image Jan 6, 2021
@sberyozkin sberyozkin changed the title BouncyCastle FIPS/JSSE tests does not work in native image BouncyCastle FIPS/JSSE test does not work in native image Jan 6, 2021
@galderz
Copy link
Member

galderz commented Mar 19, 2021

We see this failure:

19:39:39 Error: No instances of sun.security.provider.NativePRNG$Blocking are allowed in the image heap as this class should be initialized at image runtime. To see how this object got instantiated use --trace-object-instantiation=sun.security.provider.NativePRNG$Blocking.
19:39:39 Detailed message:
19:39:39 Trace: Object was reached by 
19:39:39 	reading field java.security.SecureRandom.secureRandomSpi of
19:39:39 		constant java.security.SecureRandom@781f516f reached by 
19:39:39 	reading field org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.entropySource of
19:39:39 		constant org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider@558612c3 reached by 
19:39:39 	reading field sun.security.jca.ProviderConfig.provider of
19:39:39 		constant sun.security.jca.ProviderConfig@123aa002 reached by 
19:39:39 	indexing into array
19:39:39 		constant sun.security.jca.ProviderConfig[]@2806e73c reached by 
19:39:39 	reading field sun.security.jca.ProviderList.configs of
19:39:39 		constant sun.security.jca.ProviderList@764c00fc reached by 
19:39:39 	reading field sun.security.jca.Providers.providerList
19:39:39 
19:39:39 com.oracle.svm.core.util.UserError$UserException: No instances of sun.security.provider.NativePRNG$Blocking are allowed in the image heap as this class should be initialized at image runtime. To see how this object got instantiated use --trace-object-instantiation=sun.security.provider.NativePRNG$Blocking.
19:39:39 Detailed message:
19:39:39 Trace: Object was reached by 
19:39:39 	reading field java.security.SecureRandom.secureRandomSpi of
19:39:39 		constant java.security.SecureRandom@781f516f reached by 
19:39:39 	reading field org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.entropySource of
19:39:39 		constant org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider@558612c3 reached by 
19:39:39 	reading field sun.security.jca.ProviderConfig.provider of
19:39:39 		constant sun.security.jca.ProviderConfig@123aa002 reached by 
19:39:39 	indexing into array
19:39:39 		constant sun.security.jca.ProviderConfig[]@2806e73c reached by 
19:39:39 	reading field sun.security.jca.ProviderList.configs of
19:39:39 		constant sun.security.jca.ProviderList@764c00fc reached by 
19:39:39 	reading field sun.security.jca.Providers.providerList
19:39:39 
19:39:39 	at com.oracle.svm.core.util.UserError.abort(UserError.java:82)
19:39:39 	at com.oracle.svm.hosted.FallbackFeature.reportAsFallback(FallbackFeature.java:217)
19:39:39 	at com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:766)
19:39:39 	at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:556)
19:39:39 	at com.oracle.svm.hosted.NativeImageGenerator.lambda$run$0(NativeImageGenerator.java:469)
19:39:39 	at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(ForkJoinTask.java:1407)
19:39:39 	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
19:39:39 	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
19:39:39 	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
19:39:39 	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
19:39:39 	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)
19:39:39 Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: No instances of sun.security.provider.NativePRNG$Blocking are allowed in the image heap as this class should be initialized at image runtime. To see how this object got instantiated use --trace-object-instantiation=sun.security.provider.NativePRNG$Blocking.
19:39:39 Detailed message:
19:39:39 Trace: Object was reached by 
19:39:39 	reading field java.security.SecureRandom.secureRandomSpi of
19:39:39 		constant java.security.SecureRandom@781f516f reached by 
19:39:39 	reading field org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.entropySource of
19:39:39 		constant org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider@558612c3 reached by 
19:39:39 	reading field sun.security.jca.ProviderConfig.provider of
19:39:39 		constant sun.security.jca.ProviderConfig@123aa002 reached by 
19:39:39 	indexing into array
19:39:39 		constant sun.security.jca.ProviderConfig[]@2806e73c reached by 
19:39:39 	reading field sun.security.jca.ProviderList.configs of
19:39:39 		constant sun.security.jca.ProviderList@764c00fc reached by 
19:39:39 	reading field sun.security.jca.Providers.providerList
19:39:39 
19:39:39 	at com.oracle.graal.pointsto.constraints.UnsupportedFeatures.report(UnsupportedFeatures.java:126)
19:39:39 	at com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:763)
19:39:39 	... 8 more

You can use --trace-object-instantiation=sun.security.provider.NativePRNG$Blocking to find out what's making it be accessible.

@sberyozkin
Copy link
Member Author

sberyozkin commented Mar 30, 2021

Hi @galderz

This is what I've got after tracing one of the problems,

Error: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  Object has been initialized by the org.bouncycastle.crypto.general.AES class initializer with a trace: 
 	at org.bouncycastle.crypto.fips.FipsSecureRandom.<init>(Unknown Source)
	at org.bouncycastle.crypto.fips.FipsDRBG$Builder.build(Unknown Source)
	at org.bouncycastle.crypto.fips.FipsDRBG$Builder.build(Unknown Source)
	at org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.getDefaultSecureRandom(Unknown Source)
	at org.bouncycastle.jcajce.provider.ProvRandom$1.createInstance(Unknown Source)
	at org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider$BcService.newInstance(Unknown Source)
	at java.security.SecureRandom.getDefaultPRNG(SecureRandom.java:290)
	at java.security.SecureRandom.<init>(SecureRandom.java:219)
	at org.bouncycastle.crypto.general.Utils.<clinit>(Unknown Source)
	at org.bouncycastle.crypto.general.AES$AuthParameters.<init>(Unknown Source)
	at org.bouncycastle.crypto.general.AES.<clinit>(Unknown Source)

It is similar to BC FIPS case - but there it was originating at the method level so it was just stubbed it return true;
Here these are the internal fields of BouncyCastleFipsProvider but from what I understood recomputing the fields is a build time action... So my current GraalVM native experience is not enough to get me past this issue :-) so if you can recommend something that please do :-) (but it will be doc-ed now this option does not work in native image for now). Also CC @gsmet @cescoffier

@sberyozkin
Copy link
Member Author

sberyozkin commented Mar 31, 2021

@zakkak Hi - for this specific issue I can remove the native test for now for your builds not to be affected - as I only kept that test for me to experiment locally - as @galderz spotted earlier this test is not in CI anyway.
(In general I'm not sure the customers would want to run FIPS code with the substitutions - even if they are considered safe) - though as long as the affected classes are referenced in the docs it will help the users to make the decision

@zakkak
Copy link
Contributor

zakkak commented Mar 31, 2021

@zakkak Hi - for this specific issue I can remove the native test for now for your builds not to be affected - as I only kept that test for me to experiment locally

No, there is no need for that, we can skip them on our end! (cc @Karm )
Thanks @sberyozkin!

@sberyozkin
Copy link
Member Author

sberyozkin commented Mar 31, 2021

@zakkak @galderz

I'm progressing - the current problem is:

Error: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  Object has been initialized by the javax.crypto.JceSecurity class initializer with a trace: 
 	at org.bouncycastle.crypto.fips.FipsSecureRandom.<init>(Unknown Source)
	at org.bouncycastle.crypto.fips.FipsDRBG$Builder.build(Unknown Source)
	at org.bouncycastle.crypto.fips.FipsDRBG$Builder.build(Unknown Source)
	at org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.getDefaultSecureRandom(Unknown Source)
	at org.bouncycastle.jcajce.provider.ProvRandom$1.createInstance(Unknown Source)
	at org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider$BcService.newInstance(Unknown Source)
	at java.security.SecureRandom.getDefaultPRNG(SecureRandom.java:290)
	at java.security.SecureRandom.<init>(SecureRandom.java:219)
	at javax.crypto.JceSecurity.<clinit>(JceSecurity.java:80)

final javax.crypto.JceSecurity class has a static final SecureRandom RANDOM = new SecureRandom(); - which propagates to a FIPS default secure random provider.

If I try to runtime-reinitialize javax.crypto.JceSecurity then I see:

Caused by: com.oracle.svm.core.util.VMError$HostedError: Error in @InjectAccessors handling of field javax.crypto.JceSecurity.RANDOM, accessors class com.oracle.svm.core.jdk.JceSecurityAccessor: found no method named set or setRANDOM
	at com.oracle.svm.core.util.VMError.shouldNotReachHere(VMError.java:68)

@sberyozkin
Copy link
Member Author

@zakkak @galderz

This comment is of some concern - would it explain the error above ?

@zakkak
Copy link
Contributor

zakkak commented Mar 31, 2021

This comment is of some concern - would it explain the error above ?

Yes. Since javax.crypto.JceSecurity.RANDOM is handled in Target_javax_crypto_JceSecurity you shouldn't need to reinitialize it. However, at the same time you shouldn't be getting the error in #14139 (comment) :/

@sberyozkin
Copy link
Member Author

@zakkak thanks; FYI, I'm getting it with this PR, #16105, when running integration-tests/bouncycastle-fips-jsse - the executable actually starts if I try to re-init JceSecurity :-), i.e the build itself does not fail, but then it fails with that error ...JceSecurityAccessor: found no method named set or setRANDOM during the re-initialization try...
And if I do not try to re-init it then it fails with the above build error.
But I'm feeling it is nearly there :-)

@sberyozkin
Copy link
Member Author

@zakkak FYI, the error I'm getting on main now is

Error: Detected an instance of Random/SplittableRandom class in the image heap. 
...
Trace: Object was reached by 
	reading field org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.entropySource of
		constant org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider@ba0b690 reached by 
	indexing into array
		constant java.lang.Object[]@2dd95ae reached by 
	reading field java.util.IdentityHashMap.table of
		constant java.util.IdentityHashMap@2cc655f1 reached by 
	reading field javax.crypto.JceSecurity.verificationResults
...
Trace: Object was reached by 
	reading field org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider.providerDefaultRandom of
		constant org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider@ba0b690 reached by 
	indexing into array
		constant java.lang.Object[]@2dd95ae reached by 
	reading field java.util.IdentityHashMap.table of
		constant java.util.IdentityHashMap@2cc655f1 reached by 
	reading field javax.crypto.JceSecurity.verificationResults

@zakkak
Copy link
Contributor

zakkak commented Feb 16, 2022

@sberyozkin the following gets me past the above issue:

diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/graal/BouncyCastleSubstitutions.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/graal/BouncyCastleSubstitutions.java
index c22895906b..fbbc119975 100644
--- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/graal/BouncyCastleSubstitutions.java
+++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/graal/BouncyCastleSubstitutions.java
@@ -1,10 +1,14 @@
 package io.quarkus.security.runtime.graal;
 
+import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.Set;
 import java.util.function.BooleanSupplier;
 import java.util.stream.Collectors;
 
+import com.oracle.svm.core.annotate.Alias;
+import com.oracle.svm.core.annotate.RecomputeFieldValue;
+
 final class BouncyCastlePackages {
     static final String ORG_BOUNCYCASTLE_CRYPTO_PACKAGE = "org.bouncycastle.crypto";
     static final String ORG_BOUNCYCASTLE_CRYPTO_FIPS_PACKAGE = "org.bouncycastle.crypto.fips";
@@ -86,6 +90,17 @@ final class Target_org_bouncycastle_crypto_internal_AsymmetricCipherKeyPair {
 final class Target_org_bouncycastle_crypto_fips_RsaBlindedEngine {
 }
 
+@com.oracle.svm.core.annotate.TargetClass(className = "org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider", onlyWith = BouncyCastleCryptoFips.class)
+final class Target_org_bouncycastle_jcajce_provider_BouncyCastleFipsProvider {
+    @Alias //
+    @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset) //
+    private SecureRandom entropySource;
+
+    @Alias //
+    @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset) //
+    private SecureRandom providerDefaultRandom;
+}
+
 class BouncyCastleCryptoFips implements BooleanSupplier {
     @Override
     public boolean getAsBoolean() {

But it looks like the JUNIT tests are not native-image friendly. I am now getting:

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.259 s <<< FAILURE! - in io.quarkus.it.bouncycastle.BouncyCastleFipsJsseITCase
[ERROR] io.quarkus.it.bouncycastle.BouncyCastleFipsJsseITCase  Time elapsed: 0.259 s  <<< ERROR!
org.junit.platform.commons.JUnitException: @Inject is not supported in @NativeImageTest and @QuarkusIntegrationTest tests. Offending field is io.quarkus.it.bouncycastle.BouncyCastleFipsJsseTestCase.vertx
	at io.quarkus.test.junit.IntegrationTestUtil.ensureNoInjectAnnotationIsUsed(IntegrationTestUtil.java:80)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.ensureStarted(QuarkusIntegrationTestExtension.java:88)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeAll(QuarkusIntegrationTestExtension.java:83)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$10(ClassBasedTestDescriptor.java:381)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:381)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:205)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:80)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:148)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:188)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:154)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:128)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:428)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:562)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:548)

@sberyozkin
Copy link
Member Author

@zakkak Great, how does that work, is that essentially RuntimeReInitialize but for these specific fields only ?

Re Vertx, I recall awhile back I did a code where it was created manually - but it was optimized (this injected instance is property closed), and because the native test was disabled it was missed it would not work in native.
Can you please init it to Vertx.vertx() instead and see if the test works ? I can then take care of closing it in the PR (I think I'll have to make the substitution conditional but I know how to do it now).
If the test does not work then it can also be added to AutoFeature (I can update the existing PR)

Thanks

@sberyozkin
Copy link
Member Author

@zakkak

I think I'll have to make the substitution conditional but I know how to do it now

Sorry, I did not scroll :-), see you've taken care of it, thanks

@zakkak
Copy link
Contributor

zakkak commented Feb 17, 2022

@zakkak Great, how does that work, is that essentially RuntimeReInitialize but for these specific fields only ?

Kind of, it essentially instructs GraalVM to initialize this field to a specific value different than the one it had during compilation. In that particular case I am resetting the field so its value will be null in the native image heap. Note that the value is calculated and stored during build-time so there is no (re) initialization at run-time in contrast to RuntimeReinitialize.

Can you please init it to Vertx.vertx() instead and see if the test works ?

I tried that and now it results in:

Caused by: java.security.NoSuchAlgorithmException: BCFKS KeyStore not available

So you probably need to add BCFKS as well :)

@sberyozkin
Copy link
Member Author

@zakkak Thanks for the explanation, and sure, I will try to update my PR accordingly

@zakkak
Copy link
Contributor

zakkak commented Feb 17, 2022

👍

I will try to update my PR accordingly

In case it helps I have pushed my changes (on top of your branch) in https://github.com/zakkak/quarkus/tree/bc_keypair_ecdsa_xdh-zakkak

@sberyozkin
Copy link
Member Author

sberyozkin commented Feb 22, 2022

Hi @zakkak Here is an extra code I've added:

diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java
index 72061ca374..d14c4140f2 100644
--- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java
+++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java
@@ -243,23 +243,44 @@ public class SecurityProcessor {
             autoFeatureAugmentors.produce(new NativeImageAutoFeatureAugmentorBuildItem(new AutoFeatureAugmentor() {
                 @Override
                 public void augment(TryBlock overallCatch) {
-                    final int sunJsseIndex = findProviderIndex(SecurityProviderUtils.SUN_JSSE_PROVIDER_NAME);
-
-                    final String commonProviderName = bouncyCastleJsseProvider.get().isInFipsMode()
-                            ? SecurityProviderUtils.BOUNCYCASTLE_FIPS_PROVIDER_CLASS_NAME
-                            : SecurityProviderUtils.BOUNCYCASTLE_PROVIDER_CLASS_NAME;
-
-                    ResultHandle bcCommonProvider = overallCatch
-                            .newInstance(MethodDescriptor.ofConstructor(commonProviderName));
-                    ResultHandle bcJsseProvider = overallCatch.newInstance(
-                            MethodDescriptor.ofConstructor(SecurityProviderUtils.BOUNCYCASTLE_JSSE_PROVIDER_CLASS_NAME));
-
-                    overallCatch.invokeStaticMethod(
-                            MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class, int.class),
-                            bcCommonProvider, overallCatch.load(sunJsseIndex));
-                    overallCatch.invokeStaticMethod(
-                            MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class, int.class),
-                            bcJsseProvider, overallCatch.load(sunJsseIndex + 1));
+                    if (!bouncyCastleJsseProvider.get().isInFipsMode()) {
+                        final int sunJsseIndex = findProviderIndex(SecurityProviderUtils.SUN_JSSE_PROVIDER_NAME);
+
+                        ResultHandle bcProvider = overallCatch
+                                .newInstance(
+                                        MethodDescriptor.ofConstructor(SecurityProviderUtils.BOUNCYCASTLE_PROVIDER_CLASS_NAME));
+                        ResultHandle bcJsseProvider = overallCatch.newInstance(
+                                MethodDescriptor.ofConstructor(SecurityProviderUtils.BOUNCYCASTLE_JSSE_PROVIDER_CLASS_NAME));
+
+                        overallCatch.invokeStaticMethod(
+                                MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class,
+                                        int.class),
+                                bcProvider, overallCatch.load(sunJsseIndex));
+                        overallCatch.invokeStaticMethod(
+                                MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class,
+                                        int.class),
+                                bcJsseProvider, overallCatch.load(sunJsseIndex + 1));
+                    } else {
+                        final int sunIndex = findProviderIndex(SecurityProviderUtils.SUN_PROVIDER_NAME);
+
+                        ResultHandle bcFipsProvider = overallCatch
+                                .newInstance(MethodDescriptor
+                                        .ofConstructor(SecurityProviderUtils.BOUNCYCASTLE_FIPS_PROVIDER_CLASS_NAME));
+                        MethodDescriptor bcJsseProviderConstructor = MethodDescriptor.ofConstructor(
+                                SecurityProviderUtils.BOUNCYCASTLE_JSSE_PROVIDER_CLASS_NAME, "boolean",
+                                Provider.class.getName());
+                        ResultHandle bcJsseProvider = overallCatch.newInstance(bcJsseProviderConstructor,
+                                overallCatch.load(true), bcFipsProvider);
+
+                        overallCatch.invokeStaticMethod(
+                                MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class,
+                                        int.class),
+                                bcFipsProvider, overallCatch.load(sunIndex));
+                        overallCatch.invokeStaticMethod(
+                                MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class,
+                                        int.class),
+                                bcJsseProvider, overallCatch.load(sunIndex + 1));
+                    }
                 }
             }));
         } else {

to get it registered in the feature as well, so alongside your changes it is progressing, now I'm getting a bunch of errors related to org.bouncycastle.math.ec.ECPoint (from bc-fips), it has this block of code

/*
     * Temporary code to provide randomness in the 1.0.2.X branch only
     */
    private static SecureRandom testRandom;
    private static synchronized SecureRandom getTestRandom()
    {
        if (testRandom == null)
        {
            // SP 800-89 requires use of an approved DRBG.
            testRandom = FipsDRBG.SHA256.fromEntropySource(new SecureRandom(), false).build(
                Pack.longToBigEndian(System.currentTimeMillis()), false,
                Strings.toByteArray(ECPoint.class.getName()));
        }
        return testRandom;
    }

and then, in the same class

public ECPoint normalize()
{
// some other code
SecureRandom r = getTestRandom();
            ECFieldElement b = curve.randomFieldElementMult(r);
}

We've been able to sub some test methods using static SecureRandom with no ops methods, but I'm not sure what can be done here, can you recommend something ?

Thanks

@sberyozkin
Copy link
Member Author

By the way, org.bouncycastle.math.ec.ECPoint is already requested to be reinitialized in SecurityProcessor

@zakkak
Copy link
Contributor

zakkak commented Feb 23, 2022

By the way, org.bouncycastle.math.ec.ECPoint is already requested to be reinitialized in SecurityProcessor

Actually that is what is causing the issue :)

In general the logic to approach issues with Random instances appearing in the native image heap is the following:

  1. Is the Random stored in a static field of a class? If so, we need to either reset it's value to null by 1st ensuring that the class can handle this, or register the class for runtime re-initialization. The latter though has the side effect you are observing here, i.e. instances of this class can no longer appear in the native image heap.
  2. Is the Random stored in a non-static field of a class? If so, we need to either reset it's value to null by 1st ensuring that the class can handle this, or avoid instantiating the corresponding class at build-time. The latter is usually done through substitutions.

Practically, applying the following on top of your patch:

diff --git a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java
index 72061ca374..67e3c7aad7 100644
--- a/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java
+++ b/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java
@@ -179,13 +179,6 @@ public class SecurityProcessor {
         } else {
             reflection.produce(new ReflectiveClassBuildItem(true, true, true, "org.bouncycastle.crypto.general.AES"));
             runtimeReInitialized.produce(new RuntimeReinitializedClassBuildItem("org.bouncycastle.crypto.general.AES"));
-            runtimeReInitialized
-                    .produce(new RuntimeReinitializedClassBuildItem("org.bouncycastle.math.ec.custom.sec.SecP521R1Curve"));
-            runtimeReInitialized
-                    .produce(new RuntimeReinitializedClassBuildItem("org.bouncycastle.math.ec.custom.sec.SecP384R1Curve"));
-            runtimeReInitialized
-                    .produce(new RuntimeReinitializedClassBuildItem("org.bouncycastle.math.ec.custom.sec.SecP256R1Curve"));
-            runtimeReInitialized.produce(new RuntimeReinitializedClassBuildItem("org.bouncycastle.math.ec.ECPoint"));
             runtimeReInitialized
                     .produce(new RuntimeReinitializedClassBuildItem(
                             "org.bouncycastle.crypto.asymmetric.NamedECDomainParameters"));

Makes the test pass on my machine 🎉 (Disclaimer, I didn't check to see if it breaks something else though :) )

@sberyozkin
Copy link
Member Author

sberyozkin commented Feb 23, 2022

Hi @zakkak Great stuff, and thanks for the nice explanation (can't promise I won't ask again for some clarifications though :-) ).

At some point in the past adding those lines helped to get past some of the native build issues, I'd not otherwise be even aware of those org.bouncycastle.math.ec.custom.sec classes.

I think it is safe to remove them, so I'll clean up my PR later on and this issue will also be resolved.

As a side note, if someone did want to use say org.bouncycastle.math.ec.custom.sec.SecP521R1Curve directly (which extends org.bouncycastle.math.ec.ECPoint). How would we handle it ? Random is static and setting to null would cause NPE in ECPoint.normalize. I suppose we'd next have to try and substitute ECPoint.normalize with a no-op code and see if it can handle it ?

@zakkak
Copy link
Contributor

zakkak commented Feb 23, 2022

can't promise I won't ask again for some clarifications though :-)

No worries :)
Feel free to ping me anytime.

As a side note, if someone did want to use say org.bouncycastle.math.ec.custom.sec.SecP521R1Curve directly (which extends org.bouncycastle.math.ec.ECPoint). How would we handle it ? Random is static and setting to null would cause NPE in ECPoint.normalize. I suppose we'd next have to try and substitute ECPoint.normalize with a no-op code and see if it can handle it ?

In ECPoint the SecureRandom field testRandom is private and is accessed through getTestRandom which handles the case where its value is null (at least in bc-fips-1.0.2.3.jar). So for ECPoint the right thing to do would be to reset its value with something like:

@com.oracle.svm.core.annotate.TargetClass(className = "org.bouncycastle.math.ec.ECPoint", onlyWith = BouncyCastleCryptoFips.class)
final class Target_org_bouncycastle_math_ec_ECPoint {
    @Alias //
    @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset) //
    private SecureRandom testRandom;
}

Note this is similar to what I did in zakkak@78ec0db for BouncyCastleFipsProvider.

It actually seems a good idea to add the above in the PR you are preparing :)

@sberyozkin
Copy link
Member Author

@zakkak

In ECPoint the SecureRandom field testRandom is private and is accessed through getTestRandom which handles the case where its value is null

Nice, good point; so yeah, I'll get that reset as you recommend; for now I got the test running, it was disabled :-), but now at least it runs and fails with the same one, java.security.KeyStoreException: BCFKS not found - I'm yet to verify if it is a test Vert.x client issue or not; I think I'll probably need to register the provider behind BCFKS as well

@pjgg
Copy link
Contributor

pjgg commented May 7, 2022

@sberyozkin this PR is based on your previous work and is related to this PR

quarkus-qe/quarkus-test-suite#638

I think that could be handy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants