search

goreleaser / nfpm

nFPM is Not FPM - a simple deb, rpm, apk, ipk, and arch linux packager written in Go
https://nfpm.goreleaser.com/
MIT License
2.17k stars 157 forks source link

NFPM cannot sign packages with only the subkey present #276

Closed ghost closed 3 years ago

ghost commented 3 years ago

Describe the bug

When attempting to sign packages with subkeys only (no main key present) the signing fails with the following error message:

release failed after 11.33s error=nfpm failed: signing error: armored detach sign: decoding armored PGP keyring: openpgp: unsupported feature: hash for S2K function: 0

This is most likely due to golang/go#23226

To Reproduce

Steps to reproduce the behavior:

builds:
nfpms:
  - rpm:
      signature:
        key_file: /tmp/signing.gpg
    deb:
      signature:
        key_file: /tmp/signing.gpg
    apk:
      signature:
        key_file: /tmp/signing.gpg

The key needs to be generated with the following steps:

$ gpg --full-generate-key
# Complete the key setup
$ gpg --edit-key test@example.com
gpg> addkey
# Complete adding the subkey
gpg> quit
Save changes? (y/N) y
$ gpg --list-keys --with-keygrip
$ rm -rf ~/.gnupg/private-keys-v1.d/ADD-MAIN-KEY-KEYGRIP-HERE.key
$ gpg -a --export-secret-subkey test@example.com >/tmp/signing.gpg

Expected behavior

The signing should either work with the GnuPG agent instead of using a file, or it should read the GnuPG key correctly.

Environment (please complete the following information):

Additional context:

You can verify that the key is affected by this issue by running gpg --list-packets KEYFILE.gpg and looking for the gnu-dummy S2K packet.

alephnull commented 3 years ago

I vaguely remember a bug with rpm that prevents it from using subkeys.

caarlos0 commented 3 years ago

cc/ @erikgeiser can you take a look?

erikgeiser commented 3 years ago

It seems like crypto/openpgp does not support gnu-dummy S2K packets https://github.com/golang/go/issues/13605. I don't think we can do something to fix it with our crypto/openpgp implementation. I am personally very opposed to GnuPG and don't want to enable it by building more infrastructure on top of it. So I won't be implementing a solution based on the GnuPG agent. If this is indeed a feature that is wanted and someone else is willing to do it I would still recommend against it but I won't stand in the way of course.

