Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Public keys, with signatures from unknown keys, are treated as invalid keys #1001

Closed
kaie opened this issue Dec 11, 2019 · 10 comments
Closed
Assignees
Milestone

Comments

@kaie
Copy link
Contributor

kaie commented Dec 11, 2019

Alice's public key was signed by Bob.
Alice sends her key to Carol.

Carol uses OpenPGP software based on RNP.

Carol imports Alice's key, but doesn't have Bob's key.
Carol attempts to send an encrypted message to Alice.

Actual behavior:
Sending the encrypted message fails.
RNP reports that Alice's key is invalid.
[signature_check() /home/user/github/rnp/src/librepgp/stream-sig.cpp:1099] invalid or untrusted key
[validate_pgp_key_signature() /home/user/github/rnp/src/librepgp/stream-key.cpp:1571] bad signature
(line numbers are based on revision 369a687)

Expected behavior:
Carol should be able to send an encrypted message to Alice, even if Carol doesn't have Bob's key.

@kaie
Copy link
Contributor Author

kaie commented Dec 11, 2019

To check the unknown signature is really causing the issue, I have used gnupg to edit Alice's key, removed Bob's signature, and imported this reduced key into RNP. This fixed the issue, sending the encrypted message to Alice was successful.

@ni4
Copy link
Contributor

ni4 commented Dec 11, 2019

@kaie thanks for noticing it. Within current logic signature from unknown key is considered as invalid, so the whole key is marked as invalid. And it seems to be too strict. Actually improving key trust settings is also part of the ongoing work.

@kaie
Copy link
Contributor Author

kaie commented Dec 12, 2019

This patch allows me to use my usual keys.

diff --git a/src/librepgp/stream-key.cpp b/src/librepgp/stream-key.cpp
index 20c925d7..3a28ec71 100644
--- a/src/librepgp/stream-key.cpp
+++ b/src/librepgp/stream-key.cpp
@@ -1474,7 +1474,8 @@ typedef struct validate_info_t {
  *         during the validation.
  */
 static rnp_result_t
-validate_pgp_key_signature(const pgp_signature_t *sig, validate_info_t *info)
+validate_pgp_key_signature(const pgp_signature_t *sig, validate_info_t *info,
+                           bool ignore_missing_signer_key)
 {
     rnp_result_t         res = RNP_ERROR_GENERIC;
     uint8_t              signer_id[PGP_KEY_ID_SIZE] = {0};
@@ -1493,6 +1494,9 @@ validate_pgp_key_signature(const pgp_signature_t *sig, validate_info_t *info)
     }
     sinfo.signer = rnp_key_store_get_key_by_id(info->keystore, signer_id, NULL);
     if (!sinfo.signer) {
+        if (ignore_missing_signer_key) {
+          return RNP_SUCCESS;
+        }
         sinfo.no_signer = true;
         goto done;
     }
@@ -1585,7 +1589,7 @@ validate_pgp_key_signature_list(list sigs, validate_info_t *info)
     rnp_result_t res = RNP_SUCCESS;
 
     for (list_item *s = list_front(sigs); s; s = list_next(s)) {
-        res = validate_pgp_key_signature((pgp_signature_t *) s, info);
+        res = validate_pgp_key_signature((pgp_signature_t *) s, info, true);
         if (res) {
             return res;
         }

edit: removed a leftover from a previous patch

@ni4
Copy link
Contributor

ni4 commented Jan 12, 2020

@kaie Issue should be fixed in #1017. Could you please confirm, so we can close this ticket?

@ni4 ni4 self-assigned this Jan 28, 2020
@kaie
Copy link
Contributor Author

kaie commented Feb 16, 2020

Sorry for the delay. It's on my TODO list, I'll test this in the next few days.

@ni4
Copy link
Contributor

ni4 commented Feb 17, 2020

@kaie No problem. Also you are welcome to check revocation signature export, as it was implemented in PR #1066 and merged.
P.S. Btw, from your experience, does 'revocation key signature subpacket' is widely used so has high priority for implementation? AFAIR there were some discussion in OpenPGP mailing list regarding it's deprecation.

@kaie
Copy link
Contributor Author

kaie commented Feb 18, 2020

I confirm I can successfully encrypt to keys that contain unknown signatures, as an example I used my own key 25007724, without importing any of the keys that signed it.

Regarding your question on revocation, I don't have the answer, I need to get back to you, let's involve also Patrick Brunschwig.

@kaie kaie closed this as completed Feb 18, 2020
@kaie
Copy link
Contributor Author

kaie commented Feb 18, 2020

(Tested with yesterday's tip)

@kaie
Copy link
Contributor Author

kaie commented Mar 13, 2020

@ni4 I've been able to use new API rnp_key_export_revocation and it works for me.

@ni4
Copy link
Contributor

ni4 commented Mar 13, 2020

@kaie Great, thanks for letting us know.

@ni4 ni4 added this to the v0.14.0 milestone May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants