Dev.to · 6 min read

Week 9: Verifying the Token

Week 9: Verifying the Token

Two pull requests merged this week and the most security-critical piece of the whole project went out for review on its own. This was the week the plan from week 7 started visibly paying off. Two merges On 28 July, #7654 merged — the LtiDeployment model. CircuitVerse can now store which LMS platforms it trusts: the issuer, the client ID, the deployment ID, and the platform's endpoint URLs, unique per issuer + client + deployment because that triple is how LTI 1.3 identifies a deployment. Nothing reads the table yet. That is the point of building this way, and it is the thing that needs explaining in every PR description. The validator The main work was #7715: Lti::JwtValidator, a class with one public method. Lti::JwtValidator.validate!(token, deployment:, nonce:) Give it an id_token, the deployment it should belong to, and the nonce we issued. It either returns the verified payload or raises. No controller, no session, no Rails request object. That isolation was forced by the constraint that it had to be its own pull request, and it is a much better boundary than I would have drawn otherwise. What it has to refuse The tests are mostly attacks: A token signed by the wrong key. The baseline. A token with alg: none. Early JWT libraries would accept an unsigned token if it declared its algorithm as none. Any validator that reads the algorithm out of the token and trusts it is exploitable. An HS256 token where RS256 is expected. This is algorithm confusion, and it is the subtle one. RS256 is asymmetric: the platform signs with a private key, we verify with their public key, which is published. HS256 is symmetric: the same secret signs and verifies. If a validator takes the algorithm from the token header and passes the platform's public key as the key, an attacker can sign a token using that public key as an HMAC secret — and the validator verifies it, because it is doing exactly what it was told. The public key is public, so anyone can do this. The defence is to never read the algorithm from the token. Pass an explicit allow-list: encoded_token.verify_signature!(algorithm: "RS256", key: key) Expired, wrong issuer, wrong audience, missing subject, replayed nonce, and the multi-audience case: when aud is an array with more than one entry, the azp claim must name our client ID. Resolving the platform's key The other half is fetching the right public key. A platform publishes a JWKS endpoint; the token header carries a kid identifying which key signed it. Two things made this more interesting than expected: Key rotation. Platforms rotate keys. A cached key set will not contain a newly rotated key. So a kid that is missing from the cache is treated as a signal that rotation may have happened, and triggers a refetch rather than a wait for expiry. Caching. Without it, every single launch makes an outbound HTTP request to the platform before anyone can sign in. If the platform's JWKS endpoint is slow, every login is slow; if it is down, every login is down. A short-lived cache keyed on the JWKS URL fixes that, with the rotation behaviour above as the escape hatch. What review found The bot review flagged two failure modes I had not handled, and both were correct: A blank jwks_url. A deployment configured with only a stored public key and no JWKS URL would still make an HTTP request, because a blank URL parses as a relative path rather than failing. So every launch for that deployment took a pointless network trip before falling back. One guard clause. Malformed stored key data. OpenSSL::PKey::RSA.new raises OpenSSL::PKey::RSAError on bad PEM data, and I was only rescuing JWT::DecodeError. So a misconfigured deployment record would propagate an OpenSSL exception out of a method whose entire contract is "raises ValidationError". Callers written against that contract would not catch it. That second one is a good example of a bug that is invisible while you are writing the code and obvious once named. My mental model was "this method validates tokens, so it raises validation errors." The actual behaviour was "this method raises validation errors unless the database row is malformed, in which case it raises something else entirely." I fixed both in fix(lti): harden JwtValidator key resolution per review. The refactor request Late in the week, a maintainer asked something more interesting than a bug report: could you suggest refactoring this class using JWT::Token or JWT::EncodedToken My implementation called JWT.decode twice — once with verification disabled to read the header and pick a key by kid, then again with verification on. That works, and it has a smell: there is a window where the code holds an unverified, parsed token, and nothing but my own discipline stops me from reading a claim out of it. JWT::EncodedToken closes that window at the library level. #payload refuses to decode until both the signature and the claims have been verified: def payload raise JWT::DecodeError, '...' unless @signature_verified raise JWT::DecodeError, '...' unless @claims_verified decoded_payload end The header is still read unverified, which is unavoidable — you cannot pick a key without knowing which key was used — but it is only ever used to select a candidate key, and the ordering the class used to depend on by convention is now enforced by the library. That is a strictly better property: the difference between "the code is correct" and "the code cannot easily be made incorrect by the next person to edit it." What I am taking from the week The isolation was the win. Because the validator was its own pull request, with no controller and no session attached, both the review comments were about the validator — its error contract, its failure modes — rather than about whether the launch flow around it made sense. Compare that to my POC, where the same class was buried in 2,000 lines and nobody looked at it at all. Next week: the first half of the handshake ships, and a scanner disagrees with me about CSRF.

This is a summary aggregated from Dev.to. Read the complete article on the original site:

Read full article at Dev.to

More Programming & Dev News