Commit pushed to master with unauthorised signature

Commit pushed to master with unauthorised signature

[Tobias Geerinckx-Rice]

[-- Attachment #1: Type: text/plain, Size: 3389 bytes --]

Guix,

I have very little time to write a proper post-mortem.  Luckily, 
thanks to the prompt help of rwp of #savannah fame and Ludo's sane 
‘guix pull’ design, there's not much to report, although there's 
something to improve.

Despite the scary title, at no point did anything untoward or 
malicious happen.  Users were not at risk.

Earlier today the following commit was pushed to master:

--8<---------------cut here---------------start------------->8---
commit 15092548804b6c50ea276d098f76a79bd0042398
gpg: Signature made Wed Mar 10 19:55:39 2021 CET
gpg:                using RSA key 
51A0982A58B64622464833085EEB3986CB2F65ED
gpg: Good signature from "Taylan Kammer (Debian10VM) 
<taylan.kammer@gmail.com>" [unknown]
Primary key fingerprint: 51A0 982A 58B6 4622 4648  3308 5EEB 3986 
CB2F 65ED
Author: Taylan Kammer <taylan.kammer@gmail.com>

    gnu: guile-bytestructures: Update to 1.0.10.

    * gnu/packages/guile.scm (guile-bytestructures): Update to 
    1.0.10.
--8<---------------cut here---------------end--------------->8---

The key with fingerprint 51A0 982A 58B6 4622 4648  3308 5EEB 3986 
CB2F 65ED is not present in .guix-authorizations, nor in the 
‘keyring’ branch.  This broke ‘guix pull’ for all users[0]:

--8<---------------cut here---------------start------------->8---
guix pull: error: could not authenticate commit 
15092548804b6c50ea276d098f76a79bd0042398: key 51A0 982A 58B6 4622 
4648 3308 5EEB 3986 CB2F 65ED is missing
--8<---------------cut here---------------end--------------->8---

The only solution to that is to remove the offending commit 
upstream.  Our Savannah git repository does not allow deleting or 
force-pushing master for safety reasons.  Helpful Bob Proulx of 
the Savannah team manually reset the remote master branch back to 
the previous[1] commit.

I have pushed Taylan's commit as 
b1eb7448370bbd4d494cf9f3fddae88dd0de2ca3, signed with my own key.

The good news is that ‘guix pull’ commit authentication has passed 
real-world testing, and that the mess was relatively transparent 
to users: ‘guix pull’ continues to work without extra options, 
even for those who pulled between 150925 and b1eb74 and got a 
scary error.

The less-good news is that our remote git hook should never have 
allowed this to happen in the first place, and that this weakness 
has been known for... well, a while[2].  Any committer can DoS 
guix pull in a way that even the maintainers can't fix unaided.

This also highlights the fact that many people[3] are currently 
unconditionally trusted with commit access.  This includes 
‘currently inactive members’: Savannah has no way to disable or 
even restrict commit access (to specific branches, subdirectories, 
or even repositories(?)) without removing membership altogether. 
The chance of mistakes, key confusion, forgotten commit privileges 
grows.

lfam has started removing certain inactive people from this list, 
but removing people is not a fun job nor something one proactive 
volunteer should be tasked with alone.

Kind regards,

T G-R

[0]: https://logs.guix.gnu.org/guix/2021-03-10.log#205043
[1]: 60174c9c8c307be43450af38ce7c4e268278e07c,
[2]: 
https://savannah.nongnu.org/support/?func=detailitem&item_id=109104
[3]: https://savannah.gnu.org/project/memberlist.php?group=guix

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

Re: Commit pushed to master with unauthorised signature

[Taylan Kammer]

On 10.03.2021 22:22, Tobias Geerinckx-Rice wrote:

> Earlier today the following commit was pushed to master:
> 
> --8<---------------cut here---------------start------------->8---
> commit 15092548804b6c50ea276d098f76a79bd0042398
> gpg: Signature made Wed Mar 10 19:55:39 2021 CET
> gpg:                using RSA key 51A0982A58B64622464833085EEB3986CB2F65ED
> gpg: Good signature from "Taylan Kammer (Debian10VM)
> <taylan.kammer@gmail.com>" [unknown]
> Primary key fingerprint: 51A0 982A 58B6 4622 4648  3308 5EEB 3986 CB2F 65ED
> Author: Taylan Kammer <taylan.kammer@gmail.com>
> 
>    gnu: guile-bytestructures: Update to 1.0.10.
> 
>    * gnu/packages/guile.scm (guile-bytestructures): Update to    1.0.10.
> --8<---------------cut here---------------end--------------->8---
> 
> The key with fingerprint 51A0 982A 58B6 4622 4648  3308 5EEB 3986 CB2F
> 65ED is not present in .guix-authorizations, nor in the ‘keyring’
> branch.  This broke ‘guix pull’ for all users[0]:
> 
> --8<---------------cut here---------------start------------->8---
> guix pull: error: could not authenticate commit
> 15092548804b6c50ea276d098f76a79bd0042398: key 51A0 982A 58B6 4622 4648
> 3308 5EEB 3986 CB2F 65ED is missing
> --8<---------------cut here---------------end--------------->8---

Damn, sorry about that.  I assumed of course that an improperly signed
commit would not be accepted, so I didn't pay any special mind.

However, I also assumed that adding a new GPG key to my savannah.gnu.org
account would be sufficient.  I did that via the web interface, and
ensured that the encryption test is successful.  The commit is signed
with that new GPG key.

Are the GPG keys added to one's Savannah account unrelated to commit
signing in the Guix repo, or are they not automatically synced, or is
this a further bug?..


- Taylan

Re: Commit pushed to master with unauthorised signature

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

On Thu, 2021-03-11 at 00:15 +0100, Taylan Kammer wrote:
> [...]
> Damn, sorry about that.  I assumed of course that an improperly signed
> commit would not be accepted, so I didn't pay any special mind.
> 
> However, I also assumed that adding a new GPG key to my savannah.gnu.org
> account would be sufficient.

"guix pull" only looks at the git repo (the .guix-authorizations file + the
keyring branch), and not anything else provided by savannah.  Doing so would
introduce an additional point where the "guix pull" mechanism could be
compromised.  The git repository could as well have been hosted at
$RANDOM_SPY_AGENCY or $RANDOM_FORGE.

(See ‘16.8 Commit Access’, ‘6.8 Specifying Channel Authorizations’ and
‘7.4 Invoking ‘guix git authenticate’’).

> Are the GPG keys added to one's Savannah account unrelated to commit
> signing in the Guix repo,

Yes (though they probably are same in practice).

> or are they not automatically synced,

Yes, they aren't.

> this a further bug?..

No, savannah is not ‘trusted’ beyond being online, as that would introduce
another point where "guix pull" could be compromised.

Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: Commit pushed to master with unauthorised signature

[Taylan Kammer]

On 11.03.2021 08:37, Maxime Devos wrote:
> On Thu, 2021-03-11 at 00:15 +0100, Taylan Kammer wrote:
>> [...]
>> Damn, sorry about that.  I assumed of course that an improperly signed
>> commit would not be accepted, so I didn't pay any special mind.
>>
>> However, I also assumed that adding a new GPG key to my savannah.gnu.org
>> account would be sufficient.
> 
> "guix pull" only looks at the git repo (the .guix-authorizations file + the
> keyring branch), and not anything else provided by savannah.  Doing so would
> introduce an additional point where the "guix pull" mechanism could be
> compromised.  The git repository could as well have been hosted at
> $RANDOM_SPY_AGENCY or $RANDOM_FORGE.
> 
> (See ‘16.8 Commit Access’, ‘6.8 Specifying Channel Authorizations’ and
> ‘7.4 Invoking ‘guix git authenticate’’).

Thanks, makes sense.

I'm hopping workstations recently, and my general habit is to create new
keys on each machine I'm using and register them where ever needed.
(E.g. .ssh/authorized_keys on machines I access, GitHub account, etc.)

I guess I shouldn't do that with Guix push access and instead keep a GPG
key on a USB drive or such.


- Taylan

Re: Commit pushed to master with unauthorised signature

[Tobias Geerinckx-Rice]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Taylan,

So if I needed to send you encrypted mail, I'd have to possess all 
of your current GPG keys and encrypt to all of them?  Thanks for 
the heads-up ;-)  I'm not sure if that's how GPG is supposed to 
work (‘who does’, you say? fair point).

