[JWT] allow for clock skew between servers for iat verification

[ol88]

MongooseIM version: 4.2.0
Installed from: docker image mongooseim/mongooseim:4.2.0
Erlang/OTP version: whatever the official docker image uses.

If jwt are issued from one server and used to authenticate a user on the mongooseim server, if the mongooseim server lags the issuing server by just a few milliseconds, the authentication will fail when iat and nbf are included in the token.

Currently, check_password in ejabberd_auth_jwt.erl uses jwerl:verify(Password, Alg, Key) with no options possible.:

check_password(LUser, LServer, Password) ->
    Key = case ejabberd_auth:get_opt(LServer, jwt_secret) of
              Key1 when is_binary(Key1) -> Key1;
              {env, Var} -> list_to_binary(os:getenv(Var))
          end,
    BinAlg = ejabberd_auth:get_opt(LServer, jwt_algorithm),
    Alg = binary_to_atom(jid:str_tolower(BinAlg), utf8),
    case jwerl:verify(Password, Alg, Key) of
        {ok, TokenData} ->
           [...]
        {error, Reason} ->
           [...]
    end.

jwerl.erl actually allows to pass Options:

verify(Data, Algorithm, KeyOrPem) ->
  verify(Data, Algorithm, KeyOrPem, #{}, #{}).

verify(Data, Algorithm, KeyOrPem, Claims) ->
  verify(Data, Algorithm, KeyOrPem, Claims, #{}).

verify(Data, Algorithm, KeyOrPem, Claims, Opts) ->
case decode(Data, KeyOrPem, Algorithm) of
    {ok, TokenData} when is_map(Claims) orelse is_list(Claims) ->
      case check_claims(TokenData, Claims, Opts) of
        [...]
      end;

One option can be used to set a leeway for iat verification to allow for clock skew:

check_claims(TokenData, Claims, Opts) ->
  Now = os:system_time(seconds),
  claims_errors(
    [
     check_claim(TokenData, exp, false, fun(ExpireTime) ->
                                            ExpLeeway = maps:get(exp_leeway, Opts, 0),
                                            Now < ExpireTime + ExpLeeway
                                        end, exp),
     check_claim(TokenData, iat, false, fun(IssuedAt) ->
                                            IatLeeway = maps:get(iat_leeway, Opts, 0),
                                            IssuedAt - IatLeeway =< Now
                                        end, iat),
     check_claim(TokenData, nbf, false, fun(NotBefore) ->
                                            NotBefore =< Now
                                        end, nbf)
     | [
        check_claim(
          TokenData,
          Claim,
          true,
          fun(Value) ->
              claim_match(Expected, Value)
          end, Claim)
        || {Claim, Expected} <- get_claims(Claims)
       ]
    ], []).

Also looking at the above, I realized that none of exp, iat or nbf are actually required to successfully authenticate with MongooseIM using jwt (required is set to false by default as per above). Yet the current documentation reads like all 3 fields are required. It does strictly mentions the fields being required but it can easily be interpreted like they are. It might be worth emphasizing they are optional if that is the intent.

The module checks the signature and validity of the following parameters:

exp - an expired token is rejected,
iat - a token must be issued in the past,
nbf - a token might not be valid yet.

The way I see it, we have the current possibilities:

1. Leave things in current state:

   • exp, iat and nbf can currently be omitted while still having a successful authentication.
   • Omitting iat and nbf will avoid any issue with time skew between servers during authentication.
   • As a result, JWT verification is somehow weakened.
   • The documentation should clearly reflects these parameters are optional and the risk for time skew if iat and nbf are used in
     the context of different server issuing and verifying the tokens using system time now for iat and nbf during the token
     creation.

2. Add a default leeway to iat verification (maybe 120sec as suggested in some other discussions), ideally configurable by mongooseim config file under jwt config and update doc accordingly.

3. Make some or all fields required but not necessarily verify them. iat could be required to be present but not checked against current time to avoid. This is apparently the closest solution to RFC spec. See Why validate that 'iat' is not in the future? jpadilla/pyjwt#190 for a similar discussion.

4. Some other combination of the above.

I'm honestly not good enough with erlang to offer a PR and I think this probably needs a bit of internal discussion before being implemented as it could trigger breaking changes.

For now on my side, I have no choice but to remove iat from the tokens and remove or set nbf in the past to make things work.

[gustawlippa]

Thank you for reporting the issue.
We've taken note and will decide how to proceed and may implement a change in the behaviour in the future.