uruk-project / Jwt

JSON Web Token implementation for .Net & .Net Core
MIT License
80 stars 13 forks source link

Read SignatureAlgorithm from X509Certificate if available #557

Closed watfordgnf closed 3 years ago

watfordgnf commented 3 years ago

We're using X509 certs to sign and verify JWTs, and we have come across an issue where we have to explicitly state the signature algorithm in our policy because Jwk.Alg is null.

I believe this could instead be read from the X509 cert and added to the JWK in FromX509Certificate:

X509Certificate2Collection collection = store.Certificates
                                             .Find(
                                                 X509FindType.FindByThumbprint,
                                                 thumbprint ?? throw new ArgumentNullException(nameof(thumbprint)));
if (collection.Count == 0)
{
    // ...
}

try
{
    Jwk jwk = Jwk.FromX509Certificate(collection[0], withPrivateKey);
    // jwk.Alg == null
    // collection[0].SignatureAlgorithm.FriendlyName == "sha256RSA"
    return jwk;
}
catch (Exception ex)
{
     // ...
}

Basically supporting RFC 3279 and family (https://tools.ietf.org/html/rfc3279#section-2.2).

watfordgnf commented 3 years ago

Playing around with this, the only ones I am able to infer are:

private static SignatureAlgorithm? TryReadSignatureAlgorithmFromCertificate(X509Certificate2 certificate)
{
    switch (certificate.SignatureAlgorithm.Value)
    {
        case Oids.RsaPkcs1Sha256:
            return SignatureAlgorithm.RsaSha256;
        case Oids.RsaPkcs1Sha384:
            return SignatureAlgorithm.RsaSha384;
        case Oids.RsaPkcs1Sha512:
            return SignatureAlgorithm.RsaSha512;
        case Oids.ECDsaWithSha256:
            return SignatureAlgorithm.EcdsaSha256;
        case Oids.ECDsaWithSha384:
            return SignatureAlgorithm.EcdsaSha384;
        case Oids.ECDsaWithSha512:
            return SignatureAlgorithm.EcdsaSha512;
        case Oids.RsaPss:
            // no means to easily retrieve this from X509Certificate2 as the Parameters of the
            // signature algorithm are not exposed.
        default:
            return null;
    }
}

Oids.cs from https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/System/Security/Cryptography/Oids.cs

watfordgnf commented 3 years ago

Here is the very rough experiment (against v1.9.2): https://github.com/watfordgnf/Jwt/pull/1/files

ycrumeyrolle commented 3 years ago

I am not really confident with the usage of the signatureAlgorithm for populating the SignatureAlgorithm of the JWK. According to https://tools.ietf.org/html/rfc5280#section-4.1.1.2:

The signatureAlgorithm field contains the identifier for the cryptographic algorithm used by the CA to sign this certificate.

I've played with some test vectors. For example the www.google.com certificate, the signature algorithm is sha256RSA, whereas the public key is a ECC (256 bits) with ECDSA_P256 as key parameters.

The signatureAlgorithm of the certificate is the algorithm used for signing the certificate, not the algorithm that can be used with the certificate key.

watfordgnf commented 3 years ago

Interesting, I had not tried altering the signature algorithm from what the certificate states.

I guess I should have included our motivation: Jwk's alg is private, and when I use FromCertificate I cannot stipulate which signing algorithm I want to enforce with the use of the key.

ycrumeyrolle commented 3 years ago

I'll check if there is a reason for keeping the algorithm as init-only, but I think it is just by laziness.

ycrumeyrolle commented 3 years ago

Two options:

watfordgnf commented 3 years ago

I would be happy with a new parameter on FromX509Certificate.

Maybe it helps to know what we're doing, we're using JWKS to assist in JWT certificate rotation:

  1. Our web services which write JWTs reference their signing certs via thumbprints
  2. The certs are read from the X509Store via Jwk.FromX509Certificate
  3. /.well-known/jwks.json endpoint serves up the signing cert public keys via new Jwks(certificates)
  4. Downstream services use new JwksKeyProvider("https://.../.well-known/jwks.json")
  5. SignatureValidationPolicy wants to know the signature algorithm and we'd like that to come from the JWKS URI a. We have a separate internal validation that asserts the signature algorithm is sufficient
ycrumeyrolle commented 3 years ago

What is the usage of the alg parameter of the JWK ? It should be only an indicator. The way to identifiy the correct JWK when validating the signature of the JWT, is mainly the kid parameter...

watfordgnf commented 3 years ago

With the following (test) JWKS via Jwk.FromX509Certificate:

{
  "keys": [
    {
      "kty": "RSA",
      "kid": "UWr-bIRC71YDpuIpfTEgBzhP0M737ifkyYnQN9Ki_Ao",
      "x5t": "TO6V4g9eyDe4BGBF41pPIkohlq4",
      "e": "AQAB",
      "n": "tjcCeJqSlmzL6cqc6dKyJVSIZsPTx8sFii5SfcaShpQdD5w48sEAcjfXwldv-pZVdcfbmLfqCDOcrZzd6JWFIcDNjU2VXvYX08B-2FgBDDqO8x7svO9u9MkEoJtPZlqzF2tg916zmWsKMogio3T78zA-5wutdSAvAjDoe9NVZP3ejdyz1LxU-hFWkw6F55pRvYrMmHqeFrZtiAbth_hmpTw_3CwGPnLlfFXlzD8lJO4rIYBpyF5FBR8-AbYIXUl_GsOKt4nqguZRFrtfjj3E6KYJaEFyrj0BpCozO6lgk5NUIxG6dw9HSOZLh2sGj7u_mRYNEZm5q8a-QlGjJsJ1lQ"
    }
  ]
}

Given the following token:

{
  "kid": "UWr-bIRC71YDpuIpfTEgBzhP0M737ifkyYnQN9Ki_Ao",
  "alg": "RS256"
}
.
{
  "aud": "http://nucscheduler-audience0.example.com",
  "iss": "http://nucscheduler-issuer0.example.com",
  "iat": 1616443868,
  "exp": 1617653468,
  "jti": "abc",
  "sub": "abc",
  "test": "value"
}

Running the following code gives JsonWebToken.TokenValidationStatus.InvalidSignature:

var policy = GetBasicValidationPolicyBuilder(options)
    .RequireSignature(_jwks)
    .RequireIssuer(safeIssuer)
    .Build();
return reader.TryReadToken(tokenRaw, out token);

If I include the signature algorithm in either (a) the JWKS or, (b) the RequireSignature call, it works.

watfordgnf commented 3 years ago

Minimal reproduction on v1.9.2: https://gist.github.com/watfordgnf/771f882bab8233e4def8345ad89c52ad

ycrumeyrolle commented 3 years ago

OK I understand now the issue: In version 1.X, it is required to define the signature algorithm either within the JWK with the method RequireSignature(Jwks) , or by the RequireSignature(Jwks, SignatureAlgorithm). Unfortunatly, with this design, we are not able to detect if the TokenValidationPolicy is defined correctly or not, leading to such misunderstanding, because the matching is done only at the signature validation.

In v2.X, the RequireSignature(Jwks) overloads without SignatureAlgorithm parameter are now deprecated. The issue with the v2.X design is that when the token issuer update the algorithm, it is not possible to match the keys.

watfordgnf commented 3 years ago

Being able to define the algorithm with FromX509Certificate would work well for us (allows our JWKS to interoperate with Vault), however, I believe we will still want to pass the algorithm to RequiresSignature to avoid none attacks.

ycrumeyrolle commented 3 years ago

The none attack is not possible, except if you use the method AcceptUnsecureToken (which is not recommended).

ycrumeyrolle commented 3 years ago

The version 1.9.3 introduce the alg parameter to the FromX509Certificate method, and the SignatureAlgorithm & KeyManagementAlgorithm.