I do know that UIDs like ‘Jessie Doe (professional)’ are 
discouraged because people signing your key would (according to 
GPG logic) be vouching that you are, in fact, professional.

Anyway, you still need to make sure that *all* of your keys are 
available on Savannah.  It seems they are but they've expired.

Taylan Kammer 写道：
> I'm hopping workstations recently, and my general habit is to 
> create new
> keys on each machine I'm using and register them where ever 
> needed.
> (E.g. .ssh/authorized_keys on machines I access, GitHub account, 
> etc.)

Makes good sense for SSH keys.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

Re: Commit pushed to master with unauthorised signature

[Julien Lepiller]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

Also, make sure to install the pre-push hook, it should not have let you commit without checking your commits were properly recognised.

Le 11 mars 2021 08:11:38 GMT-05:00, Taylan Kammer <taylan.kammer@gmail.com> a écrit :
>On 11.03.2021 08:37, Maxime Devos wrote:
>> On Thu, 2021-03-11 at 00:15 +0100, Taylan Kammer wrote:
>>> [...]
>>> Damn, sorry about that.  I assumed of course that an improperly
>signed
>>> commit would not be accepted, so I didn't pay any special mind.
>>>
>>> However, I also assumed that adding a new GPG key to my
>savannah.gnu.org
>>> account would be sufficient.
>> 
>> "guix pull" only looks at the git repo (the .guix-authorizations file
>+ the
>> keyring branch), and not anything else provided by savannah.  Doing
>so would
>> introduce an additional point where the "guix pull" mechanism could
>be
>> compromised.  The git repository could as well have been hosted at
>> $RANDOM_SPY_AGENCY or $RANDOM_FORGE.
>> 
>> (See ‘16.8 Commit Access’, ‘6.8 Specifying Channel Authorizations’
>and
>> ‘7.4 Invoking ‘guix git authenticate’’).
>
>Thanks, makes sense.
>
>I'm hopping workstations recently, and my general habit is to create
>new
>keys on each machine I'm using and register them where ever needed.
>(E.g. .ssh/authorized_keys on machines I access, GitHub account, etc.)
>
>I guess I shouldn't do that with Guix push access and instead keep a
>GPG
>key on a USB drive or such.
>
>
>- Taylan

