Supporting Multiple Keys in Asymmetric Algos

[drusellers]

Ok, I am building up my ASP.Net API Application to use the RS1024 algorithm.

services.AddAuthentication(JwtAuthenticationDefaults.AuthenticationScheme)
    .AddJwt(options =>
    {
        options.VerifySignature = true;
    });
services.AddJwtEncoder();
X509Certificate2 cert = LoadFromSecureStore();
services.AddSingleton<IAlgorithmFactory>(new DelegateAlgorithmFactory(new RS1024Algorithm(cert)));

This is working, I can generate and consume the JWTs.

A year goes by, and now I want to rotate those keys. Reading through JWT it looks like I can use the kid header to give the keys id's which can be used for looking up the right cert. However I'm not sure how I would ever get access to the JWT header (or the kid) in order to select the right X509. I'm probably missing something pretty obvious.

The DelegateFactory and others do take a Func<IJwtAlgorithm> but the kid isn't passed down.

I see WithSecret is used with symmetric algo's - this makes me think I might be missing something about asymmetric algo's. 🤔

A WithKeys would be helpful and could be used in the AddJwt call too.

I'm sure I'm missing something obvious, so any help would be appreciated. Also, if I've asked in the wrong forum, please let me know and I can move this conversation there.

[abatishchev]

hi! sorry for the delay in the response! my main question would be if you're encoding or decoding? just to make sure, but I guess decoding, otherwise it doesn't make too much sense. Here's what's supported today:

jwt/src/JWT/JwtDecoder.cs

Lines 235 to 236 in 7c7bfe3

	var header = DecodeHeader<JwtHeader>(jwt);
	var algorithm = _algFactory.Create(JwtDecoderContext.Create(header, decodedPayload, jwt));

and then

jwt/src/JWT/JwtDecoder.cs

Lines 76 to 78 in 7c7bfe3

	var header = new JwtParts(token).Header;
	var decoded = _urlEncoder.Decode(header);
	return GetString(decoded);

and finally

jwt/src/JWT/Algorithms/JwtAlgorithmFactory.cs

Lines 11 to 13 in 06c7556

	var algorithm = context?.Header?.Algorithm ?? throw new ArgumentNullException(nameof(context));
	var algorithmName = (JwtAlgorithmName)Enum.Parse(typeof(JwtAlgorithmName), algorithm);
	return Create(algorithmName);

What means the decoder automatically will create an algorithm based on the alg in the header.

[abatishchev]

An issue is though that the DI extensions project doesn't have a great API so doesn't support well registering the JWT classes for encoding and decoding in the same time. I tried to overcome it in #378.

What's the version of the library you're using?

[abatishchev]

Wait! Your question isn't about choosing the algorithm but the key based on the kid in the header.

What if you provide a custom factory:

public sealed class KeyedAlgorithmFactory : JwtAlgorithmFactory
{
    private readonly Func<string, X509Certificate2> _certSelector;

    public KeyedAlgorithmFactory(Func<string, X509Certificate2> certSelector)
    {
        _certSelector = certSelector;
    }

    public override IJwtAlgorithm Create(JwtDecoderContext context)
    {
        var kid = context?.Header?.KeyId;
        var cert = _certSelector(kid);
        return new RS256Algorithm(cert); // or somehow else to choose the right algo
    }
}

or something more generic:

public sealed class KeyedRSAlgorithmFactory : RSAlgorithmFactory // note the change in the base class
{
    private readonly Func<string, X509Certificate2> _certSelector;

    public KeyedRSAlgorithmFactory(Func<string, X509Certificate2> certSelector)
    {
        _certSelector = certSelector;
    }

    public override IJwtAlgorithm Create(JwtDecoderContext context)
    {
        var kid = context?.Header?.KeyId;
        var cert = _certSelector(kid);

        var algorithm = context?.Header?.Algorithm;
        return base.CreateAlgorithm(algorithm, cert); // method doesn't exist today, the base class needs refactoring
    }
}

If any of these works - then we can ship it with the library. Help to come up with a better name though.

[abatishchev]

@drusellers ping. Did you try any of the above? Does it work, does it not?

[drusellers]

@abatishchev sorry I switched tasks, but reading the code this looks like exactly what I need. I'll implement it later today and put some thoughts towards naming - config - etc.

[drusellers]

