From 0e87ceceed5a357a256bf2805132fbb2abfa0c71 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 9 Dec 2020 15:43:36 +0100 Subject: [PATCH] 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 | 26 ++++----- crypto/signify/signify.go | 98 ++++++++++++++-------------------- crypto/signify/signify_test.go | 8 +-- diff.txt | 6 --- 4 files changed, 57 insertions(+), 81 deletions(-) diff --git a/.travis.yml b/.travis.yml index d37458792..1268c6d65 100644 --- a/.travis.yml +++ b/.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 diff --git a/crypto/signify/signify.go b/crypto/signify/signify.go index e86c4f09b..7ba970549 100644 --- a/crypto/signify/signify.go +++ b/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) } diff --git a/crypto/signify/signify_test.go b/crypto/signify/signify_test.go index af77eaf22..615d4e652 100644 --- a/crypto/signify/signify_test.go +++ b/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 --git a/diff.txt b/diff.txt index a575674d7..717340c62 100644 --- a/diff.txt +++ b/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 -Date: Wed Dec 9 13:59:24 2020 +0100 - - cmd/geth: add test to verify regexps in version check (#21962)