crypto/signify, build: fix archive signing with signify (#21977)

This fixes some issues in crypto/signify and makes release signing work.

The archive signing step in ci.go used getenvBase64, which decodes the key data.
This is incorrect here because crypto/signify already base64-decodes the key.
# Conflicts:
#	build/ci.go

.travis.yml

@@ -67,22 +67,22 @@ jobs:
       script:
         # Build for the primary platforms that Trusty can manage
         - go run build/ci.go install -dlgo
-        - go run build/ci.go archive -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
         - go run build/ci.go install -dlgo -arch 386
-        - go run build/ci.go archive -arch 386 -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -arch 386 -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
 
         # Switch over GCC to cross compilation (breaks 386, hence why do it here only)
         - sudo -E apt-get -yq --no-install-suggests --no-install-recommends --force-yes install gcc-arm-linux-gnueabi libc6-dev-armel-cross gcc-arm-linux-gnueabihf libc6-dev-armhf-cross gcc-aarch64-linux-gnu libc6-dev-arm64-cross
         - sudo ln -s /usr/include/asm-generic /usr/include/asm
 
         - GOARM=5 go run build/ci.go install -dlgo -arch arm -cc arm-linux-gnueabi-gcc
-        - GOARM=5 go run build/ci.go archive -arch arm -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - GOARM=5 go run build/ci.go archive -arch arm -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
         - GOARM=6 go run build/ci.go install -dlgo -arch arm -cc arm-linux-gnueabi-gcc
-        - GOARM=6 go run build/ci.go archive -arch arm -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - GOARM=6 go run build/ci.go archive -arch arm -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
         - GOARM=7 go run build/ci.go install -dlgo -arch arm -cc arm-linux-gnueabihf-gcc
-        - GOARM=7 go run build/ci.go archive -arch arm -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - GOARM=7 go run build/ci.go archive -arch arm -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
         - go run build/ci.go install -dlgo -arch arm64 -cc aarch64-linux-gnu-gcc
-        - go run build/ci.go archive -arch arm64 -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -arch arm64 -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
 
     # This builder does the Linux Azure MIPS xgo uploads
     - stage: build
@@ -100,19 +100,19 @@ jobs:
       script:
         - go run build/ci.go xgo --alltools -- --targets=linux/mips --ldflags '-extldflags "-static"' -v
         - for bin in build/bin/*-linux-mips; do mv -f "${bin}" "${bin/-linux-mips/}"; done
-        - go run build/ci.go archive -arch mips -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -arch mips -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
 
         - go run build/ci.go xgo --alltools -- --targets=linux/mipsle --ldflags '-extldflags "-static"' -v
         - for bin in build/bin/*-linux-mipsle; do mv -f "${bin}" "${bin/-linux-mipsle/}"; done
-        - go run build/ci.go archive -arch mipsle -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -arch mipsle -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
 
         - go run build/ci.go xgo --alltools -- --targets=linux/mips64 --ldflags '-extldflags "-static"' -v
         - for bin in build/bin/*-linux-mips64; do mv -f "${bin}" "${bin/-linux-mips64/}"; done
-        - go run build/ci.go archive -arch mips64 -type tar -signer LINUX_SIGNING_KEY signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -arch mips64 -type tar -signer LINUX_SIGNING_KEY signify SIGNIFY_KEY -upload gethstore/builds
 
         - go run build/ci.go xgo --alltools -- --targets=linux/mips64le --ldflags '-extldflags "-static"' -v
         - for bin in build/bin/*-linux-mips64le; do mv -f "${bin}" "${bin/-linux-mips64le/}"; done
-        - go run build/ci.go archive -arch mips64le -type tar -signer LINUX_SIGNING_KEY -signify LINUX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -arch mips64le -type tar -signer LINUX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
 
     # This builder does the Android Maven and Azure uploads
     - stage: build
@@ -151,7 +151,7 @@ jobs:
 
         - mkdir -p $GOPATH/src/github.com/ethereum
         - ln -s `pwd` $GOPATH/src/github.com/ethereum/go-ethereum
-        - go run build/ci.go aar -signer ANDROID_SIGNING_KEY -signify ANDROID_SIGNIFY_KEY  -deploy https://oss.sonatype.org -upload gethstore/builds
+        - go run build/ci.go aar -signer ANDROID_SIGNING_KEY -signify SIGNIFY_KEY  -deploy https://oss.sonatype.org -upload gethstore/builds
 
     # This builder does the OSX Azure, iOS CocoaPods and iOS Azure uploads
     - stage: build
@@ -167,7 +167,7 @@ jobs:
         submodules: false # avoid cloning ethereum/tests
       script:
         - go run build/ci.go install -dlgo
-        - go run build/ci.go archive -type tar -signer OSX_SIGNING_KEY -signify OSX_SIGNIFY_KEY -upload gethstore/builds
+        - go run build/ci.go archive -type tar -signer OSX_SIGNING_KEY -signify SIGNIFY_KEY -upload gethstore/builds
 
         # Build the iOS framework and upload it to CocoaPods and Azure
         - gem uninstall cocoapods -a -x
@@ -182,7 +182,7 @@ jobs:
 
         # Workaround for https://github.com/golang/go/issues/23749
         - export CGO_CFLAGS_ALLOW='-fmodules|-fblocks|-fobjc-arc'
-        - go run build/ci.go xcode -signer IOS_SIGNING_KEY -signify IOS_SIGNIFY_KEY -deploy trunk -upload gethstore/builds
+        - go run build/ci.go xcode -signer IOS_SIGNING_KEY -signify SIGNIFY_KEY -deploy trunk -upload gethstore/builds
 
     # These builders run the tests
     - stage: build

crypto/signify/signify.go

@@ -20,99 +20,81 @@
 package signify
 
 import (
+	"bytes"
+	"crypto/ed25519"
 	"encoding/base64"
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"strings"
 	"time"
-
-	"crypto/ed25519"
 )
 
 var (
-	errInvalidKeyHeader = errors.New("Incorrect key header")
+	errInvalidKeyHeader = errors.New("incorrect key header")
 	errInvalidKeyLength = errors.New("invalid, key length != 104")
 )
 
-func parsePrivateKey(key string) (ed25519.PrivateKey, []byte, []byte, error) {
+func parsePrivateKey(key string) (k ed25519.PrivateKey, header []byte, keyNum []byte, err error) {
 	keydata, err := base64.StdEncoding.DecodeString(key)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-
 	if len(keydata) != 104 {
 		return nil, nil, nil, errInvalidKeyLength
 	}
-
 	if string(keydata[:2]) != "Ed" {
 		return nil, nil, nil, errInvalidKeyHeader
 	}
-
 	return ed25519.PrivateKey(keydata[40:]), keydata[:2], keydata[32:40], nil
 }
 
-func commentHasManyLines(comment string) bool {
-	firstLFIndex := strings.IndexByte(comment, 10)
-	return (firstLFIndex >= 0 && firstLFIndex < len(comment)-1)
-}
+// SignFile creates a signature of the input file.
+//
+// This accepts base64 keys in the format created by the 'signify' tool.
+// The signature is written to the 'output' file.
+func SignFile(input string, output string, key string, untrustedComment string, trustedComment string) error {
+	// Pre-check comments and ensure they're set to something.
+	if strings.IndexByte(untrustedComment, '\n') >= 0 {
+		return errors.New("untrusted comment must not contain newline")
+	}
+	if strings.IndexByte(trustedComment, '\n') >= 0 {
+		return errors.New("trusted comment must not contain newline")
+	}
+	if untrustedComment == "" {
+		untrustedComment = "verify with " + input + ".pub"
+	}
+	if trustedComment == "" {
+		trustedComment = fmt.Sprintf("timestamp:%d", time.Now().Unix())
+	}
 
-// SignifySignFile creates a signature of the input file.
-func SignifySignFile(input string, output string, key string, unTrustedComment string, trustedComment string) error {
-	in, err := os.Open(input)
+	filedata, err := ioutil.ReadFile(input)
 	if err != nil {
 		return err
 	}
-	defer in.Close()
-
-	out, err := os.Create(output)
-	if err != nil {
-		return err
-	}
-	defer out.Close()
-
 	skey, header, keyNum, err := parsePrivateKey(key)
 	if err != nil {
 		return err
 	}
 
-	filedata, err := ioutil.ReadAll(in)
-	if err != nil {
-		return err
-	}
-
+	// Create the main data signature.
 	rawSig := ed25519.Sign(skey, filedata)
+	var dataSig []byte
+	dataSig = append(dataSig, header...)
+	dataSig = append(dataSig, keyNum...)
+	dataSig = append(dataSig, rawSig...)
 
-	var sigdata []byte
-	sigdata = append(sigdata, header...)
-	sigdata = append(sigdata, keyNum...)
-	sigdata = append(sigdata, rawSig...)
+	// Create the comment signature.
+	var commentSigInput []byte
+	commentSigInput = append(commentSigInput, rawSig...)
+	commentSigInput = append(commentSigInput, []byte(trustedComment)...)
+	commentSig := ed25519.Sign(skey, commentSigInput)
 
-	// Check that the trusted comment fits in one line
-	if commentHasManyLines(unTrustedComment) {
-		return errors.New("untrusted comment must fit on a single line")
-	}
-
-	if unTrustedComment == "" {
-		unTrustedComment = "verify with " + input + ".pub"
-	}
-	out.WriteString(fmt.Sprintf("untrusted comment: %s\n%s\n", unTrustedComment, base64.StdEncoding.EncodeToString(sigdata)))
-
-	// Add the trusted comment if unavailable
-	if trustedComment == "" {
-		trustedComment = fmt.Sprintf("timestamp:%d", time.Now().Unix())
-	}
-
-	// Check that the trusted comment fits in one line
-	if commentHasManyLines(trustedComment) {
-		return errors.New("trusted comment must fit on a single line")
-	}
-
-	var sigAndComment []byte
-	sigAndComment = append(sigAndComment, rawSig...)
-	sigAndComment = append(sigAndComment, []byte(trustedComment)...)
-	out.WriteString(fmt.Sprintf("trusted comment: %s\n%s\n", trustedComment, base64.StdEncoding.EncodeToString(ed25519.Sign(skey, sigAndComment))))
-
-	return nil
+	// Create the output file.
+	var out = new(bytes.Buffer)
+	fmt.Fprintln(out, "untrusted comment:", untrustedComment)
+	fmt.Fprintln(out, base64.StdEncoding.EncodeToString(dataSig))
+	fmt.Fprintln(out, "trusted comment:", trustedComment)
+	fmt.Fprintln(out, base64.StdEncoding.EncodeToString(commentSig))
+	return ioutil.WriteFile(output, out.Bytes(), 0644)
 }

crypto/signify/signify_test.go

@@ -52,7 +52,7 @@ func TestSignify(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = SignifySignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "clé", "croissants")
+	err = SignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "clé", "croissants")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -96,7 +96,7 @@ func TestSignifyTrustedCommentTooManyLines(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = SignifySignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "", "crois\nsants")
+	err = SignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "", "crois\nsants")
 	if err == nil || err.Error() == "" {
 		t.Fatalf("should have errored on a multi-line trusted comment, got %v", err)
 	}
@@ -121,7 +121,7 @@ func TestSignifyTrustedCommentTooManyLinesLF(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = SignifySignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "crois\rsants", "")
+	err = SignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "crois\rsants", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -146,7 +146,7 @@ func TestSignifyTrustedCommentEmpty(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = SignifySignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "", "")
+	err = SignFile(tmpFile.Name(), tmpFile.Name()+".sig", testSecKey, "", "")
 	if err != nil {
 		t.Fatal(err)
 	}

diff.txt

@@ -2352,9 +2352,3 @@ Date:   Wed Dec 9 15:43:36 2020 +0100
     
     The archive signing step in ci.go used getenvBase64, which decodes the key data.
     This is incorrect here because crypto/signify already base64-decodes the key.
-
-commit 915643a3e5a487899d925250b922824fb3d65708
-Author: Martin Holst Swende <martin@swende.se>
-Date:   Wed Dec 9 13:59:24 2020 +0100
-
-    cmd/geth: add test to verify regexps in version check (#21962)