You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by rn...@apache.org on 2020/04/01 14:31:14 UTC
[couchdb] 07/08: Merge pull request #2727 from apache/jwt-kty-check
This is an automated email from the ASF dual-hosted git repository.
rnewson pushed a commit to branch backport-jwt-3.x
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 30eeea576b6cc89726515335790a8e0a17c72efe
Author: Robert Newson <rn...@apache.org>
AuthorDate: Sat Mar 28 09:26:25 2020 +0000
Merge pull request #2727 from apache/jwt-kty-check
Only trust the servers declaration of JWT key type
---
rel/overlay/etc/default.ini | 9 ++--
src/jwtf/src/jwtf_keystore.erl | 95 +++++++++++++++++++++++++----------
src/jwtf/test/jwtf_keystore_tests.erl | 57 +++++++++++++++++++++
test/elixir/test/jwtauth_test.exs | 6 +--
4 files changed, 134 insertions(+), 33 deletions(-)
diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini
index 8e5ce61..16387f2 100644
--- a/rel/overlay/etc/default.ini
+++ b/rel/overlay/etc/default.ini
@@ -151,14 +151,15 @@ max_db_number_for_dbs_info_req = 100
; If your JWT tokens do not include a "kid" attribute, use "_default"
; as the config key, otherwise use the kid as the config key.
; Examples
-; _default = aGVsbG8=
-; foo = aGVsbG8=
+; hmac:_default = aGVsbG8=
+; hmac:foo = aGVsbG8=
; The config values can represent symmetric and asymmetrics keys.
; For symmetrics keys, the value is base64 encoded;
-; _default = aGVsbG8= # base64-encoded form of "hello"
+; hmac:_default = aGVsbG8= # base64-encoded form of "hello"
; For asymmetric keys, the value is the PEM encoding of the public
; key with newlines replaced with the escape sequence \n.
-; foo = -----BEGIN PUBLIC KEY-----\nMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEDsr0lz/Dg3luarb+Kua0Wcj9WrfR23os\nwHzakglb8GhWRDn+oZT0Bt/26sX8uB4/ij9PEOLHPo+IHBtX4ELFFVr5GTzlqcJe\nyctaTDd1OOAPXYuc67EWtGZ3pDAzztRs\n-----END PUBLIC KEY-----\n\n
+; rsa:foo = -----BEGIN PUBLIC KEY-----\nMIIBIjAN...IDAQAB\n-----END PUBLIC KEY-----\n
+; ec:bar = -----BEGIN PUBLIC KEY-----\nMHYwEAYHK...AzztRs\n-----END PUBLIC KEY-----\n
[couch_peruser]
; If enabled, couch_peruser ensures that a private per-user database
diff --git a/src/jwtf/src/jwtf_keystore.erl b/src/jwtf/src/jwtf_keystore.erl
index 2f2f247..be261e6 100644
--- a/src/jwtf/src/jwtf_keystore.erl
+++ b/src/jwtf/src/jwtf_keystore.erl
@@ -14,6 +14,8 @@
-behaviour(gen_server).
-behaviour(config_listener).
+-include_lib("public_key/include/public_key.hrl").
+
% public api.
-export([
get/2,
@@ -29,19 +31,18 @@
% public functions
-get(Alg, undefined) ->
- get(Alg, "_default");
-
-get(Alg, KID) when is_binary(KID) ->
- get(Alg, binary_to_list(KID));
+get(Alg, undefined) when is_binary(Alg) ->
+ get(Alg, <<"_default">>);
-get(Alg, KID) ->
- case ets:lookup(?MODULE, KID) of
+get(Alg, KID0) when is_binary(Alg), is_binary(KID0) ->
+ Kty = kty(Alg),
+ KID = binary_to_list(KID0),
+ case ets:lookup(?MODULE, {Kty, KID}) of
[] ->
- Key = get_from_config(Alg, KID),
- ok = gen_server:call(?MODULE, {set, KID, Key}),
+ Key = get_from_config(Kty, KID),
+ ok = gen_server:call(?MODULE, {set, Kty, KID, Key}),
Key;
- [{KID, Key}] ->
+ [{{Kty, KID}, Key}] ->
Key
end.
@@ -57,13 +58,13 @@ init(_) ->
{ok, nil}.
-handle_call({set, KID, Key}, _From, State) ->
- true = ets:insert(?MODULE, {KID, Key}),
+handle_call({set, Kty, KID, Key}, _From, State) ->
+ true = ets:insert(?MODULE, {{Kty, KID}, Key}),
{reply, ok, State}.
-handle_cast({delete, KID}, State) ->
- true = ets:delete(?MODULE, KID),
+handle_cast({delete, Kty, KID}, State) ->
+ true = ets:delete(?MODULE, {Kty, KID}),
{noreply, State};
handle_cast(_Msg, State) ->
@@ -88,8 +89,14 @@ code_change(_OldVsn, State, _Extra) ->
% config listener callback
-handle_config_change("jwt_keys", KID, _Value, _, _) ->
- {ok, gen_server:cast(?MODULE, {delete, KID})};
+handle_config_change("jwt_keys", ConfigKey, _ConfigValue, _, _) ->
+ case string:split(ConfigKey, ":") of
+ [Kty, KID] ->
+ gen_server:cast(?MODULE, {delete, Kty, KID});
+ _ ->
+ ignored
+ end,
+ {ok, nil};
handle_config_change(_, _, _, _, _) ->
{ok, nil}.
@@ -102,17 +109,53 @@ handle_config_terminate(_Server, _Reason, _State) ->
% private functions
-get_from_config(Alg, KID) ->
- case config:get("jwt_keys", KID) of
+get_from_config(Kty, KID) ->
+ case config:get("jwt_keys", string:join([Kty, KID], ":")) of
undefined ->
throw({bad_request, <<"Unknown kid">>});
- Key ->
- case jwtf:verification_algorithm(Alg) of
- {hmac, _} ->
- base64:decode(Key);
- {public_key, _} ->
- BinKey = iolist_to_binary(string:replace(Key, "\\n", "\n", all)),
- [PEMEntry] = public_key:pem_decode(BinKey),
- public_key:pem_entry_decode(PEMEntry)
+ Encoded ->
+ case Kty of
+ "hmac" ->
+ try
+ base64:decode(Encoded)
+ catch
+ error:_ ->
+ throw({bad_request, <<"Not a valid key">>})
+ end;
+ "rsa" ->
+ case pem_decode(Encoded) of
+ #'RSAPublicKey'{} = Key ->
+ Key;
+ _ ->
+ throw({bad_request, <<"not an RSA public key">>})
+ end;
+ "ec" ->
+ case pem_decode(Encoded) of
+ {#'ECPoint'{}, _} = Key ->
+ Key;
+ _ ->
+ throw({bad_request, <<"not an EC public key">>})
+ end
end
end.
+
+pem_decode(PEM) ->
+ BinPEM = iolist_to_binary(string:replace(PEM, "\\n", "\n", all)),
+ case public_key:pem_decode(BinPEM) of
+ [PEMEntry] ->
+ public_key:pem_entry_decode(PEMEntry);
+ [] ->
+ throw({bad_request, <<"Not a valid key">>})
+ end.
+
+kty(<<"HS", _/binary>>) ->
+ "hmac";
+
+kty(<<"RS", _/binary>>) ->
+ "rsa";
+
+kty(<<"ES", _/binary>>) ->
+ "ec";
+
+kty(_) ->
+ throw({bad_request, <<"Unknown kty">>}).
diff --git a/src/jwtf/test/jwtf_keystore_tests.erl b/src/jwtf/test/jwtf_keystore_tests.erl
new file mode 100644
index 0000000..9ec9436
--- /dev/null
+++ b/src/jwtf/test/jwtf_keystore_tests.erl
@@ -0,0 +1,57 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(jwtf_keystore_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+-include_lib("public_key/include/public_key.hrl").
+
+-define(HMAC_SECRET, "aGVsbG8=").
+-define(RSA_SECRET, "-----BEGIN PUBLIC KEY-----\\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAztanwQtIx0sms+x7m1SF\\nh7EHJHkM2biTJ41jR89FsDE2gd3MChpaqxemS5GpNvfFKRvuHa4PUZ3JtRCBG1KM\\n/7EWIVTy1JQDr2mb8couGlQNqz4uXN2vkNQ0XszgjU4Wn6ZpvYxmqPFbmkRe8QSn\\nAy2Wf8jQgjsbez8eaaX0G9S1hgFZUN3KFu7SVmUDQNvWpQdaJPP+ms5Z0CqF7JLa\\nvJmSdsU49nlYw9VH/XmwlUBMye6HgR4ZGCLQS85frqF0xLWvi7CsMdchcIjHudXH\\nQK1AumD/VVZVdi8Q5Qew7F6VXeXqnhbw9n6Px25cCuNuh6u5+E6GUzXRrMpqo9vO\\nqQIDAQAB\\n-----END PUBLIC KEY-----\\n").
+-define(EC_SECRET, "-----BEGIN PUBLIC KEY-----\\nMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEDsr0lz/Dg3luarb+Kua0Wcj9WrfR23os\\nwHzakglb8GhWRDn+oZT0Bt/26sX8uB4/ij9PEOLHPo+IHBtX4ELFFVr5GTzlqcJe\\nyctaTDd1OOAPXYuc67EWtGZ3pDAzztRs\\n-----END PUBLIC KEY-----\\n").
+
+setup() ->
+ test_util:start_applications([config, jwtf]),
+ config:set("jwt_keys", "hmac:hmac", ?HMAC_SECRET),
+ config:set("jwt_keys", "rsa:hmac", ?HMAC_SECRET),
+ config:set("jwt_keys", "ec:hmac", ?HMAC_SECRET),
+
+ config:set("jwt_keys", "hmac:rsa", ?RSA_SECRET),
+ config:set("jwt_keys", "rsa:rsa", ?RSA_SECRET),
+ config:set("jwt_keys", "ec:rsa", ?RSA_SECRET),
+
+ config:set("jwt_keys", "hmac:ec", ?EC_SECRET),
+ config:set("jwt_keys", "rsa:ec", ?EC_SECRET),
+ config:set("jwt_keys", "ec:ec", ?EC_SECRET).
+
+teardown(_) ->
+ test_util:stop_applications([config, jwtf]).
+
+jwtf_keystore_test_() ->
+ {
+ setup,
+ fun setup/0,
+ fun teardown/1,
+ [
+ ?_assertEqual(<<"hello">>, jwtf_keystore:get(<<"HS256">>, <<"hmac">>)),
+ ?_assertThrow({bad_request, _}, jwtf_keystore:get(<<"RS256">>, <<"hmac">>)),
+ ?_assertThrow({bad_request, _}, jwtf_keystore:get(<<"ES256">>, <<"hmac">>)),
+
+ ?_assertThrow({bad_request, _}, jwtf_keystore:get(<<"HS256">>, <<"rsa">>)),
+ ?_assertMatch(#'RSAPublicKey'{}, jwtf_keystore:get(<<"RS256">>, <<"rsa">>)),
+ ?_assertThrow({bad_request, _}, jwtf_keystore:get(<<"ES256">>, <<"rsa">>)),
+
+ ?_assertThrow({bad_request, _}, jwtf_keystore:get(<<"HS256">>, <<"ec">>)),
+ ?_assertThrow({bad_request, _}, jwtf_keystore:get(<<"RS256">>, <<"ec">>)),
+ ?_assertMatch({#'ECPoint'{}, _}, jwtf_keystore:get(<<"ES256">>, <<"ec">>))
+ ]
+ }.
diff --git a/test/elixir/test/jwtauth_test.exs b/test/elixir/test/jwtauth_test.exs
index 0c13404..9d43c23 100644
--- a/test/elixir/test/jwtauth_test.exs
+++ b/test/elixir/test/jwtauth_test.exs
@@ -10,7 +10,7 @@ defmodule JwtAuthTest do
server_config = [
%{
:section => "jwt_keys",
- :key => "_default",
+ :key => "hmac:_default",
:value => :base64.encode(secret)
},
%{
@@ -49,7 +49,7 @@ defmodule JwtAuthTest do
server_config = [
%{
:section => "jwt_keys",
- :key => "_default",
+ :key => "rsa:_default",
:value => public_pem
},
%{
@@ -87,7 +87,7 @@ defmodule JwtAuthTest do
server_config = [
%{
:section => "jwt_keys",
- :key => "_default",
+ :key => "ec:_default",
:value => public_pem
},
%{