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?