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

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

Closed
fmpwizard opened this issue Dec 3, 2018 · 8 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Security
Milestone

Comments

@fmpwizard
Copy link

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
@gopherbot gopherbot added this to the Unreleased milestone Dec 3, 2018
@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 3, 2018
@FiloSottile FiloSottile self-assigned this Dec 3, 2018
@jmataa
Copy link

jmataa commented Dec 20, 2018

On the Helm project: (helm/helm#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
Copy link

jmataa commented Dec 20, 2018

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
Copy link
Author

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
Copy link
Author

@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
Copy link

arnisoph commented Jan 9, 2020

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
Copy link

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.

@alifarooq0
Copy link

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

@FiloSottile
Copy link
Contributor

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 [email protected] 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.

infraweavers pushed a commit to infraweavers/monitoring-agent that referenced this issue Apr 6, 2021
vicamo added a commit to vicamo/aptly that referenced this issue Jan 28, 2022
Internal openpgp implementation without gpg1 installed now fails due to
golang/go#29082, and that's unlikely going to
be addressed ever, because x/crypto/openpgp has been deprecated as
stated in golang/go#44226. While aptly can
still be used on legacy environments, marking the dependency inbetween
and declare the obsolete status instead.

Signed-off-by: You-Sheng Yang <[email protected]>
@golang golang locked and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Security
Projects
None yet
Development

No branches or pull requests

7 participants