From 622ecc57a25a21c494260d6b6ca25973603ed20f Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Tue, 4 Sep 2018 16:19:33 -0600 Subject: [PATCH 1/4] bump certsore dependency --- Gopkg.lock | 2 +- .../mastahyeti/certstore/certstore_darwin.go | 23 +++++-------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 1c8e1df..8ac7beb 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -11,7 +11,7 @@ branch = "master" name = "github.com/mastahyeti/certstore" packages = ["."] - revision = "ab8919c9ca63b30a6f44cc573ee236ba24317979" + revision = "b5c7743eef9630309aec884accc77963ea8b1199" [[projects]] branch = "master" diff --git a/vendor/github.com/mastahyeti/certstore/certstore_darwin.go b/vendor/github.com/mastahyeti/certstore/certstore_darwin.go index e70e26b..c64391c 100644 --- a/vendor/github.com/mastahyeti/certstore/certstore_darwin.go +++ b/vendor/github.com/mastahyeti/certstore/certstore_darwin.go @@ -18,12 +18,6 @@ import ( "unsafe" ) -// work around https://github.com/golang/go/issues/24161 -var ( - _ C.CFBooleanRef - _ C.SecPolicyRef -) - // work around https://golang.org/doc/go1.10#cgo // in go>=1.10 CFTypeRefs are translated to uintptrs instead of pointers. var ( @@ -35,6 +29,7 @@ var ( nilCFStringRef C.CFStringRef nilSecIdentityRef C.SecIdentityRef nilSecKeyRef C.SecKeyRef + nilCFAllocatorRef C.CFAllocatorRef ) // macStore is a bogus type. We have to explicitly open/close the store on @@ -210,7 +205,7 @@ func (i *macIdentity) Signer() (crypto.Signer, error) { func (i *macIdentity) Delete() error { itemList := []C.SecIdentityRef{i.ref} itemListPtr := (*unsafe.Pointer)(unsafe.Pointer(&itemList[0])) - citemList := C.CFArrayCreate(nil, itemListPtr, 1, nil) + citemList := C.CFArrayCreate(nilCFAllocatorRef, itemListPtr, 1, nil) if citemList == nilCFArrayRef { return errors.New("error creating CFArray") } @@ -284,15 +279,9 @@ func (i *macIdentity) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts return nil, err } - // Hack — algos is a SecKeyAlgorithm and SecKeyCreateSignature takes a - // SecKeyAlgorithm as its second argument. The compiler insists though that - // algos is a CFStringRef and that SecKeyCreateSignature wants a - // *C.struct___CFString as its second argument. - algoHack := (*C.struct___CFString)(unsafe.Pointer(algo)) - // sign the digest var cerr C.CFErrorRef - csig := C.SecKeyCreateSignature(kref, algoHack, cdigest, &cerr) + csig := C.SecKeyCreateSignature(kref, algo, cdigest, &cerr) if err := cfErrorError(cerr); err != nil { defer C.CFRelease(C.CFTypeRef(cerr)) @@ -406,7 +395,7 @@ func stringToCFString(gostr string) C.CFStringRef { cstr := C.CString(gostr) defer C.free(unsafe.Pointer(cstr)) - return C.CFStringCreateWithCString(nil, cstr, C.kCFStringEncodingUTF8) + return C.CFStringCreateWithCString(nilCFAllocatorRef, cstr, C.kCFStringEncodingUTF8) } // mapToCFDictionary converts a Go map[C.CFTypeRef]C.CFTypeRef to a @@ -423,7 +412,7 @@ func mapToCFDictionary(gomap map[C.CFTypeRef]C.CFTypeRef) C.CFDictionaryRef { values = append(values, unsafe.Pointer(v)) } - return C.CFDictionaryCreate(nil, &keys[0], &values[0], C.CFIndex(n), nil, nil) + return C.CFDictionaryCreate(nilCFAllocatorRef, &keys[0], &values[0], C.CFIndex(n), nil, nil) } // cfDataToBytes converts a CFDataRef to a Go byte slice. @@ -444,7 +433,7 @@ func bytesToCFData(gobytes []byte) (C.CFDataRef, error) { cptr = (*C.UInt8)(&gobytes[0]) } - cdata := C.CFDataCreate(nil, cptr, clen) + cdata := C.CFDataCreate(nilCFAllocatorRef, cptr, clen) if cdata == nilCFDataRef { return nilCFDataRef, errors.New("error creatin cfdata") } From 1a6cc13b3e28d2170b4aafe54fcf608e284d7ba6 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 5 Sep 2018 12:07:20 -0600 Subject: [PATCH 2/4] bubble up errors instead of doing fancy wrappers around panics --- command_sign.go | 24 +++++++------ command_verify.go | 37 +++++++++++--------- list_keys_command.go | 8 +++-- main.go | 80 +++++++++++++------------------------------- 4 files changed, 64 insertions(+), 85 deletions(-) diff --git a/command_sign.go b/command_sign.go index e2d9959..224f5ca 100644 --- a/command_sign.go +++ b/command_sign.go @@ -13,13 +13,13 @@ import ( "github.com/pkg/errors" ) -func commandSign() { +func commandSign() error { userIdent, err := findUserIdentity() if err != nil { - faile(err, "failed to get identity matching specified user-id") + return errors.Wrap(err, "failed to get identity matching specified user-id") } if userIdent == nil { - failf("Could not find identity matching specified user-id: %s\n", *localUserOpt) + return fmt.Errorf("Could not find identity matching specified user-id: %s\n", *localUserOpt) } // Git is looking for "\n[GNUPG:] SIG_CREATED ", meaning we need to print a @@ -29,25 +29,25 @@ func commandSign() { chain, err := userIdent.CertificateChain() if err != nil { - faile(err, "failed to get idenity certificate chain") + return errors.Wrap(err, "failed to get idenity certificate chain") } signer, err := userIdent.Signer() if err != nil { - faile(err, "failed to get idenity signer") + return errors.Wrap(err, "failed to get idenity signer") } dataBuf := new(bytes.Buffer) if _, err = io.Copy(dataBuf, os.Stdin); err != nil { - faile(err, "failed to read message from stdin") + return errors.Wrap(err, "failed to read message from stdin") } sd, err := cms.NewSignedData(dataBuf.Bytes()) if err != nil { - faile(err, "failed to create signed data") + return errors.Wrap(err, "failed to create signed data") } if err := sd.Sign(chain, signer); err != nil { - faile(err, "failed to sign message") + return errors.Wrap(err, "failed to sign message") } if *detachSignFlag { sd.Detached() @@ -55,13 +55,13 @@ func commandSign() { if len(*tsaOpt) > 0 { if err = sd.AddTimestamps(*tsaOpt); err != nil { - faile(err, "failed to add timestamp") + return errors.Wrap(err, "failed to add timestamp") } } der, err := sd.ToDER() if err != nil { - faile(err, "failed to serialize signature") + return errors.Wrap(err, "failed to serialize signature") } emitSigCreated(chain[0], *detachSignFlag) @@ -75,8 +75,10 @@ func commandSign() { _, err = os.Stdout.Write(der) } if err != nil { - fail("failed to write signature") + return errors.New("failed to write signature") } + + return nil } // findUserIdentity attempts to find an identity to sign with in the certstore diff --git a/command_verify.go b/command_verify.go index 7d4d9e9..9ae432f 100644 --- a/command_verify.go +++ b/command_verify.go @@ -8,20 +8,21 @@ import ( "os" "github.com/certifi/gocertifi" + "github.com/github/go/errors" "github.com/mastahyeti/cms" ) -func commandVerify() { +func commandVerify() error { sNewSig.emit() if len(fileArgs) < 2 { - verifyAttached() - } else { - verifyDetached() + return verifyAttached() } + + return verifyDetached() } -func verifyAttached() { +func verifyAttached() error { var ( f *os.File err error @@ -30,7 +31,7 @@ func verifyAttached() { // Read in signature if len(fileArgs) == 1 { if f, err = os.Open(fileArgs[0]); err != nil { - failef(err, "failed to open signature file (%s)", fileArgs[0]) + return errors.Wrapf(err, "failed to open signature file (%s)", fileArgs[0]) } defer f.Close() } else { @@ -39,7 +40,7 @@ func verifyAttached() { buf := new(bytes.Buffer) if _, err = io.Copy(buf, f); err != nil { - faile(err, "failed to read signature") + return errors.Wrap(err, "failed to read signature") } // Try decoding as PEM @@ -53,7 +54,7 @@ func verifyAttached() { // Parse signature sd, err := cms.ParseSignedData(der) if err != nil { - faile(err, "failed to parse signature") + return errors.Wrap(err, "failed to parse signature") } // Verify signature @@ -66,7 +67,7 @@ func verifyAttached() { sErrSig.emit() } - faile(err, "failed to verify signature") + return errors.Wrap(err, "failed to verify signature") } emitGoodSig(chains) @@ -74,19 +75,21 @@ func verifyAttached() { // TODO: Maybe split up signature checking and certificate checking so we can // output something more meaningful. emitTrustFully() + + return nil } -func verifyDetached() { +func verifyDetached() error { // Read in signature f, err := os.Open(fileArgs[0]) if err != nil { - failef(err, "failed to open signature file (%s)", fileArgs[0]) + errors.Wrapf(err, "failed to open signature file (%s)", fileArgs[0]) } defer f.Close() buf := new(bytes.Buffer) if _, err = io.Copy(buf, f); err != nil { - faile(err, "failed to read signature file") + return errors.Wrap(err, "failed to read signature file") } // Try decoding as PEM @@ -100,7 +103,7 @@ func verifyDetached() { // Parse signature sd, err := cms.ParseSignedData(der) if err != nil { - faile(err, "failed to parse signature") + return errors.Wrap(err, "failed to parse signature") } // Read in signed data @@ -108,7 +111,7 @@ func verifyDetached() { f = os.Stdin } else { if f, err = os.Open(fileArgs[1]); err != nil { - failef(err, "failed to open message file (%s)", fileArgs[1]) + errors.Wrapf(err, "failed to open message file (%s)", fileArgs[1]) } defer f.Close() } @@ -116,7 +119,7 @@ func verifyDetached() { // Verify signature buf.Reset() if _, err = io.Copy(buf, f); err != nil { - faile(err, "failed to read message file") + return errors.Wrap(err, "failed to read message file") } chains, err := sd.VerifyDetached(buf.Bytes(), verifyOpts()) @@ -128,7 +131,7 @@ func verifyDetached() { sErrSig.emit() } - faile(err, "failed to verify signature") + return errors.Wrap(err, "failed to verify signature") } emitGoodSig(chains) @@ -136,6 +139,8 @@ func verifyDetached() { // TODO: Maybe split up signature checking and certificate checking so we can // output something more meaningful. emitTrustFully() + + return nil } func verifyOpts() x509.VerifyOptions { diff --git a/list_keys_command.go b/list_keys_command.go index e1f4bd2..7a8db45 100644 --- a/list_keys_command.go +++ b/list_keys_command.go @@ -3,13 +3,15 @@ package main import ( "fmt" "strings" + + "github.com/pkg/errors" ) -func commandListKeys() { +func commandListKeys() error { for j, ident := range idents { cert, err := ident.Certificate() if err != nil { - faile(err, "failed to get identity certificate") + return errors.Wrap(err, "failed to get identity certificate") } if j > 0 { @@ -24,4 +26,6 @@ func commandListKeys() { fmt.Println(" Subject:", cert.Subject.ToRDNSequence().String()) fmt.Println(" Emails:", strings.Join(certEmails(cert), ", ")) } + + return nil } diff --git a/main.go b/main.go index 2918fd3..7c4521d 100644 --- a/main.go +++ b/main.go @@ -33,8 +33,13 @@ var ( ) func main() { - defer handleExit() + if err := runCommand(); err != nil { + fmt.Println(err) + os.Exit(1) + } +} +func runCommand() error { // Parse CLI args getopt.HelpColumn = 30 getopt.SetParameters("[files]") @@ -42,25 +47,21 @@ func main() { fileArgs = getopt.Args() if *helpFlag { - if *signFlag || *verifyFlag || *listKeysFlag { - fail("specify --help, --sign, --verify, or --list-keys") - } else { - getopt.Usage() - return - } + getopt.Usage() + return nil } // Open certificate store store, err := certstore.Open() if err != nil { - faile(err, "failed to open certificate store") + return errors.Wrap(err, "failed to open certificate store") } defer store.Close() // Get list of identities idents, err = store.Identities() if err != nil { - faile(err, "failed to get identities from certificate store") + return errors.Wrap(err, "failed to get identities from certificate store") } for _, ident := range idents { defer ident.Close() @@ -68,74 +69,41 @@ func main() { if *signFlag { if *helpFlag || *verifyFlag || *listKeysFlag { - fail("specify --help, --sign, --verify, or --list-keys") + return errors.New("specify --help, --sign, --verify, or --list-keys") } else if len(*localUserOpt) == 0 { - fail("specify a USER-ID to sign with") + return errors.New("specify a USER-ID to sign with") } else { - commandSign() - return + return commandSign() } } if *verifyFlag { if *helpFlag || *signFlag || *listKeysFlag { - fail("specify --help, --sign, --verify, or --list-keys") + return errors.New("specify --help, --sign, --verify, or --list-keys") } else if len(*localUserOpt) > 0 { - fail("local-user cannot be specified for verification") + return errors.New("local-user cannot be specified for verification") } else if *detachSignFlag { - fail("detach-sign cannot be specified for verification") + return errors.New("detach-sign cannot be specified for verification") } else if *armorFlag { - fail("armor cannot be specified for verification") + return errors.New("armor cannot be specified for verification") } else { - commandVerify() - return + return commandVerify() } } if *listKeysFlag { if *helpFlag || *signFlag || *verifyFlag { - fail("specify --help, --sign, --verify, or --list-keys") + return errors.New("specify --help, --sign, --verify, or --list-keys") } else if len(*localUserOpt) > 0 { - fail("local-user cannot be specified for list-keys") + return errors.New("local-user cannot be specified for list-keys") } else if *detachSignFlag { - fail("detach-sign cannot be specified for list-keys") + return errors.New("detach-sign cannot be specified for list-keys") } else if *armorFlag { - fail("armor cannot be specified for list-keys") + return errors.New("armor cannot be specified for list-keys") } else { - commandListKeys() - return + return commandListKeys() } } - fail("specify --help, --sign, --verify, or --list-keys") -} - -type statusCode int - -func handleExit() { - if e := recover(); e != nil { - if sc, isStatusCode := e.(statusCode); isStatusCode { - os.Exit(int(sc)) - } - - panic(e) - } -} - -func faile(err error, message string) { - fail(errors.Wrap(err, message)) -} - -func failef(err error, format string, a ...interface{}) { - fail(errors.Wrapf(err, format, a...)) -} - -func fail(a ...interface{}) { - fmt.Fprintln(os.Stderr, a...) - panic(statusCode(1)) -} - -func failf(format string, a ...interface{}) { - fmt.Fprintf(os.Stderr, format, a...) - panic(statusCode(1)) + return errors.New("specify --help, --sign, --verify, or --list-keys") } From c58aee25a55f88d07ee50529711474dbfa9913d7 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 5 Sep 2018 12:10:42 -0600 Subject: [PATCH 3/4] write errors to stderr instead of stdout --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 7c4521d..8aac5ca 100644 --- a/main.go +++ b/main.go @@ -34,7 +34,7 @@ var ( func main() { if err := runCommand(); err != nil { - fmt.Println(err) + fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } } From 281adc09729593566e3f48ff6bd2bc36c2ef8539 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 5 Sep 2018 12:12:08 -0600 Subject: [PATCH 4/4] fmt.Sprintln is simpler --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 8aac5ca..c2631a4 100644 --- a/main.go +++ b/main.go @@ -34,7 +34,7 @@ var ( func main() { if err := runCommand(); err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) + fmt.Fprintln(os.Stderr, err) os.Exit(1) } }