[-- Attachment #2: Type: text/html, Size: 1980 bytes --]

Re: Commit pushed to master with unauthorised signature

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

On Thu, Mar 11, 2021 at 12:15:19AM +0100, Taylan Kammer wrote:
> Damn, sorry about that.  I assumed of course that an improperly signed
> commit would not be accepted, so I didn't pay any special mind.

The security model is based on the client-side, i.e. `guix pull`. That
way, we don't have to trust the Git repo. We do want to improve the repo
so that it's not possible to push commits signed with unauthorized keys,
but that hasn't been done yet.
  
> However, I also assumed that adding a new GPG key to my savannah.gnu.org
> account would be sufficient.  I did that via the web interface, and
> ensured that the encryption test is successful.  The commit is signed
> with that new GPG key.

Adding your key(s) to your Savannah account is a required step...

> Are the GPG keys added to one's Savannah account unrelated to commit
> signing in the Guix repo, or are they not automatically synced, or is
> this a further bug?..

... but, we have a new code authentication system, described in the
manual section Specifying Channel Authorizations:

https://guix.gnu.org/manual/en/html_node/Specifying-Channel-Authorizations.html

Basically, committers' keys must be added to the .guix-authorizations
file in the Git repo before their work will be accepted by `guix pull`.

We are really happy that you are pushing code again :)

