New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Released
Owner: ----
Closed: Sep 26
Components:



Sign in to add a comment
link

Issue 7750: signed pushes do not seem to be enforced

Reported by webczat_...@poczta.onet.pl, Nov 13 2017

Issue description

I have gerrit 2.15-rc2.
I have receive.enableSignedPush set to true, and I have registered my public gpg key. In All-Projects I have both enable and receive signed push set to true.
I have created the test project, but when trying to push to it with --no-signed, push is allowed even though it probably should not be.
I tried to explicitly require signed pushes in the respective test project instead of All-Projects, same effect.
 

Comment 1 by logan@google.com, Nov 14 2017

Project Member
Components: -PolyGerrit Backend

Comment 2 by jrn@google.com, Nov 24 2017

Project Member
Components: -Backend ReceiveCommits docs
You can configure requiring signed push on a per-project basis: https://gerrit-review.googlesource.com/Documentation/config-project-config.html#receive.requireSignedPush.

Would a link from https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#receive.enableSignedPush to that setting have helped here? (Bonus points if you can suggest wording, or even better, a patch against Documentation/config-gerrit.txt. SUBMITTING_PATCHES has details on how that works.)

Comment 3 by jrn@google.com, Nov 24 2017

Project Member
Components: -docs
Hm, you already mentioned that you're tried to require signed pushes there. Do you have a copy of the project.config or a screenshot illustrating what happened?

Comment 4 by jrn@google.com, Nov 24 2017

Project Member
[receive] enableSignedPush gets set in etc/gerrit.config.
[receive] requireSignedPush gets set in project.config in refs/meta/config.

What is the output when you push with and without --signed?

Comment 5 by webczat_...@poczta.onet.pl, Nov 24 2017

hmmm I checked a few times (unless I did it wrong). both enable and require signed push are enabled in All-Project actually, but I also tried doing it explicitly on the specific child project. gerrit.enableSignedPush is true. but it doesn't require and also rejects signed pushes. and yes, it gives me ability to upload gpg keys, does that signify signed pushes being globally enabled, or not?

Comment 6 by webczat_...@poczta.onet.pl, Nov 24 2017

won't give you exact output because it would require testing it now. I did it once. When I tried to push without --signed, the push went through, and I don't have the push.gpgSign option set. When --signed was used, the server just rejected the push, as if signed pushes were disabled. It is still possible I made a mistake, but I still think that stopped working.

Comment 7 by jrn@google.com, Sep 16

Project Member
Status: AwaitingInformation (was: New)
> both enable and require signed push are enabled in All-Project actually

You need to set

  [receive]
    enableSignedPush = true

in your etc/gerrit.config.

Comment 8 by jrn@google.com, Sep 16

Project Member
Labels: Hotlist-SignedPush

Comment 9 by webczat_...@poczta.onet.pl, Sep 17

From what I see it is set.

Comment 10 by david.os...@gmail.com, Sep 17

Project Member
Status: Accepted (was: AwaitingInformation)
It doesn't work on master either.

It seems, that even though the hooks are added in SignedPushModule:

  if (ps.is(BooleanProjectConfig.REQUIRE_SIGNED_PUSH)) {
      hooks.add(SignedPushPreReceiveHook.Required.INSTANCE);
  }

e.g.: SignedPushPreReceiveHook instance is added to hooks, the hook
doesn't get called and that why a non-signed push is not checked
and rejected.

It seems that the hooks are added too late.
I the beginning only AsyncReceiveCommits preReceiveHook is set:

  receivePack.setPreReceiveHook(this);

Then from the ReceivePack.service() line: 280
this single preReceiveHook gets called: [1]

  preReceive.onPreReceive(this, filterCommands(Result.NOT_ATTEMPTED));

in the same call stack Worker instance is created and initialized,
and only then the other hooks (with SignedPushPreReceiveHook) gets added
and the same receivePack instance is mutated and hook chain is
assigned, but they don't get invoked any more: [2].

  rp.setPreReceiveHook(PreReceiveHookChain.newChain(hooks));

[1] http://paste.openstack.org/show/730202
[2] http://paste.openstack.org/show/730203

Comment 11 by david.os...@gmail.com, Sep 17

Project Member
The reason it works on stable-2.14 branch:

The hooks are initialized before the first hook is invoked:[1]:

  preReceive.onPreReceive(this, filterCommands(Result.NOT_ATTEMPTED));

[1] http://paste.openstack.org/show/730204

Comment 12 by david.os...@gmail.com, Sep 17

Project Member
I think that this change is related: [1]. I added a comment there.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/118292

Comment 13 by david.os...@gmail.com, Sep 17

Project Member
Status: Started (was: Accepted)

Comment 14 by david.os...@gmail.com, Sep 17

Project Member
Status: ChangeUnderReview (was: Started)
https://gerrit-review.googlesource.com/c/gerrit/+/196230

Comment 15 by david.os...@gmail.com, Sep 18

Project Member
Acceptance test is added to catch similar regression in future gerrit releases:

https://gerrit-review.googlesource.com/c/gerrit/+/196231

Comment 16 by david.os...@gmail.com, Sep 19

Project Member
Labels: Blocking-2.16

Comment 17 by david.os...@gmail.com, Sep 19

Project Member
Labels: Triaged-Yes Priority-1

Comment 18 by david.os...@gmail.com, Sep 26

Project Member
Status: Submitted (was: ChangeUnderReview)

Comment 19 by david.pu...@gmail.com, Sep 28

Labels: FixedIn-2.15.4

Comment 20 by david.pu...@gmail.com, Oct 1

Status: Released (was: Submitted)

Sign in to add a comment