golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.12k stars 17.56k forks source link

x/crypto/openpgp: silently fail to read keyring when smartcard stub is present #9312

Closed jvehent closed 3 years ago

jvehent commented 9 years ago

When a GPG secring contains a smartcard stub, the openpgp package fails to read the keyring correctly and does not return an error. A call to openpgp.ReadKeyRing() will returns the entities that precedes the stub, excluding the entity located right before the stub.

For example, if a secring contains 3 entities and the smartcard stub is in position 3, then openpgp.ReadKeyRing() will only return the first entity. The second entity and the stub are ignored. No error is returned.

Steps to reproduce:

1. Create a keyring with two regular 1024 bits RSA keys

$ mkdir /tmp/testgolangopenpgp
$ gpg --batch --no-default-keyring --keyring /tmp/testgolangopenpgp/pubring.gpg --secret-keyring /tmp/testgolangopenpgp/secring.gpg --gen-key << EOF
> Key-Type: 1
> Key-Length: 1024
> Subkey-Type: 1
> Subkey-Length: 1024
> Name-Real: test key 1
> Name-Email: testkey1@example.net
> Expire-Date: 12m
> EOF
gpg: keyring `/tmp/testgolangopenpgp/secring.gpg' created
gpg: keyring `/tmp/testgolangopenpgp/pubring.gpg' created
..+++++
...+++++
.....+++++
..+++++
gpg: key 4BACAA9F marked as ultimately trusted
$ gpg --batch --no-default-keyring --keyring /tmp/testgolangopenpgp/pubring.gpg --secret-keyring /tmp/testgolangopenpgp/secring.gpg --gen-key << EOF
> Key-Type: 1
> Key-Length: 1024
> Subkey-Type: 1
> Subkey-Length: 1024
> Name-Real: test key 2
> Name-Email: testkey2@example.net
> Expire-Date: 12m
> EOF
.+++++
.+++++
+++++
+++++
gpg: key 56CE5F62 marked as ultimately trusted

2. Import a smartcard stub from a yubikey into the keyring

$ gpg --no-default-keyring --keyring /tmp/testgolangopenpgp/pubring.gpg --secret-keyring /tmp/testgolangopenpgp/secring.gpg --allow-secret-key-import --import yubikeystub yubikeytest1_stub.asc 
gpg: key 4FB5544F: already in secret keyring
gpg: key 59BEDBFC: already in secret keyring
gpg: Total number processed: 2
gpg:       secret keys read: 2
gpg:  secret keys unchanged: 2

/tmp/testgolangopenpgp/secring.gpg
----------------------------------
sec   1024R/4BACAA9F 2014-12-14 [expires: 2015-12-09]
uid                  test key 1 <testkey1@example.net>
ssb   1024R/AEC9CD3A 2014-12-14

sec   1024R/56CE5F62 2014-12-14 [expires: 2015-12-09]
uid                  test key 2 <testkey2@example.net>
ssb   1024R/A1E03C73 2014-12-14

sec>  2048R/59BEDBFC 2014-12-14 [expires: 2015-12-09]
      Card serial no. = 0000 00000001
uid                  Yubikey Smartcard test1 <yubikeytest1@example.net>
ssb>  2048R/9B633300 2014-12-14
ssb>  2048R/A8DE1BB7 2014-12-14

3. Attempt to read the entities in the keyring. Only the first entity is returned.

$ go run readkeyring.go 
found 1 entities in keyring
reading entity with fingerprint 2FA49C2800342153CA439C90ADC366D34BACAA9F

source code:

package main

import (
    "code.google.com/p/go.crypto/openpgp"
    "encoding/hex"
    "fmt"
    "os"
    "strings"
)

func main() {
    secringFile, err := os.Open("/tmp/testgolangopenpgp/secring.gpg")
    if err != nil {
        panic(err)
    }
    defer secringFile.Close()
    keyring, err := openpgp.ReadKeyRing(secringFile)
    if err != nil {
        err = fmt.Errorf("Keyring access failed: '%v'", err)
        panic(err)
    }
    fmt.Printf("found %d entities in keyring\n", len(keyring))
    for _, entity := range keyring {
        fingerprint := strings.ToUpper(hex.EncodeToString(entity.PrimaryKey.Fingerprint[:]))
        fmt.Println("reading entity with fingerprint", fingerprint)
    }
}
jvehent commented 9 years ago

Ping @rsc & @agl who wrote the code according to git blame. Let me know if I can help test further!

jvehent commented 9 years ago

Can I do anything to help with the resolution of this issue? It's blocking some of my users...

jvehent commented 9 years ago

I took a peek at it, but this is my first dive into RFC4880 so some things are still unclear to me. Looking at the yubikey stub in the secring, if I skip the first 272 bytes, I get directly to the S2K value, which is set to 0xff:

$ hexdump -s 272 -C 000015-005.secret_key
00000110  ff 00 65 00 47 4e 55 02  10 d2 76 00 01 24 01 02  |..e.GNU...v..$..|
00000120  00 00 00 00 00 00 01 00  00                       |.........|
00000129

0xff indicates an encrypted private key. In the code [1], (pk *PrivateKey) parse() tries to read the next byte to guess the encryption algorithm, but that next byte is set to 00 which doesn't map to any valid cipher. Then, entering s2k.Parse(), the following 2 bytes are read (0x6500), and the second byte (0x00) is used in s2k.HashIdToHash(). But 0 is not a valid hash id, and the function returns error openpgp: unsupported feature: hash for S2K function: 0. The error is silenced is openpgp.ReadKeyRing [2].

I could do the analysis, but I'm afraid writing a patch is beyond my understand of the OpenPGP standard at this point...

[1] https://github.com/golang/crypto/blob/master/openpgp/packet/private_key.go#L67-L80 [2] https://github.com/golang/crypto/blob/master/openpgp/keys.go#L258

benburkert commented 8 years ago

The hexdump from @jvehent is an example of a gnupg extension S2K type, which can be identified by the 65 (101 in decimal, rfc4480#section-3.7.1 denotes 100-110 as "Private/Experimental") followed by the 47 4e 55 (GNU in ASCII). The 02 byte after that signifies that the stub is for "gnu-divert-to-card" S2K (01 signifies a "gnu-dummy" S2K as in #13605).

The relavent gnupg code is here and here.

FiloSottile commented 3 years ago

Per the accepted #44226 proposal and due to lack of maintenance, the golang.org/x/crypto/openpgp package is now frozen and deprecated. No new changes will be accepted except for security fixes. The package will not be removed.

If this is a security issue, please email security@golang.org and we will assess it and provide a fix.

If you're looking for alternatives, consider the crypto/ed25519 package for simple signatures, golang.org/x/mod/sumdb/note for inline signatures, or filippo.io/age for encryption. You can read a summary of OpenPGP issues and alternatives here.

If you are required to interoperate with OpenPGP systems and need a maintained package, we suggest considering one of multiple community forks of golang.org/x/crypto/openpgp. We don't endorse any specific one.