Ok, so that was great. It's looking like I'll be working with something like

namespace Authentication.Jwt;

using System.Security.Cryptography.X509Certificates;
using JWT;
using JWT.Algorithms;


public class KeyedRSAlgorithmFactory : IAlgorithmFactory
{
    readonly Func<string, X509Certificate2?> _certSelector;

    public KeyedRSAlgorithmFactory(Func<string, X509Certificate2?> certSelector)
    {
        _certSelector = certSelector;
    }

    public IJwtAlgorithm Create(JwtDecoderContext context)
    {
        var key = context?.Header?.KeyId;
        if (key == null)
            throw new InvalidOperationException("No KEY provided in the 'kid' header");
        
        var cert = _certSelector(key);

        if (cert == null)
            throw new InvalidOperationException($"No certificate found for key '{key}'");

        var algorithm = context?.Header?.Algorithm;
        if (algorithm == null)
            throw new InvalidOperationException("No algorithm was found in the 'alg' header");
        
        var algorithmName = (JwtAlgorithmName)Enum.Parse(typeof(JwtAlgorithmName), algorithm);

        return algorithmName switch
        {
            JwtAlgorithmName.RS256 => new RS256Algorithm(cert),
            JwtAlgorithmName.RS384 => new RS384Algorithm(cert),
            JwtAlgorithmName.RS512 => new RS512Algorithm(cert),
            JwtAlgorithmName.RS1024 => new RS1024Algorithm(cert),
            JwtAlgorithmName.RS2048 => new RS2048Algorithm(cert),
            JwtAlgorithmName.RS4096 => new RS4096Algorithm(cert),
            
            _ => throw new NotSupportedException($"{algorithm} not supported by {nameof(KeyedRSAlgorithmFactory)}")
        };
    }
}

Then I'm planning to build a registration that looks like this:

services.AddSingleton<ICertificateStore, CertificateStore>();
services.AddSingleton<IAlgorithmFactory>(provider =>
{
    var store = provider.GetRequiredService<ICertificateStore>();
    return new KeyedRSAlgorithmFactory(store.GetCertificate);
});

[abatishchev]

Looks promising! Few suggestions:

1. You can use x ?? throw new Exception(...); to shorten the checks
2. And inherit the factory which will accept the store in its ctor and pass the method on it to its base class. This way you can shorten and simplify the DI registration code, e.g.:

class CertificateStoreRSAlgorithmFactory : KeyedRSAlgorithmFactory
{
    public CertificateStoreRSAlgorithmFactory(ICertificateStore store) : base(store.GeCertificate)
    {}
}

[drusellers]

1. Ah, good point on the ?? I'll add those in.
2. Good point, I'll keep an eye on it. I may keep it separate to deal with the complexity of storing the keys in a different class / concern. But I like your direction. In the end, I may refactor my idea to use it.

I'll post back here once the code has some wear and tear on it, then I'll look to submit a PR back. :)

[abatishchev]

then I'll look to submit a PR back

Awesome, thank you!

[drusellers]

Ok, running into the null context issue again - for context it's this line of code - https://github.com/jwt-dotnet/jwt/blob/main/src/JWT/JwtEncoder.cs#L49

var store = new FileSystemCertificateStore();
var factory = new KeyedRSAlgorithmFactory(store.GetCertificate);
IJwtEncoder encoder = new JwtEncoder(factory, new JsonNetSerializer(), new JwtBase64UrlEncoder());
var extraHeaders = new Dictionary<string, object> 
{
  { "kid", "test-cert" }
};
var payload = new Dictionary<string, object> 
{
  { "uid", "bob" }
};

// key is only used for symmetric encryption
encoder.Encode(extraHeaders, payload, Array.Empty<byte>());

When I call Encode on the JwtEncoder it then does

// my factory needs a proper context
var algorithm = _algFactory.Create(null); // throws since its null

[abatishchev]

Sorry, just got back from a vacation.

Yes, the problem is that the algorithm factory that is used for decoding can't be used for encoding. Decoding has context (e.g. from the underlying HTTP request), encoding does not. Instead the latter should be driven by the configuration, simply put - the algorithm needs to be hard-coded.

Sorry for the troubles you're facing with the library. And appreciate your input and feedback, as you've uncovered a previously unused code path (hence purely designed).

