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.

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

feat: gpg-agent support #14

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: gpg-agent support #14

wants to merge 3 commits into from

Conversation

dumbasPL
Copy link

@dumbasPL dumbasPL commented Sep 19, 2024

WIP: not properly tested, missing doc

decided against using the keystore since it's meant for "normal" environments with a working PGP home, proper certs, default socket paths, depends on several GPG binaries, etc.

Changes:

  • sign_key.asc can now also be just the public key (we still need it since the agent only deals with private keys)
  • if /app/S.gpg-agent exists then the agent will be used for cryptographic operations and only the public key portion of sign_key.asc will be used.

Logic if using the agent:

  • read the PGP key
  • connect to the GPG agent
  • refresh card info
  • find keys suitable for signing
  • check if we have a private key for any of them in the agent and use the first suitable one (keys that require user confirmation are ignored)
  • Create a keypair and optionally provide a password/pin to unlock the key
  • sign as usual

the agent needs to have loopback pinentry enabled or offer keys without a password.

closes #12

My suggestion for a setup to be included in the docs

# /etc/systemd/system/serene-gpg-agent.socket

[Unit]
Description=GnuPG cryptographic agent and passphrase cache for serene

[Socket]
ListenStream=/etc/serene/gnupg/S.gpg-agent
FileDescriptorName=std
SocketMode=0600
DirectoryMode=0700

[Install]
RequiredBy=docker.service
# /etc/systemd/system/serene-gpg-agent.service

[Unit]
Description=GnuPG cryptographic agent and passphrase cache for serene
Requires=serene-gpg-agent.socket

[Service]
ExecStart=/usr/bin/gpg-agent --homedir /etc/serene/gnupg --supervised
ExecReload=/usr/bin/gpgconf --homedir /etc/serene/gnupg --reload gpg-agent
# enable the socket
sudo systemctl enable serene-gpg-agent.socket

# test if your hardware key is detected
sudo GNUPGHOME=/etc/serene/gnupg gpg-connect-agent "LEARN --sendinfo" /bye
# docker-compose.yml

services:
  serene:
    # ... remaining service definition
    volumes:
      - /path/to/public-key.asc:/app/sign_key.asc
      - /etc/serene/gnupg/S.gpg-agent:/app/S.gpg-agent
      # ... other volumes

@VirtCode
Copy link
Owner

Looking great so far! I did some shallow testing and things seem to work well with gpg-agent.

The only thing I noticed is that we fail signing completely if updating the cards fails (here). On my end, this failed because I don't have any hardware devices I could test with (so I tested with my normal keyring). I think we should only print a warning here, specifically for setups which might not be card-based.

Let me know when it's ready for review!

@dumbasPL
Copy link
Author

I think we should only print a warning here, specifically for setups which might not be card-based.

Done.

Realistically, storing the key in the agent vs directly in the container makes little to no difference because, with the docker socket exposed, you can always escape to the host with root privileges. Not sure if TPM counts as card-based, will test it tomorrow.

Let me know when it's ready for review!

Unless I'm missing something obvious, It should be feature-complete. I suck at making human-readable docs so if you want to do that feel free. I've included the config I'm currently running on my instance in the original PR comment. I will run it like that for a while and make sure it's stable. If you wanna live on the edge and merge it then go ahead, it shouldn't break any existing setups.

@dumbasPL dumbasPL marked this pull request as ready for review September 20, 2024 23:25
@dumbasPL
Copy link
Author

dumbasPL commented Sep 20, 2024

One possible improvement would be to stop any further attempts at signing anything if the password/pin provided is wrong. Many cards will destroy the key material if you enter the wrong pin too many times. Not sure how to best implement this here though.

@VirtCode
Copy link
Owner

Okay, I'll draft something up for the docs. I'll include the things for setup with systemd you have provided, that'll be a good starting point. I don't see any issues in the code or feature wise, so I'll do some last testing on my end and then it'll be ready for merging.

I am really busy today, so I'll do it tomorrow morning.

One possible improvement would be to stop any further attempts at signing anything if the password/pin provided is wrong.

I see, we could track signature failures and refuse to continue after a failure. But judging from the docs, we the pin is only used during the signing process, so I don't know whether we can differentiate the pin being wrong from any other signature failure. I'll have a look tomorrow. Otherwise I'll just add a notice to the docs for now, so people make double sure they don't mistype their password.

@dumbasPL
Copy link
Author

dumbasPL commented Sep 21, 2024

I don't know whether we can differentiate the pin being wrong from any other signature failure

Pretty sure it has a unique error message (after all, any decent frontend will usually prompt the user for a second try if they mistype it). I'm more concerned about how we track this. Putting it in a database seems overkill and hard to reset without extra effort. Storing it in a global variable seems like a decent-ish middle ground. It won't persist across restarts but will usually give the user enough time to figure out something is broken.

