You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by David Blevins <da...@gmail.com> on 2022/08/30 22:10:14 UTC

Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution

All,

I'm digging through the test failures in the MP JWT TCK and one of them is a test verifying support for downloading the keys for verifying JWTs via an http call.

The trick is the test is deploying an application that makes an HTTP request to itself to get the public key and expects that to work.  Since we validate the configuration before the application is started, this does not work -- the application can't call itself to make an HTTP request because it hasn't been deployed yet.  Chicken and egg.

I personally think it's not valid and that we should be allowed to eagerly verify the configuration before we start the app, but I can see an argument to having code that can re-check the key server for updates, which is something you'd need to do at runtime.  So, as long as we issue a big warning that the http server cannot be reached and authentications will fail until it becomes available, I can be convinced to implement something that checks for keys at runtime.

This would require reworking our code quite a bit.  Currently, we have a JWTAuthConfiguration object which we allow users to supply via CDI.  If one isn't supplied we'll use their microprofile-config.properties.  We would not easily be able to implement runtime/dynamic resolution of keys without impacting JWTAuthConfiguration, which is user facing.

Personally, I'd be happy to eliminate support for JWTAuthConfiguration as a stable, user-facing, API.

We only created JWTAuthConfiguration because MP JWT 1.0 didn't have a standard configuration mechanism yet.  Now it does and it's getting more robust every specification version.  Having JWTAuthConfiguration exposed to users basically exposes our guts and inhibits our ability to change them.  It's really better to have users relying on microprofile-config.properties.

What do you think about:

 - Supporting runtime/dynamic key resolution
 - Eliminating JWTAuthConfiguration in TomEE 9.0 before it goes final



-David


Re: MP JWT TCK passing & HTTP Key URLs (was Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution)

Posted by "Zowalla, Richard" <ri...@hs-heilbronn.de>.
Thanks David for the work!

The CI looks good (compared to main): 
https://ci-builds.apache.org/job/Tomee/job/TOMEE-4050/

Gruß
Richard

Am Montag, dem 03.10.2022 um 21:47 +0200 schrieb Jean-Louis Monteiro:
> That sounds great.
> Good feature in addition to the bean validation support for claims.
> 
> Thanks David for the hard work on this.
> 
> Only missing part is OpenTracaing as far as I know.
> --
> Jean-Louis Monteiro
> http://twitter.com/jlouismonteiro
> http://www.tomitribe.com
> 
> 
> On Mon, Oct 3, 2022 at 6:58 PM David Blevins <david.blevins@gmail.com
> >
> wrote:
> 
> > Hey All,
> > 
> > Provided we can get a good CI build on this PR, we're done with MP
> > JWT and
> > have some new functionality I'm pretty proud of and had a great
> > time
> > working on.
> > 
> >  - https://github.com/apache/tomee/pull/926
> > 
> > The new functionality in a nutshell is the ability to dynamically
> > resolve
> > and rotate JWT validation keys at runtime.  It is enabled by
> > default for
> > HTTP key locations, but can be enabled for any key location.
> > 
> > There's a full set of itests that verify our error handling and
> > logging
> > for all the various failure/recovery scenarios I could think
> > of.  Here's a
> > good example:
> > 
> >  -
> > https://github.com/apache/tomee/blob/TOMEE-4050/itests/microprofile-jwt-itests/src/test/java/org/apache/tomee/microprofile/jwt/itest/keys/http/HttpKeyRotationHttp500Test.java
> > 
> > I also wrote up a doc for MP JWT and our custom config properties:
> > 
> >  -
> > https://github.com/apache/tomee/blob/TOMEE-4050/docs/microprofile/jwt.adoc
> > 
> > If you have a fleet of servers, don't want to hardcode the keys in
> > the app
> > and need requests to work reliably even when errors occur in key
> > rotation,
> > this is your feature.
> > 
> > 
> > -David
> > 
> > 

Re: MP JWT TCK passing & HTTP Key URLs (was Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution)

Posted by Jean-Louis Monteiro <jl...@tomitribe.com>.
That sounds great.
Good feature in addition to the bean validation support for claims.

Thanks David for the hard work on this.

