Go: x/crypto/openpgp: tag byte does not have MSB set but gpg decrypts it

[fmpwizard]

What version of Go are you using (go version)?

$ go version
go version go1.11 linux/amd64

commit has for openpgp

eb0de9b17e854e9b1ccd9963efafc79862359959  (Nov 27th , latest as of today, Dec 1st 2018)

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env

GOARCH="amd64"

GOBIN=""

GOCACHE="/home/diego/.cache/go-build"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="linux"

GOOS="linux"

GOPATH="/home/diego/work/golang"

GOPROXY=""

GORACE=""

GOROOT="/usr/local/go"

GOTMPDIR=""

GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"

GCCGO="gccgo"

CC="gcc"

CXX="g++"

CGO_ENABLED="1"

GOMOD=""

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build336610440=/tmp/go-build -gno-record-gcc-switches"

What did you do?

We use this function to decrypt files we get from different companies, only files from one company fail, all others decrypt fine.

func PGPDecrypt(file io.Reader) ([]byte, error) {

    secretKeyring := "/path/to/gpg/secring.gpg"
    passphrase := 'pgpPassphrase' // of course not haardoded on prod :)

    // init some vars
    var entityList openpgp.EntityList

    // Open the private key file
    keyringFileBuffer, err := os.Open(secretKeyring)
    if err != nil {
        return nil, errors.New("os.Open(secretKeyring): " + err.Error())
    }
    defer keyringFileBuffer.Close()
    entityList, err = openpgp.ReadKeyRing(keyringFileBuffer)
    if err != nil {
        return nil, errors.New("openpgp.ReadKeyRing(keyringFileBuffer): " + err.Error())
    }

    // Get the passphrase and read the private key.
    // Have not touched the encrypted string yet
    passphraseByte := []byte(passphrase)
    // Support a user with multiple private keys
    for _, entity := range entityList {
        entity.PrivateKey.Decrypt(passphraseByte)
        for _, subkey := range entity.Subkeys {
            subkey.PrivateKey.Decrypt(passphraseByte)
        }
    }

    md, err := openpgp.ReadMessage(file, entityList, nil, nil)
        // =========== this is where I get the error:
        // tag byte does not have MSB set
    if err != nil {
        return nil, errors.New("openpgp.ReadMessage(file, entityList, nil, nil): " + err.Error())
    }
    ret, err := ioutil.ReadAll(md.UnverifiedBody)
    if err != nil {
        return nil, errors.New("ioutil.ReadAll(md.UnverifiedBody): " + err.Error())
    }

    return ret, nil
}

What did you expect to see?

No error and a decrypted zip file, which is what the command line gpg does

What did you see instead?

the error:

tag byte does not have MSB set

More details.

I brought this up on the mailing list
https://groups.google.com/d/topic/golang-nuts/-bBXt-0nVT4/discussion

but I'll paste more details here:

Part of the verbose output from gpg when it decrypts it is:

gpg --decrypt-files -vvv xyz.zip.pgp
gpg: using character set `utf-8'
gpg: armor: BEGIN PGP MESSAGE
gpg: armor header: Version: EldoS PGPBlackbox (.NET edition)
:pubkey enc packet: version 3, algo 1, keyid AFF537579B90F252
    data: [4095 bits]
....
gpg: public key encrypted data: good DEK
:pubkey enc packet: version 3, algo 1, keyid <removed from paste>
    data: [4095 bits]
gpg: public key is <removed from paste>
:encrypted data packet:
    length: unknown
    mdc_method: 2

....
:literal data packet:
    mode b (62), created 1542720445, name="xyz.zip",
    raw data: unknown length
gpg: original file name='xyz.zip'
gpg: decryption okay

[jmataa]

On the Helm project: (https://github.com/helm/helm/issues/2843) I think they @technosophos found a potential cause. There is a new keyring format for gnupg 2.1: https://gnupg.org/faq/whats-new-in-2.1.html#keybox

[jmataa]

Following up here, I was getting this error due to using ReadKeyRing when I should have been using ReadArmoredKeyRing. The key format has changed though, so it may be that as well.

[fmpwizard]

I don't think @jmataa and I have the same issue, while the error is the same, from my sample code, the error you get would be at:

entityList, err = openpgp.ReadKeyRing(keyringFileBuffer)

but in my case it is at

d, err := openpgp.ReadMessage(file, entityList, nil, nil)

And note that using the same code and key I can decrypt many other files just fine, but this one vendor sends me these files that cause issues. But I'm still getting into the details of the pgp format so maybe I'm missing something

[fmpwizard]

@FiloSottile I found the problem!

a keyring can be in two formats, binary or armored, and we have two functions to read each, ReadKeyRing and ReadArmoredKeyRing

The pgp file we get from this one company, is also armored, where all the previous files from other companies we have been getting were in binary format.

But, we don't have a ReadArmoredMessage function.

Right now I create a bufio.Reader and Peek into the file to see if it starts with -- (could prob make it read more to see if I get a full -----BEGIN PGP MESSAGE-----) , and if I do, I call openpgp/armor.Decode and then call openpgp.ReadMessage

This works for our use case and maybe you would want to close this ticket, but, would you be ok accepting a CL that:

1. Adds a ReadArmoredMessage and a bit of code comment to ReadMessage pointing out you may want to look at the *Armored* version?
   or
2. Only add a bit of code comment to ReadMessage
   or some other better solution ?

Thanks.

[arnisoph]

It also took some hours (!) for me to figure out, I have to export the secret key to the old format..

https://gist.github.com/stuart-warren/93750a142d3de4e8fdd2

[ArcticSnowman]

Why not have ReadKeyRing handle both binary or armored key rings at the same time? Really should not be up to application developers to have to test the raw keyring.

[ali-farooq0]

Why not have ReadKeyRing handle both binary or armored key rings at the same time? Really should not be up to application developers to have to test the raw keyring.

+1