* [BUG] mml2015-epg-find-usable-key finds unusable key @ 2013-02-13 19:55 Łukasz Stelmach 2013-02-14 19:28 ` Łukasz Stelmach 2013-02-15 4:05 ` Daiki Ueno 0 siblings, 2 replies; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-13 19:55 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 1238 bytes --] Hi, It looks like there is a sort of bug in mml2015-epg-encrypt at line 1117. http://git.gnus.org/cgit/gnus.git/tree/lisp/mml2015.el?id=6e422932976e94d6dbb7ad602c5be3eb481fe46c#n1117) The whole form that begins at line 1113 converts list of recipients provided as email addresses, key IDs or anything gpg can digest into epg "objects". Unfortunately there is one corner case that makes this code fail. That is when a key is valid but a uid on it has been revoked. Let's follow the code. epg-list-keys returns all keys for an ID. That seems correct, however, since it knows the uid we are looking for it might check if the uid hasn't been revoked. mml2015-epg-find-usable-key, on the other hand, knows nothing about the recipient, it just checks validity of the *keys* not uids. epg.el is a part of emacs, mml2015.el belongs to gnus. IMHO it is mml2015-epg-find-usable-key that should be fixed. The function should accept and additional (optional?) parameter with a recipient uid as passed to epg-list-keys and reject keys with all uids matching the recipient parameter revoked. I'd like to know opinion of someone who knows better than I the ideas behind this code. -- Miłego dnia, Łukasz Stelmach [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-13 19:55 [BUG] mml2015-epg-find-usable-key finds unusable key Łukasz Stelmach @ 2013-02-14 19:28 ` Łukasz Stelmach 2013-02-15 4:05 ` Daiki Ueno 1 sibling, 0 replies; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-14 19:28 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 782 bytes --] Łukasz Stelmach <stlman@poczta.fm> writes: > It looks like there is a sort of bug in mml2015-epg-encrypt at line > 1117. [...] > I'd like to know opinion of someone who knows better than I the ideas > behind this code. I did some more research and here is what I found. 1. It might be actually gpg's business not to return keys with revoked uids. I've sent a question about this to the gpg mailing list. 2. Finding a revoked uid in epg or mml2015 means reimplementing of a bit complex chunk of code responsibel for matching uids (uids are not structured) from gpg in elisp. Not necessarily a lot of fun to do. If you know something I still haven't mentioned please do share. I'll keep you informed about my findings. -- Miłego dnia, Łukasz Stelmach [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-13 19:55 [BUG] mml2015-epg-find-usable-key finds unusable key Łukasz Stelmach 2013-02-14 19:28 ` Łukasz Stelmach @ 2013-02-15 4:05 ` Daiki Ueno 2013-02-15 7:59 ` Łukasz Stelmach 2013-02-16 18:35 ` Łukasz Stelmach 1 sibling, 2 replies; 12+ messages in thread From: Daiki Ueno @ 2013-02-15 4:05 UTC (permalink / raw) To: =?iso-2022-jp-2?B?GyQoRCkoGyhCdWthc3o=?= Stelmach; +Cc: ding [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=iso-2022-jp-2, Size: 4850 bytes --] ^[$(D)(^[(Bukasz Stelmach <stlman@poczta.fm> writes: > It looks like there is a sort of bug in mml2015-epg-encrypt at line > 1117. > > http://git.gnus.org/cgit/gnus.git/tree/lisp/mml2015.el?id=6e422932976e94d6dbb7ad602c5be3eb481fe46c#n1117) > > The whole form that begins at line 1113 converts list of recipients > provided as email addresses, key IDs or anything gpg can digest into epg > "objects". Unfortunately there is one corner case that makes this code > fail. That is when a key is valid but a uid on it has been revoked. Thanks for the report. > epg.el is a part of emacs, mml2015.el belongs to gnus. IMHO it is > mml2015-epg-find-usable-key that should be fixed. The function should > accept and additional (optional?) parameter with a recipient uid as > passed to epg-list-keys and reject keys with all uids matching the > recipient parameter revoked. Sounds reasonable. I plan to apply the attached patch. Regards, -- Daiki Ueno From 9440de44c8dc03a5b77e662539d3d388f6718549 Mon Sep 17 00:00:00 2001 From: Daiki Ueno <ueno@gnu.org> Date: Fri, 15 Feb 2013 12:58:09 +0900 Subject: [PATCH] mml2015.el (mml2015-epg-find-usable-key): handle revoked user-id --- lisp/ChangeLog | 9 +++++++++ lisp/mml2015.el | 57 +++++++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/lisp/ChangeLog b/lisp/ChangeLog index 5edf6f6..3fda767 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,12 @@ +2013-02-15 Daiki Ueno <ueno@gnu.org> + + * mml2015.el (mml2015-epg-check-user-id): New function. + (mml2015-epg-check-sub-key): New function split from + mml2015-epg-find-usable-key. + (mml2015-epg-find-usable-key): Accept context, recipient, and usage, to + handle the case when user-id is unusable (all callers changed). + Reported by ^[$(D)(^[(Bukasz Stelmach <stlman@poczta.fm>. + 2013-02-14 Katsumi Yamaoka <yamaoka@jpl.org> * gnus-util.el (gnus-define-keys): Convert [?\S-\ ] to [(shift space)] diff --git a/lisp/mml2015.el b/lisp/mml2015.el index 275a486..6310ff3 100644 --- a/lisp/mml2015.el +++ b/lisp/mml2015.el @@ -786,21 +786,47 @@ If set, it overrides the setting of `mml2015-sign-with-sender'." (cons password-cache-key-id mml2015-epg-secret-key-id-list)) (copy-sequence passphrase))))) -(defun mml2015-epg-find-usable-key (keys usage) - (catch 'found +(defun mml2015-epg-check-user-id (key recipient) + (let ((pointer (epg-key-user-id-list key)) + result) + (while pointer + (if (and (equal (car (mail-header-parse-address + (epg-user-id-string (car pointer)))) + (car (mail-header-parse-address + recipient))) + (not (memq (epg-user-id-validity (car pointer)) + '(revoked expired)))) + (setq result t + pointer nil) + (setq pointer (cdr pointer)))) + result)) + +(defun mml2015-epg-check-sub-key (key usage) + (let ((pointer (epg-key-sub-key-list key)) + result) + ;; The primary key will be marked as disabled, when the entire + ;; key is disabled (see 12 Field, Format of colon listings, in + ;; gnupg/doc/DETAILS) + (unless (memq 'disabled (epg-sub-key-capability (car pointer))) + (while pointer + (if (and (memq usage (epg-sub-key-capability (car pointer))) + (not (memq (epg-sub-key-validity (car pointer)) + '(revoked expired)))) + (setq result t + pointer nil) + (setq pointer (cdr pointer))))) + result)) + +(defun mml2015-epg-find-usable-key (context recipient usage) + (let ((keys (epg-list-keys context recipient)) + key) (while keys - (let ((pointer (epg-key-sub-key-list (car keys)))) - ;; The primary key will be marked as disabled, when the entire - ;; key is disabled (see 12 Field, Format of colon listings, in - ;; gnupg/doc/DETAILS) - (unless (memq 'disabled (epg-sub-key-capability (car pointer))) - (while pointer - (if (and (memq usage (epg-sub-key-capability (car pointer))) - (not (memq (epg-sub-key-validity (car pointer)) - '(revoked expired)))) - (throw 'found (car keys))) - (setq pointer (cdr pointer))))) - (setq keys (cdr keys))))) + (if (and (mml2015-epg-check-user-id (car keys) recipient) + (mml2015-epg-check-sub-key (car keys) usage)) + (setq key (car keys) + keys nil) + (setq keys (cdr keys)))) + key)) ;; XXX: since gpg --list-secret-keys does not return validity of each ;; key, `mml2015-epg-find-usable-key' defined above is not enough for @@ -1115,8 +1141,7 @@ If no one is selected, symmetric encryption will be performed. " (mapcar (lambda (recipient) (setq recipient-key (mml2015-epg-find-usable-key - (epg-list-keys context recipient) - 'encrypt)) + context recipient 'encrypt)) (unless (or recipient-key (y-or-n-p (format "No public key for %s; skip it? " -- 1.8.1.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-15 4:05 ` Daiki Ueno @ 2013-02-15 7:59 ` Łukasz Stelmach 2013-02-15 9:14 ` Daiki Ueno 2013-02-16 18:35 ` Łukasz Stelmach 1 sibling, 1 reply; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-15 7:59 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 1380 bytes --] Daiki Ueno <ueno@gnu.org> writes: > Łukasz Stelmach <stlman@poczta.fm> writes: > >> It looks like there is a sort of bug in mml2015-epg-encrypt at line >> 1117. >> >> http://git.gnus.org/cgit/gnus.git/tree/lisp/mml2015.el?id=6e422932976e94d6dbb7ad602c5be3eb481fe46c#n1117) >> >> The whole form that begins at line 1113 converts list of recipients >> provided as email addresses, key IDs or anything gpg can digest into epg >> "objects". Unfortunately there is one corner case that makes this code >> fail. That is when a key is valid but a uid on it has been revoked. > > Thanks for the report. > >> epg.el is a part of emacs, mml2015.el belongs to gnus. IMHO it is >> mml2015-epg-find-usable-key that should be fixed. The function should >> accept and additional (optional?) parameter with a recipient uid as >> passed to epg-list-keys and reject keys with all uids matching the >> recipient parameter revoked. > > Sounds reasonable. I plan to apply the attached patch. The patch looks reasonable (-; I'll check tooday in the evening (CET). Before applying please follow the discussion (if it appears) on the gpg-devel mailing list: http://permalink.gmane.org/gmane.comp.encryption.gpg.devel/17572 Of course if a fix appears in gpg 2.0.20 your code is still relevant for thos who use older versions. -- Miłego dnia, Łukasz Stelmach [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-15 7:59 ` Łukasz Stelmach @ 2013-02-15 9:14 ` Daiki Ueno 0 siblings, 0 replies; 12+ messages in thread From: Daiki Ueno @ 2013-02-15 9:14 UTC (permalink / raw) To: =?iso-2022-jp-2?B?GyQoRCkoGyhCdWthc3o=?= Stelmach; +Cc: ding [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=iso-2022-jp-2, Size: 578 bytes --] ^[$(D)(^[(Bukasz Stelmach <lukasz.stelmach@iem.pw.edu.pl> writes: > The patch looks reasonable (-; I'll check tooday in the evening > (CET). Before applying please follow the discussion (if it appears) on > the gpg-devel mailing list: > > http://permalink.gmane.org/gmane.comp.encryption.gpg.devel/17572 I remember that the GPG author originally suggested the current code ;-) http://thread.gmane.org/gmane.emacs.gnus.general/64349 But let's see how it goes. Perhaps some built-in key filtering feature in GPG (or GPGME) might be generally useful. Regards, -- Daiki Ueno ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-15 4:05 ` Daiki Ueno 2013-02-15 7:59 ` Łukasz Stelmach @ 2013-02-16 18:35 ` Łukasz Stelmach 2013-02-16 21:11 ` Łukasz Stelmach 1 sibling, 1 reply; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-16 18:35 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 1893 bytes --] Daiki Ueno <ueno@gnu.org> writes: > Łukasz Stelmach <stlman@poczta.fm> writes: > >> It looks like there is a sort of bug in mml2015-epg-encrypt at line >> 1117. >> >> http://git.gnus.org/cgit/gnus.git/tree/lisp/mml2015.el?id=6e422932976e94d6dbb7ad602c5be3eb481fe46c#n1117) >> >> The whole form that begins at line 1113 converts list of recipients >> provided as email addresses, key IDs or anything gpg can digest into epg >> "objects". Unfortunately there is one corner case that makes this code >> fail. That is when a key is valid but a uid on it has been revoked. > > Thanks for the report. > >> epg.el is a part of emacs, mml2015.el belongs to gnus. IMHO it is >> mml2015-epg-find-usable-key that should be fixed. The function should >> accept and additional (optional?) parameter with a recipient uid as >> passed to epg-list-keys and reject keys with all uids matching the >> recipient parameter revoked. > > Sounds reasonable. I plan to apply the attached patch. The patch needed a fix: there was a space before tab and my name for the changelog has been messed up (maybe due to encoding issues?). See the first attachment. There are two more issues: 1. mml2015-find-usable-key is called from mml2015-epg-find-usable-secret-key. Your patch hasn't changed the invocation. See the second attachment. 2. IMHO mml2015-epg-check-user-id should return true if a key id is passed as a uid to check. I assume that if a caller passes id to mml2015-find-usable-key find See the third attachment. All atachments has been generated with git-format-patch and are ready to apply with git-am onto 98f5834. I've got some more patches for mml2015-epg-* but let's concentrate on the revoked uid handling. PS. It looks like this massage originally hasn't made it to the group (because of attachments?) -- Miłego dnia, Łukasz Stelmach [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-16 18:35 ` Łukasz Stelmach @ 2013-02-16 21:11 ` Łukasz Stelmach 2013-02-17 3:12 ` Daiki Ueno 0 siblings, 1 reply; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-16 21:11 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 1414 bytes --] Daiki Ueno <ueno@gnu.org> writes: > Łukasz Stelmach <stlman@poczta.fm> writes: > >> The patch needed a fix: there was a space before tab and my name for the >> changelog has been messed up (maybe due to encoding issues?). > > Perhaps it is because I didn't use attachment, but just inserted the > patch into the mail. It will be fixed in the git commit. > >> There are two more issues: >> >> 1. mml2015-find-usable-key is called from >> mml2015-epg-find-usable-secret-key. Your patch hasn't changed the >> invocation. > > Oops, right. Thanks for pointing this out. > >> 2. IMHO mml2015-epg-check-user-id should return true if a key id is >> passed as a uid to check. I assume that if a caller passes id to >> mml2015-find-usable-key find > > Yes, > >> + (string-match >> + "^\\(0x\\)?[[:xdigit:]]\\{8\\}\\([[:xdigit:]]\\{8\\}\\)?$" >> + recipient)) > > I think this can be simplified to: > > (not (string-match "\\`<" recipient)) > > since all the mml2015-epg-* functions normalize recipient addresses in > the form of "<foo@bar>", so they only match email addresses. Indeed, however I still think we should check this the hard way: return t if and *only* if it is a key-id. The simpler check *might* result in information disclosure if a message is encrypted to a revoked uid. PS. This is a followup to some direct e-mails. -- Miłego dnia, Łukasz [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-16 21:11 ` Łukasz Stelmach @ 2013-02-17 3:12 ` Daiki Ueno 2013-02-17 7:47 ` Łukasz Stelmach 0 siblings, 1 reply; 12+ messages in thread From: Daiki Ueno @ 2013-02-17 3:12 UTC (permalink / raw) To: Łukasz Stelmach; +Cc: ding Łukasz Stelmach <stlman@poczta.fm> writes: >>> + (string-match >>> + "^\\(0x\\)?[[:xdigit:]]\\{8\\}\\([[:xdigit:]]\\{8\\}\\)?$" >>> + recipient)) >> >> I think this can be simplified to: >> >> (not (string-match "\\`<" recipient)) >> >> since all the mml2015-epg-* functions normalize recipient addresses in >> the form of "<foo@bar>", so they only match email addresses. > > Indeed, however I still think we should check this the hard way: return > t if and *only* if it is a key-id. The simpler check *might* result in > information disclosure if a message is encrypted to a revoked uid. Sorry, I don't quite understand here. What's the senario you are thinking of? I think the only case RECIPIENT doesn't start with "<" is, a user sets mml2015-signers manually. How does it cause information disclosure? Regards, -- Daiki Ueno ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-17 3:12 ` Daiki Ueno @ 2013-02-17 7:47 ` Łukasz Stelmach 2013-02-17 9:02 ` Daiki Ueno 0 siblings, 1 reply; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-17 7:47 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 1626 bytes --] Daiki Ueno <ueno@gnu.org> writes: > Łukasz Stelmach <stlman@poczta.fm> writes: > >>>> + (string-match >>>> + "^\\(0x\\)?[[:xdigit:]]\\{8\\}\\([[:xdigit:]]\\{8\\}\\)?$" >>>> + recipient)) >>> >>> I think this can be simplified to: >>> >>> (not (string-match "\\`<" recipient)) >>> >>> since all the mml2015-epg-* functions normalize recipient addresses in >>> the form of "<foo@bar>", so they only match email addresses. >> >> Indeed, however I still think we should check this the hard way: return >> t if and *only* if it is a key-id. The simpler check *might* result in >> information disclosure if a message is encrypted to a revoked uid. > > Sorry, I don't quite understand here. What's the senario you are > thinking of? I think the only case RECIPIENT doesn't start with "<" is, > a user sets mml2015-signers manually. How does it cause information > disclosure? It should not be a problem in case of mml2015-signers (I have analysed this case thoroughly). However, I don't like mml2015-signers work now, an have some ideas (and even some code) to improve it. What I mean, here is a rather hypothetical case when someone writes some code forgetting about your assumption of normalisation. We can say: "let the guy shoot his own foot", but I think we shouldn't. We should rather follow the logic: assume key-id is a correct uid. In such case we should check the input for *being* the key-id rather than not being an e-mail address. I am not a security expert but this seems more security-wise correct and future proof to me. -- Miłego dnia, Łukasz Stelmach [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-17 7:47 ` Łukasz Stelmach @ 2013-02-17 9:02 ` Daiki Ueno 2013-02-17 10:20 ` Daiki Ueno 0 siblings, 1 reply; 12+ messages in thread From: Daiki Ueno @ 2013-02-17 9:02 UTC (permalink / raw) To: Łukasz Stelmach; +Cc: ding Łukasz Stelmach <stlman@poczta.fm> writes: > What I mean, here is a rather hypothetical case when someone writes > some code forgetting about your assumption of normalisation. We can > say: "let the guy shoot his own foot", but I think we shouldn't. We > should rather follow the logic: assume key-id is a correct uid. In > such case we should check the input for *being* the key-id rather than > not being an e-mail address. In that case, I'd rather modify the caller of mml2015-epg-check-user-id, not to call the function if the recipient is key-id. That makes it clearer that mml2015-epg-check-user-id only deals with user-id not key-id. Regards, -- Daiki Ueno ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-17 9:02 ` Daiki Ueno @ 2013-02-17 10:20 ` Daiki Ueno 2013-02-17 21:29 ` Łukasz Stelmach 0 siblings, 1 reply; 12+ messages in thread From: Daiki Ueno @ 2013-02-17 10:20 UTC (permalink / raw) To: Łukasz Stelmach; +Cc: ding Daiki Ueno <ueno@gnu.org> writes: > In that case, I'd rather modify the caller of mml2015-epg-check-user-id, > not to call the function if the recipient is key-id. That makes it > clearer that mml2015-epg-check-user-id only deals with user-id not > key-id. Pushed a fix in that way. Regards, -- Daiki Ueno ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] mml2015-epg-find-usable-key finds unusable key 2013-02-17 10:20 ` Daiki Ueno @ 2013-02-17 21:29 ` Łukasz Stelmach 0 siblings, 0 replies; 12+ messages in thread From: Łukasz Stelmach @ 2013-02-17 21:29 UTC (permalink / raw) To: ding [-- Attachment #1: Type: text/plain, Size: 585 bytes --] Daiki Ueno <ueno@gnu.org> writes: > Daiki Ueno <ueno@gnu.org> writes: > >> In that case, I'd rather modify the caller of mml2015-epg-check-user-id, >> not to call the function if the recipient is key-id. That makes it >> clearer that mml2015-epg-check-user-id only deals with user-id not >> key-id. > > Pushed a fix in that way. That's great. I traveled a bit today, and will do some more soon too, but hopefully I will find some time to polish and present the patches for the mml2015-epg-encrypt function I have in my .gnus.el -- Kind regards, Łukasz Stelmach [-- Attachment #2: Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-02-17 21:29 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-02-13 19:55 [BUG] mml2015-epg-find-usable-key finds unusable key Łukasz Stelmach 2013-02-14 19:28 ` Łukasz Stelmach 2013-02-15 4:05 ` Daiki Ueno 2013-02-15 7:59 ` Łukasz Stelmach 2013-02-15 9:14 ` Daiki Ueno 2013-02-16 18:35 ` Łukasz Stelmach 2013-02-16 21:11 ` Łukasz Stelmach 2013-02-17 3:12 ` Daiki Ueno 2013-02-17 7:47 ` Łukasz Stelmach 2013-02-17 9:02 ` Daiki Ueno 2013-02-17 10:20 ` Daiki Ueno 2013-02-17 21:29 ` Łukasz Stelmach
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).