Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verifying exp/iat/nbf claims #77

Open
afhole opened this issue Aug 22, 2019 · 8 comments
Open

Verifying exp/iat/nbf claims #77

afhole opened this issue Aug 22, 2019 · 8 comments

Comments

@afhole
Copy link

afhole commented Aug 22, 2019

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

@potatosalad
Copy link
Owner

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.

@afhole
Copy link
Author

afhole commented Sep 13, 2019

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

@potatosalad
Copy link
Owner

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.

@afhole
Copy link
Author

afhole commented Sep 17, 2019

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?

@potatosalad
Copy link
Owner

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.

@afhole
Copy link
Author

afhole commented Sep 20, 2019

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

@afhole
Copy link
Author

afhole commented Sep 27, 2019

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.

@afhole
Copy link
Author

afhole commented Sep 27, 2019

This seems to work for now in rebar3.config:

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

@potatosalad potatosalad added this to the jose 1.12.0 milestone Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Status: To do
Development

No branches or pull requests

2 participants