PGP finder WIP

.travis.yml

@@ -10,6 +10,7 @@ addons:
     packages:
     - python-virtualenv
     - graphviz
+    - gnupg2
 
 env:
   global:

context/context.go

@@ -387,35 +387,55 @@ func (context *AptlyContext) pgpProvider() string {
 		provider = context.config().GpgProvider
 	}
 
-	if !(provider == "gpg" || provider == "internal") { // nolint: goconst
+	switch provider {
+	case "gpg": // nolint: goconst
+	case "gpg1": // nolint: goconst
+	case "gpg2": // nolint: goconst
+	case "internal": // nolint: goconst
+	default:
 		Fatal(fmt.Errorf("unknown gpg provider: %v", provider))
 	}
 
 	return provider
 }
 
+func (context *AptlyContext) getGPGFinder(provider string) pgp.GPGFinder {
+	switch context.pgpProvider() {
+	case "gpg1":
+		return pgp.GPG1Finder()
+	case "gpg2":
+		return pgp.GPG2Finder()
+	case "gpg":
+		return pgp.GPGDefaultFinder()
+	}
+
+	panic("uknown GPG provider type")
+}
+
 // GetSigner returns Signer with respect to provider
 func (context *AptlyContext) GetSigner() pgp.Signer {
 	context.Lock()
 	defer context.Unlock()
 
-	if context.pgpProvider() == "gpg" { // nolint: goconst
-		return pgp.NewGpgSigner()
+	provider := context.pgpProvider()
+	if provider == "internal" { // nolint: goconst
+		return &pgp.GoSigner{}
 	}
 
-	return &pgp.GoSigner{}
+	return pgp.NewGpgSigner(context.getGPGFinder(provider))
 }
 
 // GetVerifier returns Verifier with respect to provider
 func (context *AptlyContext) GetVerifier() pgp.Verifier {
 	context.Lock()
 	defer context.Unlock()
 
-	if context.pgpProvider() == "gpg" { // nolint: goconst
-		return pgp.NewGpgVerifier()
+	provider := context.pgpProvider()
+	if provider == "internal" { // nolint: goconst
+		return &pgp.GoVerifier{}
 	}
 
-	return &pgp.GoVerifier{}
+	return pgp.NewGpgVerifier(context.getGPGFinder(provider))
 }
 
 // UpdateFlags sets internal copy of flags in the context

pgp/gnupg.go

@@ -3,6 +3,7 @@ package pgp
 import (
 	"bufio"
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -18,12 +19,10 @@ var (
 	_ Verifier = &GpgVerifier{}
 )
 
-// Skip GPG version check for GPG 1.x
-var skipGPGVersionCheck bool
-
 // GpgSigner is implementation of Signer interface using gpg as external program
 type GpgSigner struct {
 	gpg                        string
+	version                    GPGVersion
 	keyRef                     string
 	keyring, secretKeyring     string
 	passphrase, passphraseFile string
@@ -55,7 +54,7 @@ func (g *GpgSigner) gpgArgs() []string {
 	if g.keyring != "" {
 		args = append(args, "--no-auto-check-trustdb", "--no-default-keyring", "--keyring", g.keyring)
 	}
-	if g.secretKeyring != "" {
+	if g.secretKeyring != "" && g.version == 1 {
 		args = append(args, "--secret-keyring", g.secretKeyring)
 	}
 
@@ -64,7 +63,9 @@ func (g *GpgSigner) gpgArgs() []string {
 	}
 
 	if g.passphrase != "" || g.passphraseFile != "" {
-		args = append(args, "--no-use-agent")
+		if g.version == 1 {
+			args = append(args, "--no-use-agent")
+		}
 	}
 
 	if g.passphrase != "" {
@@ -77,53 +78,21 @@ func (g *GpgSigner) gpgArgs() []string {
 
 	if g.batch {
 		args = append(args, "--no-tty", "--batch")
-	}
-
-	return args
-}
-
-func cliVersionCheck(cmd string, marker string) bool {
-	output, err := exec.Command(cmd, "--version").CombinedOutput()
-	if err != nil {
-		return false
-	}
-	return skipGPGVersionCheck || strings.Contains(string(output), marker)
-}
-
-func findSuitableCLI(cmds []string, versionMarker string) string {
-	for _, cmd := range cmds {
-		if cliVersionCheck(cmd, versionMarker) {
-			return cmd
+		if g.version == 2 {
+			args = append(args, "--pinentry-mode", "loopback")
 		}
 	}
-	return ""
-}
-
-// We only support gpg1 at this time. Make sure we find a suitable binary.
-func findGPG1() (string, error) {
-	cmd := findSuitableCLI([]string{"gpg", "gpg1"}, "gpg (GnuPG) 1.")
-	if cmd != "" {
-		return cmd, nil
-	}
-	return "", fmt.Errorf("Couldn't find a suitable gpg executable. Make sure gnupg1 is available as either gpg or gpg1 in $PATH")
-}
 
-// We only support gpgv1 at this time. Make sure we find a suitable binary.
-func findGPGV1() (string, error) {
-	cmd := findSuitableCLI([]string{"gpgv", "gpgv1"}, "gpgv (GnuPG) 1.")
-	if cmd != "" {
-		return cmd, nil
-	}
-	return "", fmt.Errorf("Couldn't find a suitable gpgv executable. Make sure gpgv1 is available as either gpgv or gpgv1 in $PATH")
+	return args
 }
 
 // NewGpgSigner creates a new gpg signer
-func NewGpgSigner() *GpgSigner {
-	gpg, err := findGPG1()
+func NewGpgSigner(finder GPGFinder) *GpgSigner {
+	gpg, version, err := finder.FindGPG()
 	if err != nil {
 		panic(err)
 	}
-	return &GpgSigner{gpg: gpg}
+	return &GpgSigner{gpg: gpg, version: version}
 }
 
 // Init verifies availability of gpg & presence of keys
@@ -171,22 +140,27 @@ func (g *GpgSigner) ClearSign(source string, destination string) error {
 type GpgVerifier struct {
 	gpg      string
 	gpgv     string
+	version  GPGVersion
 	keyRings []string
 }
 
 // NewGpgVerifier creates a new gpg verifier
-func NewGpgVerifier() *GpgVerifier {
-	gpg, err := findGPG1()
+func NewGpgVerifier(finder GPGFinder) *GpgVerifier {
+	gpg, versionGPG, err := finder.FindGPG()
 	if err != nil {
 		panic(err)
 	}
 
-	gpgv, err := findGPGV1()
+	gpgv, versionGPGV, err := finder.FindGPGV()
 	if err != nil {
 		panic(err)
 	}
 
-	return &GpgVerifier{gpg: gpg, gpgv: gpgv}
+	if versionGPG != versionGPGV {
+		panic(errors.New("gpg and gpgv versions don't match"))
+	}
+
+	return &GpgVerifier{gpg: gpg, gpgv: gpgv, version: versionGPG}
 }
 
 // InitKeyring verifies that gpg is installed and some keys are trusted
@@ -417,11 +391,3 @@ func (g *GpgVerifier) ExtractClearsigned(clearsigned io.Reader) (text *os.File,
 
 	return
 }
-
-func init() {
-	skipCheck := os.Getenv("APTLY_SKIP_GPG_VERSION_CHECK")
-	switch strings.ToLower(skipCheck) {
-	case "1", "y", "yes", "true":
-		skipGPGVersionCheck = true
-	}
-}

pgp/gnupg_finder.go

@@ -0,0 +1,134 @@
+package pgp
+
+import (
+	"errors"
+	"os/exec"
+	"strings"
+)
+
+// Skip GPG version check for GPG 1.x
+var skipGPGVersionCheck bool
+
+// GPGVersion stores discovered GPG version
+//
+// 1 for 1.x, and 2 for 2.x
+type GPGVersion int
+
+// GPGFinder implement search for gpg executables and returns version of discovered executables
+type GPGFinder interface {
+	FindGPG() (gpg string, version GPGVersion, err error)
+	FindGPGV() (gpgv string, version GPGVersion, err error)
+}
+
+type pathGPGFinder struct {
+	gpgNames     []string
+	gpgvNames    []string
+	execPath     string
+	errorMessage string
+
+	expectedVersionSubstring string
+	version                  GPGVersion
+}
+
+type iteratingGPGFinder struct {
+	finders      []GPGFinder
+	errorMessage string
+}
+
+// GPGDefaultFinder looks for GPG1 first, but falls back to GPG2 if GPG1 is not available
+func GPGDefaultFinder() GPGFinder {
+	return &iteratingGPGFinder{
+		finders:      []GPGFinder{GPG1Finder(), GPG2Finder()},
+		errorMessage: "Couldn't find a suitable gpg executable. Make sure gnupg is installed",
+	}
+}
+
+// GPG1Finder looks for GnuPG1.x only
+func GPG1Finder() GPGFinder {
+	return &pathGPGFinder{
+		gpgNames:                 []string{"gpg", "gpg1"},
+		gpgvNames:                []string{"gpgv", "gpgv1"},
+		expectedVersionSubstring: "(GnuPG) 1.",
+		errorMessage:             "Couldn't find a suitable gpg executable. Make sure gnupg1 is available as either gpg(v) or gpg(v)1 in $PATH",
+		version:                  1,
+	}
+}
+
+// GPG2Finder looks for GnuPG2.x only
+func GPG2Finder() GPGFinder {
+	return &pathGPGFinder{
+		gpgNames:                 []string{"gpg", "gpg2"},
+		gpgvNames:                []string{"gpgv", "gpgv2"},
+		expectedVersionSubstring: "(GnuPG) 2.",
+		errorMessage:             "Couldn't find a suitable gpg executable. Make sure gnupg2 is available as either gpg(v) or gpg(v)2 in $PATH",
+		version:                  2,
+	}
+}
+
+func (pgf *pathGPGFinder) FindGPG() (gpg string, version GPGVersion, err error) {
+	for _, cmd := range pgf.gpgNames {
+		if cliVersionCheck(cmd, pgf.expectedVersionSubstring) {
+			gpg = cmd
+			break
+		}
+	}
+
+	version = pgf.version
+
+	if gpg == "" {
+		err = errors.New(pgf.errorMessage)
+	}
+
+	return
+}
+
+func (pgf *pathGPGFinder) FindGPGV() (gpgv string, version GPGVersion, err error) {
+	for _, cmd := range pgf.gpgvNames {
+		if cliVersionCheck(cmd, pgf.expectedVersionSubstring) {
+			gpgv = cmd
+			break
+		}
+	}
+
+	version = pgf.version
+
+	if gpgv == "" {
+		err = errors.New(pgf.errorMessage)
+	}
+
+	return
+}
+
+func (it *iteratingGPGFinder) FindGPG() (gpg string, version GPGVersion, err error) {
+	for _, finder := range it.finders {
+		gpg, version, err = finder.FindGPG()
+		if err == nil {
+			return
+		}
+	}
+
+	err = errors.New(it.errorMessage)
+
+	return
+}
+
+func (it *iteratingGPGFinder) FindGPGV() (gpg string, version GPGVersion, err error) {
+	for _, finder := range it.finders {
+		gpg, version, err = finder.FindGPGV()
+		if err == nil {
+			return
+		}
+	}
+
+	err = errors.New(it.errorMessage)
+
+	return
+}
+
+func cliVersionCheck(cmd string, marker string) bool {
+	output, err := exec.Command(cmd, "--version").CombinedOutput()
+	if err != nil {
+		return false
+	}
+	return strings.Contains(string(output), marker)
+}