Thread (7 messages) flat view 7 messages, 3 authors, 2021-10-11

Re: [PATCH] http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors

From: Jeff King <hidden>
Date: 2021-10-11 16:12:30

On Mon, Oct 11, 2021 at 03:23:02PM +0200, Ævar Arnfjörð Bjarmason wrote:
quoted
So it's supposed to be

  ... a http.pinnedPubKey doesn't point to the http.pinnedPubKey
  variable ...

?  I still have no idea because of the repeated config variable name.
We emit this currently:

    $ git -c http.pinnedPubKey=sha256/someNonMatchingKey ls-remote https://github.com/git/git.git
    fatal: unable to access 'https://github.com/git/git.git/': SSL: public key does not match pinned public key!

And with this change, this:

    $ git -c http.pinnedPubKey=sha256/someNonMatchingKey ls-remote https://github.com/git/git.git
    fatal: unable to access 'https://github.com/git/git.git/' with http.pinnedPubkey configuration: SSL: public key does not match pinned public key!

I.e. this replaces a generic error message from curl with something that
points the user at the config variable in question.
FWIW, I too had to stare at the commit message when I first read it.
Perhaps:

  When curl gives us an error related to http.pinnedPubKey, we pass
  along the error from curl: "public key does not match pinned public
  key". But we do not mention the http.pinnedPubKey config, so the user
  may not realize where to start looking to address this.

As you say, this is already in next, so it's too late. So just thoughts
for next time (I find this "we do X, but the problem is Y" explanation
is often more clear than "change Z", because it makes the motivation
explicit).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help