Only missing part is OpenTracaing as far as I know.
--
Jean-Louis Monteiro
http://twitter.com/jlouismonteiro
http://www.tomitribe.com


On Mon, Oct 3, 2022 at 6:58 PM David Blevins <da...@gmail.com>
wrote:

> Hey All,
>
> Provided we can get a good CI build on this PR, we're done with MP JWT and
> have some new functionality I'm pretty proud of and had a great time
> working on.
>
>  - https://github.com/apache/tomee/pull/926
>
> The new functionality in a nutshell is the ability to dynamically resolve
> and rotate JWT validation keys at runtime.  It is enabled by default for
> HTTP key locations, but can be enabled for any key location.
>
> There's a full set of itests that verify our error handling and logging
> for all the various failure/recovery scenarios I could think of.  Here's a
> good example:
>
>  -
> https://github.com/apache/tomee/blob/TOMEE-4050/itests/microprofile-jwt-itests/src/test/java/org/apache/tomee/microprofile/jwt/itest/keys/http/HttpKeyRotationHttp500Test.java
>
> I also wrote up a doc for MP JWT and our custom config properties:
>
>  -
> https://github.com/apache/tomee/blob/TOMEE-4050/docs/microprofile/jwt.adoc
>
> If you have a fleet of servers, don't want to hardcode the keys in the app
> and need requests to work reliably even when errors occur in key rotation,
> this is your feature.
>
>
> -David
>
>

MP JWT TCK passing & HTTP Key URLs (was Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution)

Posted by David Blevins <da...@gmail.com>.
Hey All,

Provided we can get a good CI build on this PR, we're done with MP JWT and have some new functionality I'm pretty proud of and had a great time working on.

 - https://github.com/apache/tomee/pull/926

The new functionality in a nutshell is the ability to dynamically resolve and rotate JWT validation keys at runtime.  It is enabled by default for HTTP key locations, but can be enabled for any key location.

There's a full set of itests that verify our error handling and logging for all the various failure/recovery scenarios I could think of.  Here's a good example:

 - https://github.com/apache/tomee/blob/TOMEE-4050/itests/microprofile-jwt-itests/src/test/java/org/apache/tomee/microprofile/jwt/itest/keys/http/HttpKeyRotationHttp500Test.java

I also wrote up a doc for MP JWT and our custom config properties:

 - https://github.com/apache/tomee/blob/TOMEE-4050/docs/microprofile/jwt.adoc

If you have a fleet of servers, don't want to hardcode the keys in the app and need requests to work reliably even when errors occur in key rotation, this is your feature.


-David


Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution

Posted by David Blevins <da...@gmail.com>.
> On Sep 9, 2022, at 8:29 PM, David Blevins <da...@gmail.com> wrote:
> 
>> On Aug 30, 2022, at 3:10 PM, David Blevins <da...@gmail.com> wrote:
>> 
>> I'm digging through the test failures in the MP JWT TCK and one of them is a test verifying support for downloading the keys for verifying JWTs via an http call.
>> 
>> The trick is the test is deploying an application that makes an HTTP request to itself to get the public key and expects that to work.  Since we validate the configuration before the application is started, this does not work -- the application can't call itself to make an HTTP request because it hasn't been deployed yet.  Chicken and egg.
> 
> Alrighty, we're down to just two failures in the MP JWT 2.0 TCK and both are due to the above.

Hacking away on this.  As it's taking me a while I figured I'd put up this draft PR so people can take a peek if they like:

 - https://github.com/apache/tomee/pull/926

The logic behind the design is that I'll have the code that needs to verify JWTs hold a java.util.function.Supplier that will supply the keys used for verification.  Behind that `java.util.function.Supplier` interface we can do all the fancy work of:
 
 - dynamically fetching keys from the HTTP server
 - handling retries if the server is down
 - blocking threads (up to a point) until we get a response
 - refreshing keys on some kind of schedule, without blocking threads

That fancy supplier logic is in a new class `CachedSupplier` and does all of the difficult threading in a way that's safe and optimal.  Or at least I hope -- I'm still writing the tests for it.