When this issue popped up yesterday, I removed your commit access just
to avoid further broken commits. Concretely, this means that I removed
you from the Guix "group" on Savannah.

However, I want to re-add you as a committer. Please read the manual
sections Commit Access. Especially, the part about the pre-push Git
hook, which would have caught this issue before pushing.

https://guix.gnu.org/manual/en/html_node/Commit-Access.html

Let me know when you've read the updated committer workflow guidelines
and installed the pre-push Git hook, and we'll add your new key to
.guix-authorizations, re-add you to the Savannah group, and then we can
continue with our happy hacking :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Commit pushed to master with unauthorised signature

[Taylan Kammer]

On 11.03.2021 15:59, Tobias Geerinckx-Rice wrote:
> Taylan,
> 
> So if I needed to send you encrypted mail, I'd have to possess all of
> your current GPG keys and encrypt to all of them?  Thanks for the
> heads-up ;-)  I'm not sure if that's how GPG is supposed to work (‘who
> does’, you say? fair point).

Hah, good point.  Shows that I've never seriously used GPG before. :-)

I'll have to get used to the idea that I need to keep around a file
backed up on a physical medium that authenticates me.  My whole life
I've always just trusted my memory to keep safe those few passwords that
can't be reset via email.


- Taylan

Re: Commit pushed to master with unauthorised signature

[Taylan Kammer]

On 11.03.2021 20:16, Leo Famulari wrote:
> On Thu, Mar 11, 2021 at 12:15:19AM +0100, Taylan Kammer wrote:
>> Damn, sorry about that.  I assumed of course that an improperly signed
>> commit would not be accepted, so I didn't pay any special mind.
> 
> The security model is based on the client-side, i.e. `guix pull`. That
> way, we don't have to trust the Git repo. We do want to improve the repo
> so that it's not possible to push commits signed with unauthorized keys,
> but that hasn't been done yet.
>   
>> However, I also assumed that adding a new GPG key to my savannah.gnu.org
>> account would be sufficient.  I did that via the web interface, and
>> ensured that the encryption test is successful.  The commit is signed
>> with that new GPG key.
> 
> Adding your key(s) to your Savannah account is a required step...
> 
>> Are the GPG keys added to one's Savannah account unrelated to commit
>> signing in the Guix repo, or are they not automatically synced, or is
>> this a further bug?..
> 
> ... but, we have a new code authentication system, described in the
> manual section Specifying Channel Authorizations:
> 
> https://guix.gnu.org/manual/en/html_node/Specifying-Channel-Authorizations.html
> 
> Basically, committers' keys must be added to the .guix-authorizations
> file in the Git repo before their work will be accepted by `guix pull`.
> 
> We are really happy that you are pushing code again :)
> 
> When this issue popped up yesterday, I removed your commit access just
> to avoid further broken commits. Concretely, this means that I removed
> you from the Guix "group" on Savannah.
> 
> However, I want to re-add you as a committer. Please read the manual
> sections Commit Access. Especially, the part about the pre-push Git
> hook, which would have caught this issue before pushing.
> 
> https://guix.gnu.org/manual/en/html_node/Commit-Access.html
> 
> Let me know when you've read the updated committer workflow guidelines
> and installed the pre-push Git hook, and we'll add your new key to
> .guix-authorizations, re-add you to the Savannah group, and then we can
> continue with our happy hacking :)

Thanks for the kind explanation!  I'll get in touch when I'm not so out
of the loop anymore.  To be honest I was just "summoned" by a bug report
on guile-bytestructures and am otherwise still overloaded with work life
plus personal projects outside of free software.


- Taylan