[GR-47922] Enforce -H:±UnlockExperimentalVMOptions

[fniephaus]

-H:±UnlockExperimentalVMOptions was introduced in GraalVM for JDK 21 (see #7105). This is a follow up issue to track outstanding tasks such as its enforcement, bugs and improvements, or API option changes.

Main task

• Turn unlocking warning into a failure. This has the highest priority.

Bug fixes and improvements

• -H:Log and other options are not listed in the experimental output, which only handles HostedOptionKey but not other OptionKeys (see Add stable option for option -H:Log #7357).
  (addressed by [GR-47922] Improve list of experimental options. #7720)
• Provide migration messages across options (e.g., -H:IncludeResources can be replaced with a resource-config.json file , see Add stable option for -H:IncludeResources and -H:ExcludeResources #7354 (comment)). Currently, migration hints are only generated for hosted options that are annotated with @APIOption. Another example is -H:Path which can be replaced with -o, which in turn belongs to -H:Name.
  (addressed by [GR-47922] Improve list of experimental options. #7720)
• Only print warning for existing hosted options (e.g., native-image -H:+Nonono -m jdk.httpserver should not show the warning) won't fix, not considered a bug

API changes

• Introduce new API options for debugging features. In [GR-48956] Add stable option for debug options OmitInlinedMethodDebugLineInfo, TrackNodeSourcePosition, DeleteLocalSymbols and SourceLevelDebug #7356, we discussed -gperf and -gfull.

[zakkak]

Feature request

Add stable option for -H:AdditionalSecurityProviders

Is your feature request related to a problem? Please describe.

Starting with 23.1 using -H:AdditionalSecurityProviders results in a warning like the following:

Warning: The option '-H:AdditionalSecurityProviders=org.wildfly.security.password.WildFlyElytronPasswordProvider' is experimental and must be enabled via '-H:+UnlockExperimentalVMOptions' in the future.
Warning: Please re-evaluate whether any experimental option is required, and either remove or unlock it. The build output lists all active experimental options, including where they come from and possible alternatives. If you think an experimental option should be considered as stable, please file an issue.

Four core Quarkus Extensions (security, elytron-security-jdbc, kafka-client, infinispan-client) rely on this option to define additional security providers.

Describe the solution you'd like.

Provide a stable way to define additional security providers, either making this option stable or adding a new API option.

Describe who do you think will benefit the most.
GraalVM users and developers of libraries and frameworks which depend on GraalVM

Describe alternatives you've considered.
Wrap it in -H:+UnlockExperimentalVMOptions and -H:-UnlockExperimentalVMOptions.

[cstancu]

@zakkak I'm actually curious why do you need to use -H:AdditionalSecurityProviders? Is it because the WildFlyElytronPasswordProvider relies on the SHA512_256MessageDigest service which is not JCA compliant, i.e., it doesn't declare a getInstance() method? Why not make SHA512_256MessageDigest JCA compliant instead and use -H:AdditionalSecurityServiceTypes to declare the custom service type and enable automatic processing by SecurityServicesFeature.registerServiceReachabilityHandlers(). Or do you completely disable -H:-EnableSecurityServicesFeature?

[zakkak]

@sberyozkin can you please have a look at ^^?

Or do you completely disable -H:-EnableSecurityServicesFeature?

No we don't disable this.

[zakkak]

@sberyozkin can you please have a look at ^^?

@sberyozkin this is a kind reminder.

[zakkak]

@sberyozkin can you please have a look at ^^?

@sberyozkin this is a kind reminder.

@sberyozkin it looks like this fell through the cracks, can you please have a look?

[sberyozkin]

Apologies for totally missing this issue. Let me CC @fjuma and @darranl - can you please check #7370 (comment), is it possible to add .getInstance() to SHA512_256MessageDigest.java ? Thanks

[darranl]

@sberyozkin Can I please double check what is being asked for here?

I am just looking at JCA documentation to clarify the requirements and I am currently in this section:

https://docs.oracle.com/en/java/javase/11/security/howtoimplaprovider.html#GUID-CB446B7A-CEA2-4F4A-A4AF-4D492CB58733

Is this about adding a .getInstance() to the SHA512_256MessageDigest.java or is this about adding the newInstance(Object) method where we register the Provider.Service ? I assume this also applies to all Security Provider registration and not just this example?

[sberyozkin]

@darranl Thanks for checking it. Now that you have asked, I have to admit, I'm not sure.

I've reread the comment from @zakkak where he indeed mentions org.wildfly.security.password.WildFlyElytronPasswordProvider, and I believe @cstancu assumed the reason Quarkus had to add it to native image with an -H:AdditionalSecurityProviders option to workaround the fact that the Elytron SHA512_256MessageDigest misses getInstance().

@zakkak has also mentioned other Quarkus extensions which use -H:AdditionalSecurityProviders, quarkus-security, quarkus-security-jdbc, quarkus-infinispan-client, quarkus-kafka-client.

quarkus-security-jdbc also adds org.wildfly.security.password.WildFlyElytronPasswordProvider with -H:AdditionalSecurityProviders but quarkus-infinispan-client and quarkus-kafka-client add com.sun.security.sasl.Provider (quarkus-kafka-client adds a couple of other providers).

I've tested that quarkus-security-jdbc integration tests are passing in native mode without adding org.wildfly.security.password.WildFlyElytronPasswordProvider with -H:AdditionalSecurityProviders.

However with quarkus-infinispan-client, avoiding adding com.sun.security.sasl.Provider with -H:AdditionalSecurityProviders leads to the following error:

2024-03-10 21:16:46,416 WARN  [io.net.cha.ChannelInitializer] (HotRod-client-async-pool-3-3) Failed to initialize a channel. Closing: [id: 0x5477bf1b]: java.lang.IllegalStateException: SaslClientFactory implementation not found
	at org.infinispan.client.hotrod.impl.transport.netty.ChannelInitializer.g

Does it mean -H:AdditionalSecurityProviders is really needed ?

@cstancu What are you thoughts about it ?