Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

interfaces/gpg-keys: Allow access to gpg-agent and creation of lockfiles #7366

Closed
wants to merge 1 commit into from
Closed

interfaces/gpg-keys: Allow access to gpg-agent and creation of lockfiles #7366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 10 additions & 1 deletion interfaces/builtin/gpg_keys.go
View file
Open in desktop
Expand Up @@ -19,7 +19,7 @@

package builtin

const gpgKeysSummary = `allows reading gpg user configuration and keys and updating gpg's random seed file`
const gpgKeysSummary = `allows limited access to gpg-agent, reading gpg user configuration and keys and updating gpg's random seed file`

const gpgKeysBaseDeclarationSlots = `
gpg-keys:
Expand Down Expand Up @@ -49,6 +49,15 @@ deny @{HOME}/.gnupg/trustdb.gpg w,
# trust to access the user's keys and the level of trust for the random_seed
# is commensurate with accessing the private keys.
owner @{HOME}/.gnupg/random_seed wk,

# allow access to gpg-agent's extra socket for passphrase entry.
# This socket exposes a very limited subset of gpg-agent's functionality and
# only allows the confined application to use gpg's cryptographic functions
# but not anything related to managing the keys (adding, trusting etc).
Copy link

@jdstrand jdstrand Sep 4, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't list what the extra socket can do. The comment should probably instead say:

# Allow access to gpg-agent's extra socket. This socket exposes a limited subset
# of gpg-agent's functionality for signing and decryption as described in:
#  - https://lists.gnupg.org/pipermail/gnupg-users/2019-August/062504.html

That said, see my other comment.

owner /run/user/[0-9]*/gnupg/S.gpg-agent.extra wr,
Copy link

@jdstrand jdstrand Sep 4, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking, the original intent of this interface is to allow access to the keys, not the agent, which is why the interface is named 'gpg-keys', not 'gpg', 'gpg-support' or similar. This is for several reasons:

  • for keys where the user has already provided the password, the agent will give access to the private key without a password
  • when the agent is configured to manage ssh keys and when the user has already provided the password, the agent will give access to the private ssh key without a password
  • there is no reasonable guarantee that the agent on the host system will match what the snap expects

For the first point, when keys are protected by a passphrase, this is a surprising amount of access to give to the snap. While trust needs to be given to a snap to access and unlock a key (ie, the current gpg-keys interface), that is a different amount of trust than 'use all unlocked keys the agent will ever have access to'.

For the second point, allowing access to the gpg agent would allow bypassing the protections afforded by disconnecting the ssh-keys interface for systems with the gpg agent configured for ssh. While newer gpg-agents use S.gpg-agent.ssh, older versions use a shared S.gpg-agent (see below for why this is important). The first point also applies for ssh keys.

Some of the problems due to the third point are described at length in @oSoMoN's initial post in https://forum.snapcraft.io/t/signing-files-with-gpg-from-within-a-snap/5566. The gpg-agent, tools and libraries are generally designed to work with matching versions of each other. Some older distributions will use gnupg while newer ones use gnupg2, Ubuntu Core itself doesn't ship the gpg-agent at all and Ubuntu 16.04 ships both gnupg and gnupg2 but with the gpg-agent from gnupg2, but in all cases, the snap may use either the gpg{,2} from the base runtime or whatever it ships (v1 for core/core16 and v2 for core18+). There are a number of important differences between v1 and v2 and while gnupg is generally expected to work with a gnupg2 agent, the extra socket is not available with gnupg, so if we add the extra socket to this interface, users would then expect S.gpg-agent to be supported for older releases, which brings in ssh and all of the agent APIs. Furthermore, different distros will put the socket in different places. Eg, Ubuntu 16.04 puts it in ~/.gnupg/ where Ubuntu 19.10 puts it in /run/user/[0-9]*/gnupg/.

As with the ssh-agent (eg, https://forum.snapcraft.io/t/ssh-agent-plug-request/1250/3), it was always expected for gpg access that a snap would start its own agent. In this manner, snaps are free to unlock the keys they need (which could even be snap-specific) without having to worry about exposing those keys to other snaps since the sockets would (well, at least should) be placed in the snap-specific areas of the filesystem. As mentioned elsewhere, expected use of this interface (and ssh-keys for that matter) should be documented. @degville or the snap advocacy team could probably help here (perhaps this should even be a snapcraft part).

If agent access (ie, gpg or ssh) were going to be added to snapd, then I strongly feel it should be done in a separate interface, not the existing 'gpg-keys' interface and that interface would grant full use of the agent. That said, I'm quite concerned about the robustness of this new interface considering the above. All considered and since the existing interfaces are expected to work with a snap-specific agent, I'd prefer this scenario be properly documented and then what and see if there are legitimate use cases for the session-wide host agent (but I'll defer to @mvo5 and @pedronis on if a separate agent interface should wait or not).

Copy link
Author

@ppd ppd Sep 5, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely stunning elaboration, thank you @jdstrand. I agree that a separate interface is likely the way to go if full access is something that is desirable.

To start an agent specific to the snap is a good solution if no external interfaces (besides the keyboard) are required. My concern here is stuff like Smartcards (pcscd, scdaemon). This is pretty much the same story again, only now we can't run two scdaemons concurrently, as they compete for the same device. I'll have to research this scenario a little more though...

All considered and since the existing interfaces are expected to work with a snap-specific agent, I'd prefer this scenario be properly documented[...]

I feel that's a good solution for a decent chunk of interface users. This could maybe even be implemented as a snapcraft extension.


# gpg creates lockfiles on every invocation
owner @{HOME}/.gnupg/.#lk* rwk,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not required for /usr/bin/gpg{,1,2,v} in the past when not using the agent. What is creating these lockfiles? Can you provide exact steps on how to reproduce (when not using the agent)? snap run --strace ... would likely help shed some light.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not required for /usr/bin/gpg{,1,2,v} in the past when not using the agent. What is creating these lockfiles? Can you provide exact steps on how to reproduce (when not using the agent)? snap run --strace ... would likely help shed some light.

I looked at this and found that the core18 base snap only contains gpgv. So I copied gpg and various libs it needed to SNAP_COMMON of a snap with this interface connected and did:

LD_LIBRARY_PATH=$SNAP_COMMON GNUPGHOME=/home/jamie/.gnupg $SNAP_COMMON/gpg --list-keys --no-auto-check-trustdb
...
uid           [ unknown] Test Foo <test.foo@example.com>
...

$ LD_LIBRARY_PATH=$SNAP_COMMON GNUPGHOME=/home/jamie/.gnupg $SNAP_COMMON/gpg --sign foo
...
gpg: failed to create temporary file '.../.gnupg/.#lk0x00005603e517cda0.sec-bionic-amd64.6029': Permission denied

As such, if this rule were to be added, I think it should be:

# required for talking with agent
owner @{HOME}/.gnupg/.#lk0x[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f].*.[0-9]* rw,

but this exercise reminded me that gpg v2 always must be able to talk to the agent. Per the man page:

       --use-agent
       --no-use-agent
              This is dummy option. gpg always requires the agent.

As such, there is no point adding these lockfiles to this interface since they are for use with the agent. The above rule would need to be in the new agent interface, if it was created.

As mentioned, none of this is needed by consumers of this interface. I created a test-gnupg2 snap (it is not production; obtaining a tty is non-trivial and so there are limitations) and can demonstrate that everything works[1]:

$ test-gnupg2.gpg --version
gpg (GnuPG) 2.2.4
libgcrypt 1.8.1
Copyright (C) 2017 Free Software Foundation, Inc.
...
$ test-gnupg2.gpgconf --list-dirs
...
socketdir:/home/ubuntu/snap/test-gnupg2/current/.gnupg
...
agent-socket:/home/ubuntu/snap/test-gnupg2/current/.gnupg/S.gpg-agent
homedir:/home/ubuntu/snap/test-gnupg2/current/.gnupg

# generate a key in ~/.gnupg on the host, outside of the snap
$ gpg --quick-gen-key --yes host.key@example.com
...
uid                      host.key@example.com
...

# now do various operations within the snap
$ test-gnupg2.gpg --list-keys
gpg: keybox '/home/ubuntu/snap/test-gnupg2/current/.gnupg/pubring.kbx' created
gpg: /home/ubuntu/snap/test-gnupg2/current/.gnupg/trustdb.gpg: trustdb created

$ test-gnupg2.gpg --quick-gen-key --yes test.me@example.com
...
gpg: key 5C38153FCF4E759B marked as ultimately trusted
gpg: directory '/home/ubuntu/snap/test-gnupg2/current/.gnupg/openpgp-revocs.d' created
gpg: revocation certificate stored as '/home/ubuntu/snap/test-gnupg2/current/.gnupg/openpgp-revocs.d/85384036A5605D16004490295C38153FCF4E759B.rev'
...
uid                      test.me@example.com

$ echo testme > ~/snap/test-gnupg2/common/foo

$ cd ~/snap/test-gnupg2/common ; test-gnupg2.gpg -r test.me@example.com -e ~/snap/test-gnupg2/common/foo
gpg: checking the trustdb
gpg: marginals needed: 3  completes needed: 1  trust model: pgp
gpg: depth: 0  valid:   1  signed:   0  trust: 0-, 0q, 0n, 0m, 0f, 1u
gpg: next trustdb check due at 2021-09-04

$ cd ~/snap/test-gnupg2/common ; test-gnupg2.gpg -d ~/snap/test-gnupg2/common/foo.gpg 
gpg: encrypted with 3072-bit RSA key, ID 4FCA2D2BAF62CAE9, created 2019-09-05
      "test.me@example.com"
testme

# now try to use the key outside the snap
$ cd ~/snap/test-gnupg2/common ; test-gnupg2.gpg -r host.key@example.com -e ~/snap/test-gnupg2/common/foo
...
gpg: error retrieving 'host.key@example.com' ...
...

$ test-gnupg2.import-from-host
Imported /home/ubuntu/.gnupg to /home/ubuntu/snap/test-gnupg2/x8/.gnupg.host
Use --homedir=~/.gnupg.host for these keys

$ cd ~/snap/test-gnupg2/common ; test-gnupg2.gpg --homedir=~/.gnupg.host -r host.key@example.com -e ~/snap/test-gnupg2/common/foo # gpg expands '~' to $HOME, ie, $SNAP_USER_DATA
gpg: checking the trustdb
gpg: marginals needed: 3  completes needed: 1  trust model: pgp
gpg: depth: 0  valid:   1  signed:   0  trust: 0-, 0q, 0n, 0m, 0f, 1u
gpg: next trustdb check due at 2021-09-04
File '/home/ubuntu/snap/test-gnupg2/common/foo.gpg' exists. Overwrite? (y/N) y

$ cd ~/snap/test-gnupg2/common ; test-gnupg2.gpg --homedir=~/.gnupg.host -d ~/snap/test-gnupg2/common/foo.gpg 
gpg: encrypted with 3072-bit RSA key, ID A561E82BF3C5C438, created 2019-09-05
      "host.key@example.com"
testme

[1]gpg unconditionally tries to create the /run/user/id/gnupg/d.... directory but falls back to GNUPGHOME if it is unable to, so there is a spurious AppArmor noisy denial: type=1400 audit(1567701445.512:1673): apparmor="DENIED" operation="mkdir" profile="snap.test-gnupg2.gpg" name="/run/user/1001/gnupg/d.tbfxb9f6by7d4myapk8qbyud/" pid=11740 comm="gpg-agent" requested_mask="c" denied_mask="c" fsuid=1001 ouid=1001

`

func init() {
Expand Down
2 changes: 1 addition & 1 deletion interfaces/builtin/gpg_keys_test.go
View file
Open in desktop
Expand Up @@ -83,7 +83,7 @@ func (s *GpgKeysInterfaceSuite) TestStaticInfo(c *C) {
si := interfaces.StaticInfoOf(s.iface)
c.Assert(si.ImplicitOnCore, Equals, true)
c.Assert(si.ImplicitOnClassic, Equals, true)
c.Assert(si.Summary, Equals, `allows reading gpg user configuration and keys and updating gpg's random seed file`)
c.Assert(si.Summary, Equals, `allows limited access to gpg-agent, reading gpg user configuration and keys and updating gpg's random seed file`)
c.Assert(si.BaseDeclarationSlots, testutil.Contains, "gpg-keys")
}

Expand Down