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

Quarkus-security - there is no way of selecting providers from extensions #18615

Closed
JiriOndrusek opened this issue Jul 12, 2021 · 11 comments · Fixed by #18652
Closed

Quarkus-security - there is no way of selecting providers from extensions #18615

JiriOndrusek opened this issue Jul 12, 2021 · 11 comments · Fixed by #18652
Labels
area/security kind/enhancement New feature or request
Milestone

Comments

@JiriOndrusek
Copy link
Contributor

Description

I'm working on apache/camel-quarkus#2005. I'm trying to use quarkus-security from camel-quarkus extensions.

I'm facing a problem, when extensions knows, that bouncycastle provider has to be used. This could be achieved by config property quarkus.security.security-providers=BC. Unfortunatelly I haven't found a way how to define it in an extension at build time. The only possible solution (which I see) is to force users, to define this property in their apps.

Implementation ideas

I can imagine several ways of providing this new configuration option.
For example:

  • Existing build item BouncyCastleProviderBuildItem could be changed to allow multiple produce points, which would allow to configure it in extension.
  • New build item could be created, which would allow definition of provider.

In both cases this new configuration could be validated against property.

@JiriOndrusek JiriOndrusek added the kind/enhancement New feature or request label Jul 12, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 12, 2021

/cc @sberyozkin

@JiriOndrusek
Copy link
Contributor Author

I can try to provide a solution

@sberyozkin
Copy link
Member

@JiriOndrusek Hi, can the single produce point do ? If the users are not expected to configure it manually then a custom extension producing it should work ? Sorry if I'm missing something

@JiriOndrusek
Copy link
Contributor Author

@sberyozkin I probably don't understand you comment. Here is my case, where I can not find a solution with the current code (but I might be missing something):

  • I'm writing a custom extension, which uses bouncycastle
  • I need to set quarkus.security.security-providers=BC to achieve this
  • I don't want to forrce user to set quarkus.security.security-providers=BC
    -> my problem is that I can not find a way of setting this property in my extension, because I can not use RunTimeConfigurationDefaultBuildItem, I can not set quarkus.security.security-providers=BC into extension application.properties (it does not have any effect) and I can not create BouncyCastleProviderBuildItem in my buildStep, because it is already produced by SecureProcessor

I'm suggesting (if there is no other way of achieving this) to modify current BouncyCastleProviderBuildItem or create new build item so it allows provider configuration in custom extension's buildStep.

@JiriOndrusek
Copy link
Contributor Author

@sberyozkin We can discuss also an option of creating a separate bouncycastle extension (or extending the current one) - based on camel-quarkus's one. I can work on it.

@sberyozkin
Copy link
Member

sberyozkin commented Jul 12, 2021

Hi @JiriOndrusek - the reason no dedicated extension exists yet is that at a time the code was added there was no clear idea how BC FIPS could be supported at the product level - AFAIK - this is still an open question - so for now lets try to work around the existing code.

So, what I meant, can your custom extension do:

@BuildStep
    void produceBouncyCastleProvider(BuildProducer<BouncyCastleProviderBuildItem> bouncyCastleProvider) {
        bouncyCastleProvider.produce(new BouncyCastleProviderBuildItem());
    }

? I think it should not be a multiple point producer - i.e either the user configures it manually or the custom extension like the one you are working on does it indirectly - in this case the code producing the same item in quarkus-security will not be triggered. can you try it please ?

@sberyozkin
Copy link
Member

Even if both the custom extension does the above code and the user sets the property at the same time it should still work; if it is a multiple producer point then we'd have to accept a list of these items which given that there is only one BC provider instance would not be correct; so yes, please try the above code - hope it will be fine

@JiriOndrusek
Copy link
Contributor Author

Hi @sberyozkin I originally used the same approach as you are suggesting. I tried it again (now) with

@BuildStep
    void produceBouncyCastleProvider(BuildProducer<BouncyCastleProviderBuildItem> bouncyCastleProvider) {
        bouncyCastleProvider.produce(new BouncyCastleProviderBuildItem());
    }

The result is an error:

Caused by: java.lang.RuntimeException: io.quarkus.builder.ChainBuildException: Multiple producers of item class io.quarkus.security.deployment.BouncyCastleProviderBuildItem (io.quarkus.security.deployment.SecurityProcessor#produceJcaSecurityProviders)

...
Caused by: java.lang.Throwable: This is the location of the conflicting producer (org.apache.camel.quarkus.component.crypto.deployment.CryptoProcessor#produceBouncyCastleProvider)

I think that problem is caused by the fact, that build item extends SimpleBuildItem and SecurityProcessor accepts Optional<BouncyCastleProviderBuildItem> bouncyCastleProvider and nt list.

@sberyozkin
Copy link
Member

sberyozkin commented Jul 13, 2021

Hi @JiriOndrusek, it can't be a list since there could be only a single BC instance - this build item also has a fips property so accepting a list would make it impossible to decide if FIPS must be enforced or not when one item has it on and another one - off.

Though, can you please try to make it a multiple producer item as you suggested but also change Optonal<BouncyCastleProviderBuildItem> to Set<BouncyCastleProviderBuildItem> (and the same for BouncyCastleJsseProviderBuildItem), with hashCode/equals added to both items ? This will ensure the set contains a single item even if both the custom extension and the user configure BC or BCJSSE etc

Then we'd throw an exception if this set contains more than one item registration (say, BC and BCFIPS) ("Only a single Bouncy Castle registration can be provided").

If Set is not supported then we can indeed use List but verify by converting to set...

Can you try it please

@JiriOndrusek
Copy link
Contributor Author

JiriOndrusek commented Jul 13, 2021

Edited: Sorry , the same thing is a part of your proposal, please ignore it.

@sberyozkin Your proposal makes sense. I will try it.

I see a small modification:

  • As you said, let buildItem be a multi-one, then security processor will accept a List. Build items should have hashCode / equals reworked as you said. But in case that there are 2 or more items (which means that several items differ - e.g. in fips flag or something),some kind of validation error with explanation will be thrown - if there are 2 or more different configurations for the same provider. What do you think? It's just a suggestion, which will avoid Set and defines what to do if there are e.g. BC fips = true and BC fips = false defined by 2 different extensions.

@sberyozkin
Copy link
Member

sberyozkin commented Jul 13, 2021

@JiriOndrusek Np at all, great we agree about the potential problem of having it as List. As far as I understand we agree with how to address it as well, let me know please if I misunderstood

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants