From 6d6e3e348de18bb0b51a82ccf3715fb4c823e9a6 Mon Sep 17 00:00:00 2001
From: Evgeny Khramtsov <ekhramtsov@process-one.net>
Date: Thu, 18 Jul 2019 22:31:08 +0300
Subject: [PATCH] Improve robustness of reading jwt_key option

---
 src/econf.erl             |  2 ++
 src/ejabberd_auth_jwt.erl |  5 +----
 src/ejabberd_option.erl   |  4 ++--
 src/ejabberd_options.erl  | 10 ++++++++--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/econf.erl b/src/econf.erl
index b150056ca..ec127c4e1 100644
--- a/src/econf.erl
+++ b/src/econf.erl
@@ -159,6 +159,8 @@ format_error({bad_pem, Why, Path}) ->
 	   [Path, pkix:format_error(Why)]);
 format_error({bad_cert, Why, Path}) ->
     format_error({bad_pem, Why, Path});
+format_error({bad_jwt_key, Path}) ->
+    format("No valid JWT key found in file: ~s", [Path]);
 format_error({bad_jid, Bad}) ->
     format("Invalid XMPP address: ~s", [Bad]);
 format_error({bad_user, Bad}) ->
diff --git a/src/ejabberd_auth_jwt.erl b/src/ejabberd_auth_jwt.erl
index 7b14835d2..4d52b11d0 100644
--- a/src/ejabberd_auth_jwt.erl
+++ b/src/ejabberd_auth_jwt.erl
@@ -64,7 +64,7 @@ check_password(User, AuthzId, Server, Token) ->
 %%% Internal functions
 %%%----------------------------------------------------------------------
 check_jwt_token(User, Server, Token) ->
-    JWK = get_jwk(Server),
+    JWK = ejabberd_option:jwt_key(Server),
     try jose_jwt:verify(JWK, Token) of
         {true, {jose_jwt, Fields}, Signature} ->
             ?DEBUG("jwt verify: ~p - ~p~n", [Fields, Signature]),
@@ -100,9 +100,6 @@ check_jwt_token(User, Server, Token) ->
             false
     end.
 
-get_jwk(Host) ->
-    jose_jwk:from_binary(ejabberd_option:jwt_key(Host)).
-
 %% TODO: auth0 username is defined in 'jid' field, but we should
 %% allow customizing the name of the field containing the username
 %% to adapt to custom claims.
diff --git a/src/ejabberd_option.erl b/src/ejabberd_option.erl
index 7ba1db170..82a03491c 100644
--- a/src/ejabberd_option.erl
+++ b/src/ejabberd_option.erl
@@ -424,10 +424,10 @@ include_config_file() ->
 include_config_file(Host) ->
     ejabberd_config:get_option({include_config_file, Host}).
 
--spec jwt_key() -> binary().
+-spec jwt_key() -> jose_jwk:key().
 jwt_key() ->
     jwt_key(global).
--spec jwt_key(global | binary()) -> binary().
+-spec jwt_key(global | binary()) -> jose_jwk:key().
 jwt_key(Host) ->
     ejabberd_config:get_option({jwt_key, Host}).
 
diff --git a/src/ejabberd_options.erl b/src/ejabberd_options.erl
index cbd36cca6..a9b435dfb 100644
--- a/src/ejabberd_options.erl
+++ b/src/ejabberd_options.erl
@@ -399,7 +399,13 @@ opt_type(jwt_key) ->
       econf:file(),
       fun(Path) ->
               case file:read_file(Path) of
-                  {ok, Binary} -> Binary;
+                  {ok, Data} ->
+		      try jose_jwk:from_binary(Data) of
+			  {error, _} -> econf:fail({bad_jwt_key, Path});
+			  Ret -> Ret
+		      catch _:_ ->
+			      econf:fail({bad_jwt_key, Path})
+		      end;
                   {error, Reason} ->
                       econf:fail({read_file, Reason, Path})
               end
@@ -422,7 +428,7 @@ opt_type(jwt_key) ->
 		    {shaper, #{atom() => ejabberd_shaper:shaper_rate()}} |
 		    {shaper_rules, [{atom(), [ejabberd_shaper:shaper_rule()]}]} |
 		    {api_permissions, [ejabberd_access_permissions:permission()]} |
-		    {jwt_key, binary()} |
+		    {jwt_key, jose_jwk:key()} |
 		    {append_host_config, [{binary(), any()}]} |
 		    {host_config, [{binary(), any()}]} |
 		    {define_macro, any()} |