In other words, I don't know how to make DI container friendly two factories (classes) that implement the same interface. An only (and rough) idea that comes to my mind is to have two new interfaces that would implement the current one. So each can be registered with a different implementation.

[drusellers]

@abatishchev absolutely no worries.

Yes, the problem is that the algorithm factory that is used for decoding can't be used for encoding. Decoding has context (e.g. from the underlying HTTP request), encoding does not. Instead the latter should be driven by the configuration, simply put - the algorithm needs to be hard-coded.

Oh, this just kinda sank in for me. Ok, now I'm tracking as to the why. :D

Sorry for the troubles you're facing with the library. And appreciate your input and feedback, as you've uncovered a previously unused code path (hence purely designed).

Thank you for your patience in explaining things, and for even providing the library. :D

In other words, I don't know how to make DI container friendly two factories (classes) that implement the same interface. An only (and rough) idea that comes to my mind is to have two new interfaces that would implement the current one. So each can be registered with a different implementation.

Now that I better understand the constraint, I'll see what my caffeine addled brain comes up with, and let you know. Otherwise, it might be that I use the Builder Model for the encoding. I'll at least share with you the solution that I came up with.

[abatishchev]

hey @drusellers, how this one going?

[drusellers]

Working on it today. Just getting all of my cert generation buttoned up and automated - and then I'll be testing my setup. I should have an update later today. :)

[drusellers]

Ok, now that I have everything working, and I think a bit better understanding here is what I ended up doing.

So the decoding process works great, and and all of my work is really about working through the encoding side.

• for decoding I have a KeyedRSAlgorithmFactory that looks up the key based on the kid in the headers. This is working well so far. I want to add some logging and just bake it a bit more and then I'm happy to share the code but in the end its pretty much what you recommended.

• for encoding I have a ConfiguredRSAlgorithmFactory that leans on the configuration to select the key and algorithm to use for signing things. Rather than use the IJwtEncoder interface, I introduced my own JwtGenerator that brings together the ConfiguredRSAlgorithmFactory, the JwtConfiguration class (which has the Algo and Key Id).

Thoughts

What is the purpose of the byte[] key in the interfaces? It's for symmetric encryption right? How do we make this work for asymmetric encryption now that there is a compiler warning encouraging people to select one.

What if this had an JwtEncoderContext similar to a JwtDecoderContext. The encoder context could store the extra headers, the key data, and other items important to encoding - like ALGO selection etc.

Looking at the encodingFactory.Create you take a JwtDecoderContext but I need to generate the Algo for use in Encoding as well. Maybe the AlgorithmFactory has two methods - Create(JwtDecodingContext cxt) and Create(JwtEncodingContext cxt). (just the idea not the actual method names).

[abatishchev]

Sorry for a slow response, working on two things in parallel (my primary job is cool but high-demanding) requires context switching and it's hard to me.

What is the purpose of the byte[] key in the interfaces? It's for symmetric encryption right?

Primarily yes. But also it works as public key for asymmetric algorithms but that part is a magic to me. Or maybe not, need to look into the code. But if you wake me up in the middle of the night, I won't be able to explain :)

How do we make this work for asymmetric encryption now that there is a compiler warning encouraging people to select one.

I think we should drop it from the interface. The current version is 10.0.0-betaX, i made some breaking changes recently so it'd be perfect timing to make more drastic changes.

I've declared HMAC algorithms obsolete but I don't want to remove them from the library altogether, as there might be (legacy) users still.

What if this had an JwtEncoderContext similar to a JwtDecoderContext. The encoder context could store the extra headers, the key data, and other items important to encoding - like Algo selection etc.

Yes!

Looking at the EncodingFactory.Create() you take a JwtDecoderContext but I need to generate the Algo for use in Encoding as well. Maybe the AlgorithmFactory has two methods - Create(JwtDecodingContext cxt) and Create(JwtEncodingContext cxt).

Yes! We might keep old Create(), as a workaround and for back-compatibility, maybe temporally until next version and/or while the API design is being polished.

[drusellers]

@abatishchev no worries - you have been very responsive and I'm very much appreciate your time. :)

I would be game to mark some the methods supporting symmetric encryption as Obsolete to further encourage the use of newer methods that better support asymmetry. I'm also happy to help contribute a section on key generation and how I'm working through that and supporting it. I def don't know all of the best practices around this, so I'm looking forward to learning from others. :D