Could you please elaborate on your use case? Is it common to have a subkey without the main key (it does sound a bit weird, doesn't it?). When would you encounter such a situation?

ghost commented 3 years ago

@erikgeiser thank you for your response.

The use case is security: when you upload your GPG key to, say GitHub Actions, you are entering a risk of exposing that key, whether it is via a bug in GitHub, or via a badly written shell script. If that key is exposed you then need to actively ask your users to switch keys.

The alternative to this is using subkeys that can be revoked separately. You create a subkey, export it in such a way that the main secret key is not included and upload that subkey to GH Actions. In this case the subkey can be revoked separately without compromising the main key.

erikgeiser commented 3 years ago

So the main key has no special purpose other than link multiple revoked and unrevoked subkeys to the same identity? In this case I think your use case makes sense, but I wonder why this use case is implemented in such a tacky way by GnuPG. At least that's what it feels like to me. Judgin by the namegnu-dummy s2k, it seems like a GnuPG specific extension and not part of regular PGP.

Sorry for being so dismissive of a GnuPG agent based solution but I've done this multiple times and only had bad experiences. From abysmal error messages to "API" changes between the versions in multiple distros (if you can call some extra options on a binary an API) to the fact that the GnuPG agent fails ungracefully and intransparently when calling gpg simultaniously in less than 10 processes. I don't know if @caarlos0 or @djgilcrease maybe want to deal with that to support this use case but I've been burned too many times.

djgilcrease commented 3 years ago

So I see 3 options with this;

  1. create a PR to fix https://github.com/golang/go/issues/23226 or wait for someone else to.
  2. switch to https://github.com/keybase/go-crypto which is a hard fork of golang/crypto and has fixed this issue (but has not kept up with upstream).
  3. Say this is not supported.
  4. calling out to gpg is not an option since we desire zero dependencies.

This is the MR in keybase/go-crypto https://github.com/keybase/go-crypto/pull/10 that fixed the issue, though a quick test of this seems to cause a panic when something tries to use the function returned by Parse since it is nil

@janoszen could you test https://github.com/goreleaser/nfpm/tree/test-fix-276 with your key this is pulling in a potential fix for https://github.com/golang/go/issues/23226 @ https://github.com/djgilcrease/crypto/commit/24fd8f26b469b040c76ed490eec3d40c9526c888

ghost commented 3 years ago

Hey @djgilcrease thanks for the quick response.

Option 1 is a non-option I think. I have a PR open to fix a bug in the SSH library since June and the response I got was that the SSH library has no owner, so nobody will really look at it. The same is true for the OpenPGP library. This bears the question how much we can rely on these libraries since non-security issues don't seem to get fixed even when there's a patch available.

Option 2 is an option, the OpenPGP package is fairly reasonable in size, but it would still mean a maintenance effort. Maybe we could extract enough functionality to make this work without needing to fork everything.

Option 3 would make me a sad panda and not represent best practices in terms of security.

I tested your fix by recompiling the goreleaser source with nfpm from your branch but it didn't work, same error. I've sent in a PR with a test you can verify against.

djgilcrease commented 3 years ago

@janoszen Ok I pulled your tests in and got it working to sign (with changes to both crypto and nfpm), though it fails with

    pgp_test.go:42:
                Error Trace:    pgp_test.go:42
                Error:          Received unexpected error:
                                openpgp: signature made by unknown entity
                Test:           TestPGPSignerAndVerify/gpg_subkey_armored_unprotected
    pgp_test.go:45:
                Error Trace:    pgp_test.go:45
                Error:          Received unexpected error:
                                openpgp: signature made by unknown entity
                Test:           TestPGPSignerAndVerify/gpg_subkey_armored_unprotected
    pgp_test.go:48:
                Error Trace:    pgp_test.go:48
                Error:          Received unexpected error:
                                openpgp: signature made by unknown entity
                Test:           TestPGPSignerAndVerify/gpg_subkey_armored_unprotected

I will be out of town for the weekend I added you to my forked crypto if you have time to keep digging in on why it is not validating now.

erikgeiser commented 3 years ago

I'm still struggeling to understand if this is something people do. @janoszen's explanation does make sense but how would you even export only a subkey without the main key. Manually deleting the key data from ~/.gnupg/private-keys-v1.d/ is obviously a hack and not an intended workflow in GnuPG, right? And if GnuPG doesn't even officially support this workflow it would be weird to do so much work to accommodate for a hack around GnuPG.

Please correct me if I'm still not understanding something.

EDIT: Also take a look at https://tools.ietf.org/html/rfc4880#section-11.1 (section 11.1 and section 11.2 which is more relevant but references 11.1 heavily) which suggests that sub keys without main keys are actually not in-spec for OpenPGP if I understand it correctly. Also the gpg2 man page has this to say about --export-secret-subkeys:

The second form of the command has the special property to render the
secret  part  of  the primary key useless; this is a GNU extension to
OpenPGP and other implementations can not be expected to successfully
import  such a key.  Its intended use is to generated a full key with
an additional signing subkey on a dedicated machine  and  then  using
this  command  to  export the key without the primary key to the main
machine.
ghost commented 3 years ago

I'm on my mobile so forgive the brevity. The simpelst way to imagine this is using a smart card (e.g. Yubikey). You create your key and then move it to the smart card using the appropriate commands, then create a subkey. That way you have an "offline" main key and use the subkey for daily work.

djgilcrease commented 3 years ago

According to the keybase bug report and pr it is mostly an issue for hardware keys where the primary key is on the hardware then the signing subkeys are used offline in build pipelines and such.

I am hopeful I will be able to get a patch into the golang/crypto as I can present it as a security issue at my company and they can work with Google directly to get it merged. We have done this in the past so there are already channels we can use, but it's not a guarantee.

ghost commented 3 years ago

@djgilcrease that's awesome. I'm happy to code the fix for it, but my problem so far was punching it through. Depending on how it goes I'd love to fix other crypto-related issues too. It's a shame that this package is so neglected.

ghost commented 3 years ago

For posterity I'd like to leave this link here, it helped me a lot in figuring out what the problem was: https://davesteele.github.io/gpg/2014/09/20/anatomy-of-a-gpg-key/

ghost commented 3 years ago

@djgilcrease I'm working through patching support in, here's what I found:

  1. The OpenPGP standard indeed does not support not having a usable key.
  2. When I look at the implementation it is almost as if there was a thought to make this happen. See ReadKeyRing in keys.go. However, reading to the next public key is not what should be done.

Any ideas where the implementation should be done?

djgilcrease commented 3 years ago

Talking to the team at my company responsible for signing the artifacts we ship to customers being able to sign without access to the primary private key does not make any sense at all. They understood the desire for security, but they said it should still be needed to be paired with the hardware key or special machine that had access to the primary key that you secured in another way.

I will talk to a few other teams as well who work more on building and shipping stuff from github, but if I remember correctly they were doing all the signing on a jenkins VM hosted inside our internal network.

ghost commented 3 years ago

So what is your scenario for the case when Jenkins gets compromised? Do you have one?

djgilcrease commented 3 years ago

Ok so clarified a bit with the team and we do have a master key that is used for generating all the signing keys, and we rotate out our signing keys every 90 days. So not a big deal for us to just invalidate one early. Now if the master key got compromised then it would be an issue, though I am sure our IT department has plans for that, though I don't get to know those =p

Also, the team clarified that you CANNOT use the master public key to verify something signed by a subkey which is what this test is doing. So I will work on the crypto code a little more to get things cleaned up there, and I will export the public subkey which will be needed to validate against and it should work.

djgilcrease commented 3 years ago

https://riseup.net/en/security/message-security/openpgp/best-practices#have-a-separate-subkey-for-signing

https://security.stackexchange.com/questions/174738/using-primary-public-key-to-verify-message-signed-by-subkey

ghost commented 3 years ago

Maybe I'm completely off-track here, but when you rotate the signing keys how do you make sure your customers then get the new signing key? Or do you mark the main key as trusted, but then how do the clients know about the signing key?

ghost commented 3 years ago

A little update on this issue. I did some digging into how to rotate keys properly and it seems that I was a bit off track. Using subkeys only does not solve the problem.

My current plan is creating one repo that will be signed by the main key and contains a single package: containerssh-keyring. This will install the keys and the repo for the normal packages and updates to this repo will rotate the keys. It's clunky as hell, but I have found no better way.

This somewhat mitigates the need for signing with subkeys as I can just create separate keys for signing the day-to-day packages. It would still be nice to be able to sign with subkeys.

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.