potatosalad/erlang-jose

Verifying exp/iat/nbf claims

Opened this issue · 8 comments

From what I can tell, the temporal claims (exp/iat/nbf) are not verified by e.g. jose_jwt:verify_strict.
So for example an expired token is verified as OK despite the expiry date being in the past.

Is this beyond the scope of this library? It would certainly be convenient to have expired tokens be rejected (or similarly, tokens that have nbf/iat in the future).

I was previously using jwerl which had this feature (and swapped to this library for JWK support):
https://github.com/G-Corp/jwerl/blob/master/src/jwerl.erl#L102

Is this beyond the scope of this library?

Initially: yes, beyond the scope. Primarily because the JWT RFC alone is overly vague on how these claims are intended to be validated (note that every single claim defined is described as "OPTIONAL", versus something like the OpenID Connect Core 1.0 RFC which provides improved definitions and even tags certain claims as "REQUIRED").

However, a few years ago I put together iam_assert and iam_claims as part of the iam library and have found myself regularly relying on them for performing things like exp/iat/nbf validation.

Example usage of iam_claims:

def sign(client_id, user_id, sig_secret_key, now) do
  # Cast all of the keys to JWKs
  sig_secret_jwk = JOSE.JWK.from(sig_secret_key)

  # Sign
  issued_at = now
  expire_at = issued_at + div(:timer.minutes(15), :timer.seconds(1))
  not_before = issued_at - div(:timer.minutes(5), :timer.seconds(1))

  jws =
    sig_secret_jwk
    |> JOSE.JWK.signer()
    |> JOSE.JWS.merge(%{
      "typ" => "JWT"
    })

  jwt =
    :iam_claims.new()
    |> :iam_claims.put(:audience, client_id)
    |> :iam_claims.put(:issuer, "https://as.idp/")
    |> :iam_claims.put(:subject, user_id)
    |> :iam_claims.issue(issued_at)
    |> :iam_claims.not_before(not_before)
    |> :iam_claims.put(:expiration_time, expire_at)
    |> :iam_claims.put(:jwt_id, {:hash, :md5})

  jwt = %:iam_claims{jwt | key_id: false}

  :iam_claims.sign(jwt, jws, sig_secret_jwk)
end

Example usage of iam_assert:

def authenticate(access_token, client_id, user_id, sig_public_key, now) do
  # Cast all of the keys to JWKs
  sig_public_jwk = JOSE.JWK.from(sig_public_key)

  # Validate-and-verify
  assert =
    %{
      jwt_id: {:hash, :md5},
      now: now,
      window: 5
    }
    |> :iam_assert.new()
    |> :iam_assert.require([
      :audience,
      :issuer,
      :subject,
      :issued_at,
      :not_before,
      :expiration_time,
      :jwt_id
    ])
    |> :iam_assert.check(:audience, client_id)
    |> :iam_assert.check(:issuer, "https://as.idp/")
    |> :iam_assert.check(:subject, user_id)

  try do
    case :iam_assert.authenticate(assert, access_token, ["EdDSA"], sig_public_jwk) do
      %:iam_assert{claims: claims, validated: true, verified: true} ->
        {:ok, claims}

      _ ->
        :error
    end
  catch
    _class, _reason ->
      :error
  end
end

At this point, I think it warrants including something similar to the iam modules in this library.

Thanks for such a detailed response, makes perfect sense. The iam modules look really useful, I am tempted to use them in place of my ad hoc assertions for exp etc

I'd recommend using it in the meantime, if nothing else but to prevent mistakes in re-implementing the same functionality in multiple places. The window feature in iam_assert may also be useful to you (where now is allow to have a time skew of up to +/- window seconds).

Also, if this helps: the iam library uses jose underneath and is being used in multiple services in production today. It's just a little quirky/clunky to use and really ought to be rolled up into the jose library itself.

Unfortunately I'm using Erlang 22 and getting erlang:get_stacktrace/0: deprecated; use the new try/catch syntax for retrieving the stack backtrace warnings with OJSON and IAM. Shall I open issues on those projects?

Yup, that's fine. The deprecation warnings for the stacktrace stuff should be harmless on OTP 22, though, they're being emitted internally from within OTP itself and are not warnings like the ones emitted by Elixir.

Ah yes of course - I had assumed they were causing problems but it must have been a red herring and just a problem with my attempt at translating the Elixir examples to Erlang. I'll have another bash at implementing it

Just tried again and I'd forgotten that this is a compile failure (in erl 22/22.1 at least):

===> Compiling _build/default/lib/ojson/src/ojson_decoder.erl failed
_build/default/lib/ojson/src/ojson_decoder.erl:89: erlang:get_stacktrace/0: deprecated; use the new try/catch syntax for retrieving the stack backtrace
_build/default/lib/ojson/src/ojson_decoder.erl:91: erlang:get_stacktrace/0: deprecated; use the new try/catch syntax for retrieving the stack backtrace

Not sure how to get around this - I disabled rebar3's warnings_as_errors to no avail.

This seems to work for now in rebar3.config:

{overrides,
    [{override, ojson, [{erl_opts, [debug_info]}]},
     {override, iam, [{erl_opts, [debug_info]}]}]
}.