Subject: Keys: move signing part out of find_by_thp() and to find_jws() (#81) ID: CVE-2021-4076 Origin: v10-9-ge82459f Upstream-Author: Sergio Correia Date: Tue Dec 14 08:18:41 2021 -0300 Handle just signing keys in find_jws(), to make sure we are responding only to proper queries. Tests were also failing to detect this issue and were updated accordingly. Issue discovered by Twitter Kernel and OS team during a source code audit while evaluating Tang/Clevis for their needs. Fixes CVE-2021-4076 --- a/src/keys.c +++ b/src/keys.c @@ -263,20 +263,7 @@ if (strcmp(thumbprint, target) != 0) { continue; } - - if (jwk_valid_for_deriving_keys(jwk)) { - return json_incref(jwk); - } else if (jwk_valid_for_signing(jwk)) { - json_auto_t* sign = json_deep_copy(tki->m_sign); - if (json_array_append(sign, jwk) == -1) { - return NULL; - } - json_auto_t* jws = jwk_sign(tki->m_payload, sign); - if (!jws) { - return NULL; - } - return json_incref(jws); - } + return json_incref(jwk); } } return NULL; @@ -436,7 +423,21 @@ } return json_incref(jws); } - return find_by_thp(tki, thp); + + json_auto_t* jwk = find_by_thp(tki, thp); + if (!jwk_valid_for_signing(jwk)) { + return NULL; + } + + json_auto_t* sign = json_deep_copy(tki->m_sign); + if (json_array_append(sign, jwk) == -1) { + return NULL; + } + json_auto_t* jws = jwk_sign(tki->m_payload, sign); + if (!jws) { + return NULL; + } + return json_incref(jws); } json_t* --- a/tests/adv +++ b/tests/adv @@ -36,11 +36,11 @@ sleep 0.5 # Make sure requests on the root fail -! fetch / +fetch / && expected_fail # The request should fail (404) for non-signature key IDs -! fetch /adv/`jose jwk thp -i $TMP/db/exc.jwk` -! fetch /adv/`jose jwk thp -a S512 -i $TMP/db/exc.jwk` +fetch /adv/`jose jwk thp -i $TMP/db/exc.jwk` && expected_fail +fetch /adv/`jose jwk thp -a S512 -i $TMP/db/exc.jwk` && expected_fail # The default advertisement fetch should succeed and pass verification fetch /adv @@ -52,17 +52,17 @@ fetch /adv/`jose jwk thp -a S512 -i $TMP/db/sig.jwk` | ver $TMP/db/sig.jwk # Requesting an adv by an advertised key ID should't be signed by hidden keys -! fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.sig.jwk -! fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.oth.jwk +fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.sig.jwk && expected_fail +fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.oth.jwk && expected_fail # Verify that the default advertisement is not signed with hidden signature keys -! fetch /adv/ | ver $TMP/db/.oth.jwk -! fetch /adv/ | ver $TMP/db/.sig.jwk +fetch /adv/ | ver $TMP/db/.oth.jwk && expected_fail +fetch /adv/ | ver $TMP/db/.sig.jwk && expected_fail # A private key advertisement is signed by all advertised keys and the requested private key fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/sig.jwk fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/.sig.jwk -! fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/.oth.jwk +fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/.oth.jwk && expected_fail # Verify that the advertisements contain the cty parameter fetch /adv | jose fmt -j- -Og protected -SyOg cty -Sq "jwk-set+json" -E --- a/tests/helpers +++ b/tests/helpers @@ -60,3 +60,8 @@ # Skip test if socat is not available. [ -n "${SOCAT}" ] || exit 77 } + +expected_fail () { + echo "Test was expected to fail" >&2 + exit 1 +} --- a/tests/rec +++ b/tests/rec @@ -42,8 +42,8 @@ sleep 0.5 # Make sure that GET fails -! curl -sf http://127.0.0.1:$PORT/rec -! curl -sf http://127.0.0.1:$PORT/rec/ +curl -sf http://127.0.0.1:$PORT/rec && expected_fail +curl -sf http://127.0.0.1:$PORT/rec/ && expected_fail # Make a recovery request (NOTE: this is insecure! Don't do this in real code!) good=`jose jwk exc -i '{"alg":"ECMR","key_ops":["deriveKey"]}' -l $TMP/exc.jwk -r $TMP/db/exc.jwk` --- a/tests/test-keys.c.in +++ b/tests/test-keys.c.in @@ -104,6 +104,11 @@ {"ugJ4Ula-YABQIiJ-0g3B_jpFpF2nl3W-DNpfLdXArhTusV0QCcd1vtgDeGHEPzpm7jEsyC7VYYSSOkZicK22mw", 1}, {"up0Z4fRhpd4O5QwBaMCXDTlrvxCmZacU0MD8kw", 1}, {"vllHS-M0aQFCo2yUCcAahMU4TAtXACyeuRf-zbmmTPBg7V0Pb-RRFGo5C6MnpzdirK8B3ORLOsN8RyXClvtjxA", 1}, + {"-bWkGaJi0Zdvxaj4DCp28umLcRA", 0}, + {"WEpfFyeoNKkE2-TosN_bP-gd9UgRvQCZpVasZQ", 0}, + {"L4xg2tZXTEVbsK39bzOZM1jGWn3HtOxF5gh6F9YVf5Q", 0}, + {"9U8qgy_YjyY6Isuq6QuiKEiYZgNJShcGgJx5FJzCu6m3N6zFaIPy_HDkxkVqAZ9E", 0}, + {"Cy73glFjs6B6RU7wy6vWxAc-2bJy5VJOT9LyK80eKgZ8k27wXZ-3rjsuNU5tua_yHWtluyoSYtjoKXfI0E8ESw", 0}, {NULL, 1}, {"a", 0}, {"foo", 0},