Edit 2: now that I think about it, gpg is capable of tracking the "tries left" counter. We could probably use that and refuse to sign if it's at 1

@VirtCode
Copy link
Owner

VirtCode commented Sep 27, 2024

Okay, I've drafted up something for the docs and pushed it onto this branch. Let me know if I missed something.

I've also had a look at the bad passphrase issue. As it stands it's not that straight forward to "detect" a bad passphrase, because the library just shoves the error message from the agent into an "Operation failed" error (see here). I don't know whether these messages are always the same for bad passphrases, for me it always was "Bad passphrase", is this the same for keys on hardware devices?
I also could not find any way to find the tries left with the sequoia api, but I might've also missed something. Using the tries counter would indeed be the cleanest way (would work between restarts, etc.), but it currently looks like we'd have to match the error message string and then use a global variable on our side. What do you think?

Other than that I've done my testing, and everything works flawlessly, so it would be good to go.

@dumbasPL
Copy link
Author

dumbasPL commented Oct 1, 2024

Sorry for the late response.

I also could not find any way to find the tries left with the sequoia api, but I might've also missed something

It's there but it's not parsed. It's returned as part of the card_info response. Here is what this looks like on raw GPG:

~ ❯ gpg-connect-agent "learn --sendinfo" /bye
S PROGRESS learncard k 0 0
S PROGRESS learncard k 0 0
S PROGRESS learncard k 0 0
S UIF-3 %00+
S UIF-2 %00+
S UIF-1 %00+
S KDF �%01%00
S SIG-COUNTER 12
S CHV-STATUS +1+127+127+127+5+0+5
S KEY-TIME 3 1726417377
S KEY-TIME 2 1726417375
S KEY-TIME 1 1726417374
S KEY-FPR 3 D98A6E4F9E7E82B210FEAFBB7D3FAC2947DC86EC
S KEY-FPR 2 63031F4B6827779E90D58A420A638C24BF61C20C
S KEY-FPR 1 5F33EBCB4A7B1BB5D692BA9393B90E052A87CC14
S LOGIN-DATA nezu+<[email protected]>
S DISP-SEX 9
S MANUFACTURER 6 Yubico
S EXTCAP gc=1+ki=1+fc=1+pd=1+mcl3=2048+aac=1+sm=0+si=5+dec=0+bt=1+kdf=1
S APPVERSION 304
S APPTYPE openpgp
S CARDVERSION 50403
S CARDTYPE yubikey
S SERIALNO D2760001240100000006223177990000
S READER Yubico YubiKey FIDO+CCID 00 00
S KEYPAIRINFO 42938C8BB7CE03F6747386A0DF74CEFD9F02CC6E OPENPGP.3
S KEYPAIRINFO 9AABB0EC0D99A1ADB498002CBE24C20F8EFA4B05 OPENPGP.2
S KEYPAIRINFO F3F1E5FF7EAC65EA5F6AA061AB4B9A06DE0768F8 OPENPGP.1
OK

The part we are interested in is CHV-STATUS +1+127+127+127+5+0+5

Here is how gpg parses it and here are the descriptions

The last 5 numbers of CHV-STATUS tell us the retries left for PIN, Reset code, and Admin PIN. I don't have a reset code set, so the second one is 0 for me. For serene we only care about the first one, aka the normal PIN

We could either try to upstream the parsing into sequoia or just grab CardInfo::raw() and parse it ourselves. Or maybe both 🤔

Let me know what you think, I'll try to get this implemented later when I have some time.

Other than that I've done my testing, and everything works flawlessly, so it would be good to go.

Same on my end

EDIT:

From the doc:

The serene server needs to know which secret key to use from the agent.

A little 🤓 nitpick here, it's not only needed for picking the right key, but the main reason it's there is that it's impossible to create a valid signature without knowing the full details of the public key. The agent only deals with the private part and the actual crypto.

@VirtCode
Copy link
Owner

VirtCode commented Oct 1, 2024

Let me know what you think, I'll try to get this implemented later when I have some time.

I think parsing the relevant parts of CardInfo::raw() for the time being is a good idea, we can then still switch to the sequoia API once something is upstreamed. This is already much cleaner than what we could've done with just stopping on some failure string. Would be awesome if you could implement that when you find some time, so people won't accidentally wipe their keycards.

it's not only needed for picking the right key, but

Sure, feel free to rephrase anything in the docs. I'm not too deep in the weeds of pgp myself, so there might be a couple of mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better PGP support (HSM, smartcards, tpm, gpg-agent, etc)
2 participants