1639480721.v10-9-ge82459f.keys-move-signing-part-out-of-find-by-thp-and-to-find-jws-81.patch 5.0 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142
  1. Subject: Keys: move signing part out of find_by_thp() and to find_jws() (#81)
  2. ID: CVE-2021-4076
  3. Origin: v10-9-ge82459f <https://github.com/latchset/tang/commit/v10-9-ge82459f>
  4. Upstream-Author: Sergio Correia <scorreia@redhat.com>
  5. Date: Tue Dec 14 08:18:41 2021 -0300
  6. Handle just signing keys in find_jws(), to make sure we are
  7. responding only to proper queries.
  8. Tests were also failing to detect this issue and were updated
  9. accordingly.
  10. Issue discovered by Twitter Kernel and OS team during a source
  11. code audit while evaluating Tang/Clevis for their needs.
  12. Fixes CVE-2021-4076
  13. --- a/src/keys.c
  14. +++ b/src/keys.c
  15. @@ -263,20 +263,7 @@
  16. if (strcmp(thumbprint, target) != 0) {
  17. continue;
  18. }
  19. -
  20. - if (jwk_valid_for_deriving_keys(jwk)) {
  21. - return json_incref(jwk);
  22. - } else if (jwk_valid_for_signing(jwk)) {
  23. - json_auto_t* sign = json_deep_copy(tki->m_sign);
  24. - if (json_array_append(sign, jwk) == -1) {
  25. - return NULL;
  26. - }
  27. - json_auto_t* jws = jwk_sign(tki->m_payload, sign);
  28. - if (!jws) {
  29. - return NULL;
  30. - }
  31. - return json_incref(jws);
  32. - }
  33. + return json_incref(jwk);
  34. }
  35. }
  36. return NULL;
  37. @@ -436,7 +423,21 @@
  38. }
  39. return json_incref(jws);
  40. }
  41. - return find_by_thp(tki, thp);
  42. +
  43. + json_auto_t* jwk = find_by_thp(tki, thp);
  44. + if (!jwk_valid_for_signing(jwk)) {
  45. + return NULL;
  46. + }
  47. +
  48. + json_auto_t* sign = json_deep_copy(tki->m_sign);
  49. + if (json_array_append(sign, jwk) == -1) {
  50. + return NULL;
  51. + }
  52. + json_auto_t* jws = jwk_sign(tki->m_payload, sign);
  53. + if (!jws) {
  54. + return NULL;
  55. + }
  56. + return json_incref(jws);
  57. }
  58. json_t*
  59. --- a/tests/adv
  60. +++ b/tests/adv
  61. @@ -36,11 +36,11 @@
  62. sleep 0.5
  63. # Make sure requests on the root fail
  64. -! fetch /
  65. +fetch / && expected_fail
  66. # The request should fail (404) for non-signature key IDs
  67. -! fetch /adv/`jose jwk thp -i $TMP/db/exc.jwk`
  68. -! fetch /adv/`jose jwk thp -a S512 -i $TMP/db/exc.jwk`
  69. +fetch /adv/`jose jwk thp -i $TMP/db/exc.jwk` && expected_fail
  70. +fetch /adv/`jose jwk thp -a S512 -i $TMP/db/exc.jwk` && expected_fail
  71. # The default advertisement fetch should succeed and pass verification
  72. fetch /adv
  73. @@ -52,17 +52,17 @@
  74. fetch /adv/`jose jwk thp -a S512 -i $TMP/db/sig.jwk` | ver $TMP/db/sig.jwk
  75. # Requesting an adv by an advertised key ID should't be signed by hidden keys
  76. -! fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.sig.jwk
  77. -! fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.oth.jwk
  78. +fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.sig.jwk && expected_fail
  79. +fetch /adv/`jose jwk thp -i $TMP/db/sig.jwk` | ver $TMP/db/.oth.jwk && expected_fail
  80. # Verify that the default advertisement is not signed with hidden signature keys
  81. -! fetch /adv/ | ver $TMP/db/.oth.jwk
  82. -! fetch /adv/ | ver $TMP/db/.sig.jwk
  83. +fetch /adv/ | ver $TMP/db/.oth.jwk && expected_fail
  84. +fetch /adv/ | ver $TMP/db/.sig.jwk && expected_fail
  85. # A private key advertisement is signed by all advertised keys and the requested private key
  86. fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/sig.jwk
  87. fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/.sig.jwk
  88. -! fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/.oth.jwk
  89. +fetch /adv/`jose jwk thp -i $TMP/db/.sig.jwk` | ver $TMP/db/.oth.jwk && expected_fail
  90. # Verify that the advertisements contain the cty parameter
  91. fetch /adv | jose fmt -j- -Og protected -SyOg cty -Sq "jwk-set+json" -E
  92. --- a/tests/helpers
  93. +++ b/tests/helpers
  94. @@ -60,3 +60,8 @@
  95. # Skip test if socat is not available.
  96. [ -n "${SOCAT}" ] || exit 77
  97. }
  98. +
  99. +expected_fail () {
  100. + echo "Test was expected to fail" >&2
  101. + exit 1
  102. +}
  103. --- a/tests/rec
  104. +++ b/tests/rec
  105. @@ -42,8 +42,8 @@
  106. sleep 0.5
  107. # Make sure that GET fails
  108. -! curl -sf http://127.0.0.1:$PORT/rec
  109. -! curl -sf http://127.0.0.1:$PORT/rec/
  110. +curl -sf http://127.0.0.1:$PORT/rec && expected_fail
  111. +curl -sf http://127.0.0.1:$PORT/rec/ && expected_fail
  112. # Make a recovery request (NOTE: this is insecure! Don't do this in real code!)
  113. good=`jose jwk exc -i '{"alg":"ECMR","key_ops":["deriveKey"]}' -l $TMP/exc.jwk -r $TMP/db/exc.jwk`
  114. --- a/tests/test-keys.c.in
  115. +++ b/tests/test-keys.c.in
  116. @@ -104,6 +104,11 @@
  117. {"ugJ4Ula-YABQIiJ-0g3B_jpFpF2nl3W-DNpfLdXArhTusV0QCcd1vtgDeGHEPzpm7jEsyC7VYYSSOkZicK22mw", 1},
  118. {"up0Z4fRhpd4O5QwBaMCXDTlrvxCmZacU0MD8kw", 1},
  119. {"vllHS-M0aQFCo2yUCcAahMU4TAtXACyeuRf-zbmmTPBg7V0Pb-RRFGo5C6MnpzdirK8B3ORLOsN8RyXClvtjxA", 1},
  120. + {"-bWkGaJi0Zdvxaj4DCp28umLcRA", 0},
  121. + {"WEpfFyeoNKkE2-TosN_bP-gd9UgRvQCZpVasZQ", 0},
  122. + {"L4xg2tZXTEVbsK39bzOZM1jGWn3HtOxF5gh6F9YVf5Q", 0},
  123. + {"9U8qgy_YjyY6Isuq6QuiKEiYZgNJShcGgJx5FJzCu6m3N6zFaIPy_HDkxkVqAZ9E", 0},
  124. + {"Cy73glFjs6B6RU7wy6vWxAc-2bJy5VJOT9LyK80eKgZ8k27wXZ-3rjsuNU5tua_yHWtluyoSYtjoKXfI0E8ESw", 0},
  125. {NULL, 1},
  126. {"a", 0},
  127. {"foo", 0},