-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Bouncy Castle BCFIPS provider fails native executable build after the BC FIPS version bump to 1.0.2.4 #37500
Comments
/cc @sberyozkin |
@michalvavrik native build passes in Quarkus, may be more substitutions are needed, I'm not seeing there where exactly that SecureRandom is set. By the way it is not supported at the product level |
@sberyozkin IIRC the same error is thrown in the |
@michalvavrik, @zakkak, thanks Foivos, so can we have #14139 closed as a duplicate it looks like this issue refers to the same existing problem with the BC FIPS JSSE but via a different reproducer ? |
There must be some difference considering the reproducer I provided worked until last week and #14139 is opened for more than 2 years. I'm not sure you wouldn't loose some information by closing it. |
@michalvavrik Sure but in both cases it is not BC FIPS on its own but the BC_FIPS and BC_TLS_FIPS combo which is specifically about securing the TLS connection as opposed to just dealing with the FIPS algorithms. Let me adjust the title a bit |
@michalvavrik https://quarkus.io/guides/security-customization#bouncy-castle-jsse-fips documents this is currently not supported in native, so a bit of a good news it does not break any user expectations, but I'll give it another try, as soon as possible, will try again @zakkak 's suggestions at #14139 etc |
makes sense, I've since found out we only have |
@zakkak I've had a look today, with
where
and then during the native build:
I've tried 3 things:
in both cases having a similar message, here is the one when the substitution is used:
Please have a quick look next week if you can, keeping fingers crossed it will be an easy one for you :-) Thanks |
That only works for static fields that are initialized at class initialization, it doesn't help for object instantiation.
Not sure why you are getting this error, but it again wouldn't solve the issue since it would essentially store a
Building with
However, passing We need to understand why GraalVM seems to ignore these instructions in this particular case. |
Thanks @zakkak for analyzing it, indeed, I guessed afterwards re that the static init vs the instance one, though not sure I follow the comment about resetting fields since we have at least one substitution reset for another field, may be it is a different situation. |
The substitution will only work for object instances in the image heap (i.e. for classes initialized and instantiated at build-time). We can have a short call if you want to discuss about it.
I tried that and it still didn't work. Feel free to give it a go if you want though, as I might have missed something. |
Thanks Foivos, @zakkak, I'd like to believe I understand now :-), but will be glad to have a call at some point to clarify a few things about native images, look forward to it. As far as those 2 files are concerned, I'm not sure what I can do beyond all the tries you have done, I've searched around and I've only found So it looks like it is re-initialized correctly in some cases, I wonder if it is may be a Java 11 vs Java 17 thing |
The Removing it allows Quarkus to initialize it at runtime and also removes the traces showing What's interesting though is that in the To my understanding this happens because the two tests register the BCFIPS provider at a different index:
Relevant code segments: Lines 13 to 36 in c069f5f
(In the failing fips-jsse case Similarly in Lines 351 to 381 in c069f5f
@sberyozkin is there a reason you don't always use the same strategy? Which one is correct? |
Hi @zakkak Sorry, I've totally missed your comments, with all code and doc PRs, thanks for testing it further. As far as I recall for BC JSSE to be activated correctly, it had to be inserted before Sun JSSE, and since it depends on FIPS, FIPS provider had to come before it. This is the only combination which let me get BC JSSE+FIPS working correctly, if it is the only possible option, I'm not 100% sure, if you'd like please double check and insert the BC JSSE/FIPS at the end (using the code I used for adding BC FIPS only), I believe it will fail though. Thanks |
May be Sun JSSE provider has to be removed instead and BC JSSE/FIPS then added at the end. I might've tried it too but can't quite recall |
@sberyozkin I was able to get the native image tests to pass with the following patch (places BCFIPS first and then BCJSSE without touching the rest of the providers): 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 d781085f835..5edc5572cb1 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
@@ -348,8 +348,6 @@ public void write(String s, byte[] bytes) {
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));
@@ -360,13 +358,11 @@ public void write(String s, byte[] bytes) {
overallCatch.load(true), bcFipsProvider);
overallCatch.invokeStaticMethod(
- MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class,
- int.class),
- bcFipsProvider, overallCatch.load(sunIndex));
+ MethodDescriptor.ofMethod(Security.class, "addProvider", int.class, Provider.class),
+ bcFipsProvider);
overallCatch.invokeStaticMethod(
- MethodDescriptor.ofMethod(Security.class, "insertProviderAt", int.class, Provider.class,
- int.class),
- bcJsseProvider, overallCatch.load(sunIndex + 1));
+ MethodDescriptor.ofMethod(Security.class, "addProvider", int.class, Provider.class),
+ bcJsseProvider);
}
} else {
// BC or BCFIPS
However, when applying the equivalent for JVM-mode, i.e.: diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderRecorder.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderRecorder.java
index 01eb8e2c6bc..4377ca6d387 100644
--- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderRecorder.java
+++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderRecorder.java
@@ -28,10 +28,9 @@ public void addBouncyCastleJsseProvider() {
public void addBouncyCastleFipsJsseProvider() {
Provider bc = loadProvider(SecurityProviderUtils.BOUNCYCASTLE_FIPS_PROVIDER_CLASS_NAME);
- int sunIndex = findProviderIndex(SecurityProviderUtils.SUN_PROVIDER_NAME);
- insertProvider(bc, sunIndex);
+ addProvider(bc);
Provider bcJsse = loadProviderWithParams(SecurityProviderUtils.BOUNCYCASTLE_JSSE_PROVIDER_CLASS_NAME,
new Class[] { boolean.class, Provider.class }, new Object[] { true, bc });
- insertProvider(bcJsse, sunIndex + 1);
+ addProvider(bcJsse);
}
}
diff --git a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderUtils.java b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderUtils.java
index d34a25235ae..503e63639c7 100644
--- a/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderUtils.java
+++ b/extensions/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityProviderUtils.java
@@ -8,9 +8,7 @@
import io.quarkus.runtime.configuration.ConfigurationException;
public final class SecurityProviderUtils {
- public static final String SUN_PROVIDER_NAME = "SUN";
public static final String SUN_JSSE_PROVIDER_NAME = "SunJSSE";
- public static final String SUN_JSSE_PROVIDER_CLASS_NAME = "com.sun.net.ssl.internal.ssl.Provider";
public static final String BOUNCYCASTLE_PROVIDER_NAME = "BC";
public static final String BOUNCYCASTLE_JSSE_PROVIDER_NAME = BOUNCYCASTLE_PROVIDER_NAME + "JSSE";
public static final String BOUNCYCASTLE_FIPS_PROVIDER_NAME = "BCFIPS"; JVM-mode fails with:
Any ideas? |
I also tried removing SUNJSSE but that didn't work as you predicted although according to https://downloads.bouncycastle.org/fips-java/BC-FJA-(D)TLSUserGuide-1.0.3.pdf the minimum config is:
|
Hey @zakkak Apologies I've missed it, thanks for your time. May be this is the solution, do it only in the native mode. I'll try to get back to this issue once I'll clear some backlog, the fact you have managed to get the tests passing in native mode is a good news indeed, thanks |
I am removing JSSE from the PR title as issue seems to be in BCFIPS provider and BTW still there. |
@sberyozkin this is a kind reminder. |
Describe the bug
I have app with
org.bouncycastle:bc-fips
andorg.bouncycastle:bctls-fips
dependencies that I build to native executable. After #37354 build fails with exeception.Expected behavior
Native executable is built.
Actual behavior
Native executable build fails:
How to Reproduce?
Steps to reproduce:
git clone [email protected]:quarkus-qe/quarkus-test-suite.git
cd quarkus-test-suite/security/bouncycastle-fips/bcFipsJsse
mvn clean verify -Dnative
Output of
uname -a
orver
Fedora 38
Output of
java -version
17
MANDREL 23.1.1.0 JDK 21.0.1+12-LTS
Quarkus version or git rev
999-SNAPSHOT
Build tool (ie. output of
mvnw --version
orgradlew --version
)Apache Maven 3.9.3
Additional information
No response
The text was updated successfully, but these errors were encountered: