x/crypto/openpgp: NewEntity creates malformed PGP key: unexpected EOF #25463

onetwopunch · 2018-05-19T06:03:59Z

What I'm trying to do is programmatically generate a PGP keypair, store the private key somewhere only this program has access to write to, such as KMS or something, then use the public key. The problem I'm facing is that it seems my method of creating a key pair is not working such that it can be re-ingested by this same library.

I'm fully aware that I may be doing something wrong here, but on the off chance this is an actual error in the openpgp package I figured I might as well submit an issue.

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

go version go1.10.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

Mac OSX

What did you do?

I'm trying to autogenerate a PGP key pair and validate the public key armored using the openpgp package in the following way:

package main

import (
	"bytes"
	"encoding/base64"
	"fmt"
	"golang.org/x/crypto/openpgp"
	"golang.org/x/crypto/openpgp/armor"
	"golang.org/x/crypto/openpgp/packet"
	"log"
)

func main() {
	buf := createKeyPair()
	fmt.Println(buf)
	encoded, err := readPGP(buf)
	if err != nil {
		log.Fatal(err)
	}
	validate(encoded)
}

func createKeyPair() *bytes.Buffer {
	conf := &packet.Config{
		RSABits: 2048,
	}
	entity, err := openpgp.NewEntity("Rick Sanchez", "Autogenerated GPG Key", "test@example.com", conf)
	// TODO: Store private key somewhere safe
	if err != nil {
		log.Fatal(err)
	}
	serializedEntity := bytes.NewBuffer(nil)
	entity.Serialize(serializedEntity)

	buf := bytes.NewBuffer(nil)
	headers := map[string]string{"Version": "GnuPG v1"}
	w, err := armor.Encode(buf, openpgp.PublicKeyType, headers)
	if err != nil {
		log.Fatal(err)
	}
	_, err = w.Write(serializedEntity.Bytes())
	if err != nil {
		log.Fatalf("error armoring serializedEntity: %s")
	}
	w.Close()

	return buf
}

func validate(keystring string) {
	data, err := base64.StdEncoding.DecodeString(keystring)
	if err != nil {
		log.Fatalf("error decoding given PGP key: %s", err)
	}
	_, err = openpgp.ReadEntity(packet.NewReader(bytes.NewBuffer(data)))
	if err != nil {
		log.Fatalf("error parsing given PGP key: %s", err)
	}
	fmt.Println("PASSED")
}

func readPGP(armoredKey *bytes.Buffer) (string, error) {
	keyReader := bytes.NewReader(armoredKey.Bytes())
	entityList, err := openpgp.ReadArmoredKeyRing(keyReader)
	if err != nil {
		log.Fatalf("error reading armored key %s", err)
	}
	serializedEntity := bytes.NewBuffer(nil)
	err = entityList[0].Serialize(serializedEntity)
	if err != nil {
		return "", fmt.Errorf("error serializing entity for file %s", err)
	}

	return base64.StdEncoding.EncodeToString(serializedEntity.Bytes()), nil
}

What did you expect to see?

<public key>
PASSED

What did you see instead?

Also, using this same createKeyPair method but just outputting it to stdout then attempting to import it to gpg (instead of this validation code) also did not work but with a different error:

$ go run gpg_test.go | gpg --import
gpg: key 962282668E9C926F: no valid user IDs
gpg: this may be caused by a missing self-signature
gpg: Total number processed: 1
gpg:           w/o user IDs: 1

The text was updated successfully, but these errors were encountered:

james-lawrence · 2018-05-19T09:49:54Z

I believe you need to use e.SerializePrivate to get the gpg import to work. but if you want the full key exported you need to use both Serialize and SerializePrivate.

onetwopunch · 2018-05-19T15:32:57Z

@james-lawrence You should be able to export/armor just the public portion for distribution. I can import the key into my gpg key ring, but I have to add the option: --allow-non-selfsigned-uid. I just can't seem to decode it with openpgp.

$ gpg --import --allow-non-selfsigned-uid /tmp/test.asc
gpg: key 962282668E9C926F: accepted non self-signed user ID "Rick Sanchez (Autogenerated GPG Key) <test@example.com>"
gpg: key 962282668E9C926F: public key "Rick Sanchez (Autogenerated GPG Key) <test@example.com>" imported
gpg: Total number processed: 1
gpg:               imported: 1