CachedSupplier is generic and doesn't have any actual HTTP or key parsing logic.  Instead you construct `CachedSupplier` by passing in another `java.util.function.Supplier` that does the HTTP call, or disk read, or pulls from anywhere we'd like to support.

Basically `CachedSupplier` is a wrapper around a `java.util.function.Supplier` that is presumably doing something very expensive and we don't want to execute that operation very often.  I threw it in the org.apache.openejb.util package as it's a cool tool and we could easily use it for other things in the future.


-David




Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution

Posted by David Blevins <da...@gmail.com>.
I tried out the MP-JWT 2.1 RC3 TCK today and there are just two more test failures and three new configuration flags:

 - `mp.jwt.decrypt.key.algorithm` property for supporting an RSA-OAEP-256 key management algorithm has been introduced. 
 - `mp.jwt.verify.token.age` property for restricting a token age has been introduced.
 - `mp.jwt.verify.clock.skew` property for configuring a leeway for the token expiry and age verification has been introduced.

My disagreement is with how `mp.jwt.decrypt.key.algorithm` has been defined.

The configuration flag allows the user to specify the exact algorithm they want supported for decrypting encrypted JWTs.  Having a flag users can set is great.  No disagreement there.

The disagreement is with the proposed handling of defaults for that setting.  The draft spec says there's a default value of RSA-OAEP and that we must reject requests that have the stronger RSA-OAEP-256 encryption.  Further that the default will change to RSA-OAEP-256 in MP JWT 3.0 and then we'll have to reject JWTs with RSA-OAEP by default.

The issues I see with this:

 - MP JWT 2.0 required RSA-OAEP support, but implementors could support more.  We support RSA-OAEP-256, RSA-OAEP-384
RSA-OAEP-512 and RSA-OAEP out of the box with no config.

 - MP JWT 2.1 RC3 basically says we're not allowed to support RSA-OAEP-256 encryption out of the box with no config.  Applications that may have worked successfully on MP JWT 2.0 leveraging RSA-OAEP-256, RSA-OAEP-384 or
RSA-OAEP-512 must now fail until the user adds the new flag.

 - MP JWT 3.0 is "planned" to change the default from RSA-OAEP to RSA-OAEP-256.  This means MP JWT 2.1 applications that use out of the box settings are again required to break on upgrade.

It's the continuous breaking of applications that I'm not fond of.

There should be a flag.  There should be a set of algorithms that must be standardly supported by an MP JWT impl.  However the default when users do not use the flag should allow vendors to support the stronger RSA-OAEP-256, RSA-OAEP-384, and RSA-OAEP-512.

Our implementation does this and ideally we can leave it that way.  I'll implement the `mp.jwt.decrypt.key.algorithm` configuration flag so that when set it will reject tokens that use a different algorithm, but by default will allow the required RSA-OAEP plus the much better RSA-OAEP-256, RSA-OAEP-384, and RSA-OAEP-512 (as it does now).

I'll be creating a PR for the spec and TCK and bring it up for discussion over on the MP JWT side of things.


-David



> On Sep 9, 2022, at 11:54 PM, Richard Zowalla <rz...@apache.org> wrote:
> 
> Hi David,
> 
> thanks for the update. 
> 
> I think it is a good idea to look at the (unreleased) JWT 2.1 while
> your head is still "in the zone". Mybe you find some corner/edge/we-
> dont-like things in the next spec and we can change before it happens.
> 
> Regarding your original discussion / question:
> 
> I think, that we can eliminate support for JWTAuthConfiguration. We
> already switched to smallrye and did some major version upgrades of
> microprofile. That will - most certainly - break user applications, so
> their code needs to be touched anyway. If we don't do it now, the next
> opportunity might be TomEE 10 ;)
> 
> Gruß
> Richard
> 
> Am Freitag, dem 09.09.2022 um 20:29 -0700 schrieb David Blevins:
>>> On Aug 30, 2022, at 3:10 PM, David Blevins <david.blevins@gmail.com
>>>> wrote:
>>> 
>>> I'm digging through the test failures in the MP JWT TCK and one of
>>> them is a test verifying support for downloading the keys for
>>> verifying JWTs via an http call.
>>> 
>>> The trick is the test is deploying an application that makes an
>>> HTTP request to itself to get the public key and expects that to
>>> work.  Since we validate the configuration before the application
>>> is started, this does not work -- the application can't call itself
>>> to make an HTTP request because it hasn't been deployed
>>> yet.  Chicken and egg.
>> 
>> Alrighty, we're down to just two failures in the MP JWT 2.0 TCK and
>> both are due to the above.
>> 
>> As mentioned, fixing this is going to require ripping the code up a
>> bit and backwards incompatible changes to JWTAuthConfiguration which
>> was user-facing in TomEE 8.0.
>> 
>> Before I do that I'm tempted to take a look at implementing the
>> unreleased JWT 2.1 requirements & TCK tests while my head is in this
>> space.  I know there's at least one change that I'm not a fan of and
>> there may be others.  I'd prefer to get that feedback in asap before
>> that spec goes final, if possible.
>> 
>> Anyway, things are looking good!
>> 
>> 
>> -David
>> 
>> 
>> 
> 


Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution

