dirmngr should not apply Kristian's CA when fetching from a keyserver that is not `hkps.pool.sks-keyservers.net`
Open, LowPublic

Description

the current code in dirmngr applies @kristianf's CA whenever the keyserver hostname is the default hostname.

But if the default ever changes away from hkps.pool.sks-keyservers.net (e.g. if a distributor patches it), then it is inappropriate to permit @kristianf's CA to authenticate it.

Details

Version
2.2.16
dkg created this task.Jun 30 2019, 6:09 PM
dkg updated the task description. (Show Details)
dkg added a comment.Jun 30 2019, 6:12 PM

I've just pushed 1c9cc97e9d47d73763810dcb4a36b6cdf31a2254 to the branch dkg-fix-T4593

werner closed this task as Wontfix.Jul 1 2019, 9:50 AM
werner claimed this task.
werner added a subscriber: werner.

I see no need for this.

dkg reopened this task as Open.Jul 1 2019, 6:31 PM

If the default keyserver is not hkps.pool.sks-keyservers.net, then @kristianf's CA certificate has no business certifying it.

It would be a security vulnerability if we were to unilaterally add, for example, Turktrust's CA to the list of trusted authorities in some circumstances that did not reflect any specific knowledge that we expect Turktrust to be the authority of record.

As such, this is also a security vulnerability. it is especially relevant as we are starting to see distros patch out the default keyserver, and we already have one instance of them doing that without noticing that they'd left open this gap.

I'm reopening this because it is clearly the right thing to do to avoid further problems, but I'm frankly tired of fighting over every change, especially in cases like this when you offer no explanation for rejecting what is a small, obviously correct fix.

I'm trying to help this project be useful and safe for more people. Do you want that help or do you want me to give up?

werner added a comment.Jul 1 2019, 8:05 PM

Come on, if someone changes the software and breaks it, it is their's fault ant not ours. The whole thing on which keyserver and certificate to use as been discussed ad nausea in the past. Given all the problems with the keyservers I do not see a reason to change it right away to a state we had before. Keyserver code is pretty hard to test and has thus always been prone to regressions.

(I guess we should have a phone call)

dkg added a comment.Jul 1 2019, 9:03 PM

We're writing free software, which we know that people use and modify downstream. if we know that the software has a particular sharp edge that people who are modifying it are likely to cut themselves on, we have two options:

  • leave the sharp edge in place and say "you broke it you get to keep both pieces"
  • sand it down so that it's unlikely to cut someone again in that particular way.

One of these choices seems more responsible than the other.

In our community, upstream is widely considered the authoritative source for a project. Adoption of this security fix by upstream (even before a release) signals to any downstream that this is a correct and sensible modification. Rejection of the fix by upstream is likely to cast some doubt for some downstreams as to whether they should use it, despite its being correct. I can't imagine why an long-standing free software project with an experienced lead developer who is interested in responsible maintenance would reject this.

I did my best here to reduce the size and scope of the change so that it had nothing that could possibly be objectionable.

For example, i did not include changes that would have made the default keyserver settable with ./configure --with-default-keyserver=$foo, because it had clearly not been done that way originally (i don't know why) and i didn't want an argument about whether it was appropriate as a ./configure option to distract from getting the specific security fix in place.

Nevertheless, it turned into an argument. This is unfortunately a pattern with working in this project, and i don't think i'm the only one who has experienced it. I don't know whether i am lucky or unlucky that i am stubborn enough to persist in trying to fix things here.

I'm willing to have a call if you're interested in having other contributors on this project.

dkg added a subscriber: Valodim.EditedJul 1 2019, 9:13 PM

I should add that i don't really care whose fault it is if the software is broken by some downstream. if it harms any users, and we can fix it, we should fix it, especially if the fix is easy.

But please, @werner, don't comment further on this (relatively minor) bug report. T4393 is significantly more important, has even more of a security impact on users (as it affects ability to retrieve revocations from uid-less keystores), and has seen no response from you or anyone else upstream since @Valodim sent his DCO to the list nearly two weeks ago. If you're going to spend any energy responding to security concerns or to external contributions, please prioritize a response to T4393.

werner triaged this task as Low priority.Jul 1 2019, 9:18 PM
georg added a subscriber: georg.Jul 2 2019, 8:11 AM
tianon added a subscriber: tianon.Jul 3 2019, 9:34 PM
dkg added a comment.Jul 16 2019, 8:08 PM

Just a note that we're now shipping this patch in debian unstable. It would be great if it was merged upstream.