bcmills · 2018-05-21T17:28:48Z

(CC: @FiloSottile)

james-lawrence · 2018-05-21T19:30:50Z

@onetwopunch try using SerializePrivate to ioutil.Discard then just the Serialize to your buffer. it may not work, but iirc you need to invoke SerializePrivate to trigger the self signing aspect (could be completely wrong, going off memory atm)

onetwopunch · 2018-05-21T20:31:11Z

@james-lawrence You're right: https://github.com/golang/crypto/blob/master/openpgp/keys.go#L550

Is this desired though? I'd think that we should have the Serialize func sign the identities with the private key (if one exists as part of the entity) as well right?

When I change to use the SerializePrivate function with the config I get the public-private key pair armored together. I verified this by importing the key into GPG and the secret key was also imported.

I was able to update the Serialize method to give me what I need. I'm happy to create a pull request if a contributer deems it appropriate:

diff --git a/openpgp/keys.go b/openpgp/keys.go
index fd582a8..5ff4218 100644
--- a/openpgp/keys.go
+++ b/openpgp/keys.go
@@ -29,6 +29,7 @@ type Entity struct {
        Identities  map[string]*Identity // indexed by Identity.Name
        Revocations []*packet.Signature
        Subkeys     []Subkey
+       config      *packet.Config
 }

 // An Identity represents an identity claimed by an Entity and zero or more