Posted by Richard Zowalla <rz...@apache.org>.
Hi David,

thanks for the update. 

I think it is a good idea to look at the (unreleased) JWT 2.1 while
your head is still "in the zone". Mybe you find some corner/edge/we-
dont-like things in the next spec and we can change before it happens.

Regarding your original discussion / question:

I think, that we can eliminate support for JWTAuthConfiguration. We
already switched to smallrye and did some major version upgrades of
microprofile. That will - most certainly - break user applications, so
their code needs to be touched anyway. If we don't do it now, the next
opportunity might be TomEE 10 ;)

Gruß
Richard

Am Freitag, dem 09.09.2022 um 20:29 -0700 schrieb David Blevins:
> > On Aug 30, 2022, at 3:10 PM, David Blevins <david.blevins@gmail.com
> > > wrote:
> > 
> > I'm digging through the test failures in the MP JWT TCK and one of
> > them is a test verifying support for downloading the keys for
> > verifying JWTs via an http call.
> > 
> > The trick is the test is deploying an application that makes an
> > HTTP request to itself to get the public key and expects that to
> > work.  Since we validate the configuration before the application
> > is started, this does not work -- the application can't call itself
> > to make an HTTP request because it hasn't been deployed
> > yet.  Chicken and egg.
> 
> Alrighty, we're down to just two failures in the MP JWT 2.0 TCK and
> both are due to the above.
> 
> As mentioned, fixing this is going to require ripping the code up a
> bit and backwards incompatible changes to JWTAuthConfiguration which
> was user-facing in TomEE 8.0.
> 
> Before I do that I'm tempted to take a look at implementing the
> unreleased JWT 2.1 requirements & TCK tests while my head is in this
> space.  I know there's at least one change that I'm not a fan of and
> there may be others.  I'd prefer to get that feedback in asap before
> that spec goes final, if possible.
> 
> Anyway, things are looking good!
> 
> 
> -David
> 
> 
> 


Re: Discuss changes to MP JWT support / JWTAuthConfiguration / Key resolution

Posted by David Blevins <da...@gmail.com>.
> On Aug 30, 2022, at 3:10 PM, David Blevins <da...@gmail.com> wrote:
> 
> I'm digging through the test failures in the MP JWT TCK and one of them is a test verifying support for downloading the keys for verifying JWTs via an http call.
> 
> The trick is the test is deploying an application that makes an HTTP request to itself to get the public key and expects that to work.  Since we validate the configuration before the application is started, this does not work -- the application can't call itself to make an HTTP request because it hasn't been deployed yet.  Chicken and egg.

Alrighty, we're down to just two failures in the MP JWT 2.0 TCK and both are due to the above.

As mentioned, fixing this is going to require ripping the code up a bit and backwards incompatible changes to JWTAuthConfiguration which was user-facing in TomEE 8.0.

Before I do that I'm tempted to take a look at implementing the unreleased JWT 2.1 requirements & TCK tests while my head is in this space.  I know there's at least one change that I'm not a fan of and there may be others.  I'd prefer to get that feedback in asap before that spec goes final, if possible.

Anyway, things are looking good!


-David