123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142 |
- Subject: Keys: move signing part out of find_by_thp() and to find_jws() (#81)
- ID: CVE-2021-4076
- Origin: v10-9-ge82459f <https://github.com/latchset/tang/commit/v10-9-ge82459f>
- Upstream-Author: Sergio Correia <scorreia@redhat.com>
- 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},
|