@@ -483,6 +484,7 @@ func NewEntity(name, comment, email string, config *packet.Config) (*Entity, err
                PrimaryKey: packet.NewRSAPublicKey(currentTime, &signingPriv.PublicKey),
                PrivateKey: packet.NewRSAPrivateKey(currentTime, signingPriv),
                Identities: make(map[string]*Identity),
+               config:     config,
        }
        isPrimaryId := true
        e.Identities[uid.Id] = &Identity{
@@ -585,6 +587,12 @@ func (e *Entity) Serialize(w io.Writer) error {
                if err != nil {
                        return err
                }
+               if e.PrivateKey != nil {
+                       err = ident.SelfSignature.SignUserId(ident.UserId.Id, e.PrimaryKey, e.PrivateKey, e.config)
+                       if err != nil {
+                               return err
+                       }
+               }
                err = ident.SelfSignature.Serialize(w)
                if err != nil {
                        return err
@@ -601,6 +609,12 @@ func (e *Entity) Serialize(w io.Writer) error {
                if err != nil {
                        return err
                }
+               if e.PrivateKey != nil {
+                       err = subkey.Sig.SignKey(subkey.PublicKey, e.PrivateKey, e.config)
+                       if err != nil {
+                               return err
+                       }
+               }
                err = subkey.Sig.Serialize(w)
                if err != nil {

james-lawrence · 2018-05-21T20:41:09Z

@onetwopunch I would say its unnessarily confusing. just pointing you in the direction to unblock you, not making claims about the usability of the API. =)

onetwopunch · 2018-05-21T20:50:11Z

For now and for future users who have this problem, it can also be solved using your method like so:

// Signs the identities of the entity reference
entity.SerializePrivate(bytes.NewBuffer(nil), conf)

// Serialize only the public key.
serializedEntity := bytes.NewBuffer(nil)
entity.Serialize(serializedEntity)

I'm still happy to make a PR to clean this up a bit if someone wants. :) Thanks so much for the help @james-lawrence

FiloSottile · 2018-05-21T21:13:30Z

Please feel free to submit a fix as a PR. Thanks!

You might want to look a bit more into what the most appropriate time to fill in the signature is. I would not expect Serialize to cause a sign op every time it's called. (But I haven't looked at the code in depth so you might be right.)

onetwopunch · 2018-05-21T21:28:06Z

@FiloSottile Thanks, I'll do that. I think I might just add a separate method with the same signature as SerializePrivate called SerializeWithSignature so that we don't have to add the config to the entity and then we can allow users to serialize with the signature only when they want to, not affecting Serialize

FiloSottile · 2018-05-21T21:44:26Z

We should fix Serialize rather than grow a new API. Generating invalid output based on the order things are called in is not good. Just suggesting maybe the result should be cached.

onetwopunch · 2018-05-21T23:02:24Z

Yeah that makes sense. I'll get started tonight

onetwopunch · 2018-05-22T05:36:51Z

@FiloSottile when you have a moment, would you mind having a look at the above PR?

jarondl · 2018-06-08T15:59:31Z

Notice that this changes the API and broke some of our fragile code.

We had a function that created a key with NewEntity, and later changed the lifetime of a subkey. Since the signature was only done later, it worked. But now because of this change it suddenly stopped working and the subkeys had no expiration.
Our unit-tests caught it, but I think this change should be noted in the release notes or whatever, because it changes the behavior of exported functions.

gopherbot · 2018-06-11T18:54:02Z

Change https://golang.org/cl/118015 mentions this issue: openpgp: restore signing in SerializePrivate

Previously if you created a new Entity then ran `Serialize` _before_ running `SerializePrivate`, the resulting armored public key was corrupted, giving the error of `unexpected EOF`. This fix signs the public key with the private key upon creation of a NewEntity. Since SerializePrivate only is applicable to entities created with NewEntity per the docs we can also safely remove the signing portion from that function. Fixes golang#25463 Change-Id: I58b808987ee173079f33bce3d6c3527f9233b2cd GitHub-Last-Rev: 2c4b8e4d630a06d782816f8fbd3d59f01ece2565 GitHub-Pull-Request: golang/crypto#47 Reviewed-on: https://go-review.googlesource.com/114001 Reviewed-by: Filippo Valsorda <filippo@golang.org>

Signing was moved in NewEntity as it makes more sense there, but there might be code that relies on SerializePrivate to make signatures with parameters that were modified after NewEntity. As it used to always sign in SerializePrivate, it shouldn't break anything to sign in both places. Fixes golang/go#25463 Change-Id: Ia7f509daf31ac05fedc441225d554f333b288d70 Reviewed-on: https://go-review.googlesource.com/118015 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Yaron de Leeuw <jarondl@google.com> Reviewed-by: Alexandre Viau <viau.alexandre@gmail.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>

gopherbot added this to the Unreleased milestone May 19, 2018

agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 19, 2018

FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label May 21, 2018

gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 21, 2018

FiloSottile changed the title ~~x/crypto: openpgp NewEntity creates malformed PGP key: unexpected EOF~~ x/crypto/openpgp: NewEntity creates malformed PGP key: unexpected EOF May 21, 2018

onetwopunch mentioned this issue May 22, 2018

openpgp: sign Entity during instantiation in NewEntity golang/crypto#47

Closed

gopherbot closed this as completed in golang/crypto@a8fb68e Jun 14, 2018

golang locked and limited conversation to collaborators Jun 14, 2019

gopherbot added the FrozenDueToAge label Jun 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/crypto/openpgp: NewEntity creates malformed PGP key: unexpected EOF #25463

x/crypto/openpgp: NewEntity creates malformed PGP key: unexpected EOF #25463

onetwopunch commented May 19, 2018

james-lawrence commented May 19, 2018

onetwopunch commented May 19, 2018

bcmills commented May 21, 2018

james-lawrence commented May 21, 2018

onetwopunch commented May 21, 2018 •

edited

james-lawrence commented May 21, 2018

onetwopunch commented May 21, 2018 •

edited

FiloSottile commented May 21, 2018

onetwopunch commented May 21, 2018

FiloSottile commented May 21, 2018

onetwopunch commented May 21, 2018

onetwopunch commented May 22, 2018

jarondl commented Jun 8, 2018

gopherbot commented Jun 11, 2018

x/crypto/openpgp: NewEntity creates malformed PGP key: unexpected EOF #25463

x/crypto/openpgp: NewEntity creates malformed PGP key: unexpected EOF #25463

Comments

onetwopunch commented May 19, 2018

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

james-lawrence commented May 19, 2018

onetwopunch commented May 19, 2018

bcmills commented May 21, 2018

james-lawrence commented May 21, 2018

onetwopunch commented May 21, 2018 • edited

james-lawrence commented May 21, 2018

onetwopunch commented May 21, 2018 • edited

FiloSottile commented May 21, 2018

onetwopunch commented May 21, 2018

FiloSottile commented May 21, 2018

onetwopunch commented May 21, 2018

onetwopunch commented May 22, 2018

jarondl commented Jun 8, 2018

gopherbot commented Jun 11, 2018

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

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

onetwopunch commented May 21, 2018 •

edited

onetwopunch commented May 21, 2018 •

edited