[Sks-devel] SKS should not accept or propagate User IDs with no self-sigs [was: SKS should not accept or replay non-exportable certifications]

[Daniel Kahn Gillmor]

Hi John, all--

On 09/14/2013 09:46 PM, John Clizbe wrote:
> Careful here. Gossip is referring to recon and there is no other option in
> recon. Keys are just blobs of bits with a hash value.  We can control what we
> accept, to try and make it conform to RFC 4880, et alia,... as much as
> possible, and we can control what is delivered by the server by cleaning and
> or filtering. The second is a dangerous approach because we are then
> controlling what is delivered, i.e., we're censoring.

Right, i'm not arguing for censorship here; i'm arguing for only
accepting standard, well-formed data that is intended for the
keyservers.  So while OpenPGP public key data, user IDs, user
attributes, and exportable certifications all fall in this category, the
keyserver network should not accept (or try to propagate) any of: an
S/MIME certificate; OpenPGP-encrypted messages; a certification
signature packet explicitly marked as "non-exportable"; OpenPGP data
signatures; OpenPGP secret key material; OpenPGP certificates that do
not contain the right kind and sequence of packets (e.g. no User ID);
and so forth.

> I think the task of
> verifying what is acceptable after a key is requested is probably best left to
> the client OpenPGP implementations.

I completely agree that HKP client applications have a responsibility to
filter data they receive over the network (even if all keyservers worked
absolutely perfectly, the clients would have that responsibility because
of potential network compromise).

That this responsibility inheres in the HKP client doesn't imply that
the keyservers themselves should be willing to propagate malformed data,
(including data that has been clearly marked by the author as being not
intended for propagation on the keyservers).

> 1) Dan signs John's key with lsign and then keyserver/export options with
> export-local-sigs and sends the key with local-sigs to the keyserver network
> where the lsigs are accepted. This is clearly an error and these sigs should
> be removed when a key is submitted containing them. On a get, we have to make
> the call, do we enforce or put it on the client. I leave that for discussion.

clearly i think that such data should neither propagate on the
keyservers nor be accepted or transmitted by the clients.  Both sides of
the transaction should be actively filtering to minimize unwanted data
leakage.

> 2) JimBob lsigns his own key, creating a non-exportable selfsig then delsigs
> all of the exportable selfsigs.  This is shooting oneself in the foot. If we
> honor no-export on a selfsig, we create keys with UIDs that have no binding
> signature. THIS IS VERY VERY BAD. I think the RFC folks should probably have
> been more explicit on this case, but to be fair, it's probably a use case they
> did not anticipate.
> 
> My compromise suggestion of trying to DTRT but with minimum harm is in the
> case of 1, where signing key != signed key, strip the non-exportable sig
> before we import into the key store.
> 
> In the case of 2, where signing key == signed key (lsign your own key) we have
> a user either intentionally or accidentally shooting himself in the crypto
> foot. We can a) hold our noses and accept the key, or b) reject the entire key
> as malformed -- there is no way to honor the no-export sig flag and still have
> a valid key.
> 
> Another possibility is that if there are earlier or later exportable
> selfsig(s), just strip the errant selfsig with the no-export flag.

I favor (b), but getting that to happen would require SKS to actually
reject OpenPGP User IDs which have no selfsigs.  This is not currently
the case for sks 1.1.4.

I believe the attached patch (also pushed to
https://bitbucket.org/dkgdkg/sks-keyserver/) implements this additional
verification.  Again, my ocaml is in its infancy, so i would welcome any
sanity checking, and any advice about what i could do better in the code.

(there is one other fix published in my bitbucket hg repository which is
a minor documentation cleanup).

Please let me know what you think about these two changes.

Regards,

        --dkg

changeset:   358:0b577212aab4
tag:         tip
user:        address@hidden
date:        Tue Sep 17 16:40:47 2013 -0400
summary:     Only process User IDs that have a self-sig on them

diff -r 780fe1f766c6 -r 0b577212aab4 Makefile
--- a/Makefile  Tue Sep 17 14:37:15 2013 -0400
+++ b/Makefile  Tue Sep 17 16:40:47 2013 -0400
@@ -80,8 +80,8 @@
 ROBJS=$(ROBJS.bc:.cmo=.cmx)
 
 OBJS.bc=packet.cmo parsePGP.cmo sStream.cmo bdbwrap.cmo \
-       key.cmo keyHash.cmo keyMerge.cmo fixkey.cmo \
-       fingerprint.cmo keydb.cmo armor.cmo \
+       key.cmo keyHash.cmo fingerprint.cmo keyMerge.cmo fixkey.cmo \
+       keydb.cmo armor.cmo \
        dbMessages.cmo htmlTemplates.cmo wserver.cmo \
        membership.cmo tester.cmo request.cmo \
        stats.cmo index.cmo mRindex.cmo pTreeDB.cmo \
diff -r 780fe1f766c6 -r 0b577212aab4 keyMerge.ml
--- a/keyMerge.ml       Tue Sep 17 14:37:15 2013 -0400
+++ b/keyMerge.ml       Tue Sep 17 16:40:47 2013 -0400
@@ -245,12 +245,24 @@
          )
   in
   Map.to_alist map
+    
+let sig_has_issuer = fun issuer sigx ->
+    match (ParsePGP.parse_signature sigx) with
+      V3sig s -> issuer = s.v3s_keyid
+    | V4sig s -> List.exists
+          (fun ssp -> 
+            ssp.ssp_type = 16 &&
+            ssp.ssp_length = 8 &&
+            ssp.ssp_body = issuer)
+          (s.v4s_hashed_subpackets @ s.v4s_unhashed_subpackets)
 
+let has_sig_from = fun key uid ->
+  List.exists (sig_has_issuer (Fingerprint.from_packet key).Fingerprint.keyid) 
(snd uid)
 
 let dedup_pkey pkey =
   { pkey with
       selfsigs = Utils.dedup pkey.selfsigs;
-      uids = dedup_sigpairs pkey.uids;
+      uids = List.filter (fun u -> has_sig_from pkey.key u) (dedup_sigpairs 
pkey.uids);
       subkeys = dedup_sigpairs pkey.subkeys;
   }

signature.asc
Description: OpenPGP digital signature