diff --git a/.travis.yml b/.travis.yml index 7653bc4150..2ad4485d01 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,10 @@ email: false go: - 1.7 - script: make tools .gitvalidation validate test test-skopeo + env: + - BUILDTAGS='btrfs_noversion libdm_no_deferred_remove' + - BUILDTAGS='btrfs_noversion libdm_no_deferred_remove containers_image_openpgp' + script: make tools .gitvalidation validate test test-skopeo BUILDTAGS="$BUILDTAGS" dist: trusty os: - linux diff --git a/Makefile b/Makefile index 4ea77ec51a..a8b7457fa1 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,7 @@ test-skopeo: rm -rf $${vendor_path} && cp -r . $${vendor_path} && rm -rf $${vendor_path}/vendor && \ cd $${skopeo_path} && \ make BUILDTAGS="$(BUILDTAGS)" binary-local test-all-local && \ - $(SUDO) make check && \ + $(SUDO) make BUILDTAGS="$(BUILDTAGS)" check && \ rm -rf $${skopeo_path} validate: lint diff --git a/README.md b/README.md index 429755c0ec..7c1c5af676 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,16 @@ libraries you should use with this package in your own projects. What this project tests against dependencies-wise is located [here](https://github.com/containers/image/blob/master/vendor.conf). +## Building + +For ordinary use, `go build ./...` is sufficient. + +When developing this library, please use `make` to take advantage of the tests and validation. + +Optionally, you can use the `containers_image_openpgp` build tag (using `go build -tags …`, or `make … BUILDTAGS=…`). +This will use a Golang-only OpenPGP implementation for signature verification instead of the default cgo/gpgme-based implementation; +the primary downside is that creating new signatures with the Golang-only implementation is not supported. + ## License ASL 2.0 diff --git a/copy/copy.go b/copy/copy.go index 45fe8afddc..3f054cd3c3 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -235,6 +235,11 @@ func Image(policyContext *signature.PolicyContext, destRef, srcRef types.ImageRe if err != nil { return errors.Wrap(err, "Error initializing GPG") } + defer mech.Close() + if err := mech.SupportsSigning(); err != nil { + return errors.Wrap(err, "Signing not supported") + } + dockerReference := dest.Reference().DockerReference() if dockerReference == nil { return errors.Errorf("Cannot determine canonical Docker reference for destination %s", transports.ImageName(dest.Reference())) diff --git a/signature/docker_test.go b/signature/docker_test.go index 6d2f0b3d71..3776c8f386 100644 --- a/signature/docker_test.go +++ b/signature/docker_test.go @@ -11,6 +11,12 @@ import ( func TestSignDockerManifest(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) + defer mech.Close() + + if err := mech.SupportsSigning(); err != nil { + t.Skipf("Signing not supported: %v", err) + } + manifest, err := ioutil.ReadFile("fixtures/image.manifest.json") require.NoError(t, err) @@ -41,6 +47,7 @@ func TestSignDockerManifest(t *testing.T) { func TestVerifyDockerManifestSignature(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) + defer mech.Close() manifest, err := ioutil.ReadFile("fixtures/image.manifest.json") require.NoError(t, err) signature, err := ioutil.ReadFile("fixtures/image.signature") diff --git a/signature/mechanism.go b/signature/mechanism.go index 4b4393bcaf..bdf26c531f 100644 --- a/signature/mechanism.go +++ b/signature/mechanism.go @@ -9,19 +9,20 @@ import ( "io/ioutil" "strings" - "github.com/mtrmac/gpgme" "golang.org/x/crypto/openpgp" ) // SigningMechanism abstracts a way to sign binary blobs and verify their signatures. +// Each mechanism should eventually be closed by calling Close(). // FIXME: Eventually expand on keyIdentity (namespace them between mechanisms to // eliminate ambiguities, support CA signatures and perhaps other key properties) type SigningMechanism interface { - // ImportKeysFromBytes imports public keys from the supplied blob and returns their identities. - // The blob is assumed to have an appropriate format (the caller is expected to know which one). - // NOTE: This may modify long-term state (e.g. key storage in a directory underlying the mechanism). - ImportKeysFromBytes(blob []byte) ([]string, error) - // Sign creates a (non-detached) signature of input using keyidentity + // Close removes resources associated with the mechanism, if any. + Close() error + // SupportsSigning returns nil if the mechanism supports signing, or a SigningNotSupportedError. + SupportsSigning() error + // Sign creates a (non-detached) signature of input using keyIdentity. + // Fails with a SigningNotSupportedError if the mechanism does not support signing. Sign(input []byte, keyIdentity string) ([]byte, error) // Verify parses unverifiedSignature and returns the content and the signer's identity Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error) @@ -33,109 +34,34 @@ type SigningMechanism interface { UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) } -// A GPG/OpenPGP signing mechanism. -type gpgSigningMechanism struct { - ctx *gpgme.Context +// SigningNotSupportedError is returned when trying to sign using a mechanism which does not support that. +type SigningNotSupportedError string + +func (err SigningNotSupportedError) Error() string { + return string(err) } -// NewGPGSigningMechanism returns a new GPG/OpenPGP signing mechanism. +// NewGPGSigningMechanism returns a new GPG/OpenPGP signing mechanism for the user’s default +// GPG configuration ($GNUPGHOME / ~/.gnupg) +// The caller must call .Close() on the returned SigningMechanism. func NewGPGSigningMechanism() (SigningMechanism, error) { return newGPGSigningMechanismInDirectory("") } -// newGPGSigningMechanismInDirectory returns a new GPG/OpenPGP signing mechanism, using optionalDir if not empty. -func newGPGSigningMechanismInDirectory(optionalDir string) (SigningMechanism, error) { - ctx, err := gpgme.New() - if err != nil { - return nil, err - } - if err = ctx.SetProtocol(gpgme.ProtocolOpenPGP); err != nil { - return nil, err - } - if optionalDir != "" { - err := ctx.SetEngineInfo(gpgme.ProtocolOpenPGP, "", optionalDir) - if err != nil { - return nil, err - } - } - ctx.SetArmor(false) - ctx.SetTextMode(false) - return gpgSigningMechanism{ctx: ctx}, nil -} - -// ImportKeysFromBytes implements SigningMechanism.ImportKeysFromBytes -func (m gpgSigningMechanism) ImportKeysFromBytes(blob []byte) ([]string, error) { - inputData, err := gpgme.NewDataBytes(blob) - if err != nil { - return nil, err - } - res, err := m.ctx.Import(inputData) - if err != nil { - return nil, err - } - keyIdentities := []string{} - for _, i := range res.Imports { - if i.Result == nil { - keyIdentities = append(keyIdentities, i.Fingerprint) - } - } - return keyIdentities, nil -} - -// Sign implements SigningMechanism.Sign -func (m gpgSigningMechanism) Sign(input []byte, keyIdentity string) ([]byte, error) { - key, err := m.ctx.GetKey(keyIdentity, true) - if err != nil { - return nil, err - } - inputData, err := gpgme.NewDataBytes(input) - if err != nil { - return nil, err - } - var sigBuffer bytes.Buffer - sigData, err := gpgme.NewDataWriter(&sigBuffer) - if err != nil { - return nil, err - } - if err = m.ctx.Sign([]*gpgme.Key{key}, inputData, sigData, gpgme.SigModeNormal); err != nil { - return nil, err - } - return sigBuffer.Bytes(), nil -} - -// Verify implements SigningMechanism.Verify -func (m gpgSigningMechanism) Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error) { - signedBuffer := bytes.Buffer{} - signedData, err := gpgme.NewDataWriter(&signedBuffer) - if err != nil { - return nil, "", err - } - unverifiedSignatureData, err := gpgme.NewDataBytes(unverifiedSignature) - if err != nil { - return nil, "", err - } - _, sigs, err := m.ctx.Verify(unverifiedSignatureData, nil, signedData) - if err != nil { - return nil, "", err - } - if len(sigs) != 1 { - return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Unexpected GPG signature count %d", len(sigs))} - } - sig := sigs[0] - // This is sig.Summary == gpgme.SigSumValid except for key trust, which we handle ourselves - if sig.Status != nil || sig.Validity == gpgme.ValidityNever || sig.ValidityReason != nil || sig.WrongKeyUsage { - // FIXME: Better error reporting eventually - return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Invalid GPG signature: %#v", sig)} - } - return signedBuffer.Bytes(), sig.Fingerprint, nil +// NewEphemeralGPGSigningMechanism returns a new GPG/OpenPGP signing mechanism which +// recognizes _only_ public keys from the supplied blob, and returns the identities +// of these keys. +// The caller must call .Close() on the returned SigningMechanism. +func NewEphemeralGPGSigningMechanism(blob []byte) (SigningMechanism, []string, error) { + return newEphemeralGPGSigningMechanism(blob) } -// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, +// gpgUntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, // along with a short identifier of the key used for signing. // WARNING: The short key identifier (which correponds to "Key ID" for OpenPGP keys) // is NOT the same as a "key identity" used in other calls ot this interface, and // the values may have no recognizable relationship if the public key is not available. -func (m gpgSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { +func gpgUntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { // This uses the Golang-native OpenPGP implementation instead of gpgme because we are not doing any cryptography. md, err := openpgp.ReadMessage(bytes.NewReader(untrustedSignature), openpgp.EntityList{}, nil, nil) if err != nil { diff --git a/signature/mechanism_gpgme.go b/signature/mechanism_gpgme.go new file mode 100644 index 0000000000..4825ab27c6 --- /dev/null +++ b/signature/mechanism_gpgme.go @@ -0,0 +1,175 @@ +// +build !containers_image_openpgp + +package signature + +import ( + "bytes" + "fmt" + "io/ioutil" + "os" + + "github.com/mtrmac/gpgme" +) + +// A GPG/OpenPGP signing mechanism, implemented using gpgme. +type gpgmeSigningMechanism struct { + ctx *gpgme.Context + ephemeralDir string // If not "", a directory to be removed on Close() +} + +// newGPGSigningMechanismInDirectory returns a new GPG/OpenPGP signing mechanism, using optionalDir if not empty. +// The caller must call .Close() on the returned SigningMechanism. +func newGPGSigningMechanismInDirectory(optionalDir string) (SigningMechanism, error) { + ctx, err := newGPGMEContext(optionalDir) + if err != nil { + return nil, err + } + return &gpgmeSigningMechanism{ + ctx: ctx, + ephemeralDir: "", + }, nil +} + +// newEphemeralGPGSigningMechanism returns a new GPG/OpenPGP signing mechanism which +// recognizes _only_ public keys from the supplied blob, and returns the identities +// of these keys. +// The caller must call .Close() on the returned SigningMechanism. +func newEphemeralGPGSigningMechanism(blob []byte) (SigningMechanism, []string, error) { + dir, err := ioutil.TempDir("", "containers-ephemeral-gpg-") + if err != nil { + return nil, nil, err + } + removeDir := true + defer func() { + if removeDir { + os.RemoveAll(dir) + } + }() + ctx, err := newGPGMEContext(dir) + if err != nil { + return nil, nil, err + } + mech := &gpgmeSigningMechanism{ + ctx: ctx, + ephemeralDir: dir, + } + keyIdentities, err := mech.importKeysFromBytes(blob) + if err != nil { + return nil, nil, err + } + + removeDir = false + return mech, keyIdentities, nil +} + +// newGPGMEContext returns a new *gpgme.Context, using optionalDir if not empty. +func newGPGMEContext(optionalDir string) (*gpgme.Context, error) { + ctx, err := gpgme.New() + if err != nil { + return nil, err + } + if err = ctx.SetProtocol(gpgme.ProtocolOpenPGP); err != nil { + return nil, err + } + if optionalDir != "" { + err := ctx.SetEngineInfo(gpgme.ProtocolOpenPGP, "", optionalDir) + if err != nil { + return nil, err + } + } + ctx.SetArmor(false) + ctx.SetTextMode(false) + return ctx, nil +} + +func (m *gpgmeSigningMechanism) Close() error { + if m.ephemeralDir != "" { + os.RemoveAll(m.ephemeralDir) // Ignore an error, if any + } + return nil +} + +// importKeysFromBytes imports public keys from the supplied blob and returns their identities. +// The blob is assumed to have an appropriate format (the caller is expected to know which one). +// NOTE: This may modify long-term state (e.g. key storage in a directory underlying the mechanism); +// but we do not make this public, it can only be used through newEphemeralGPGSigningMechanism. +func (m *gpgmeSigningMechanism) importKeysFromBytes(blob []byte) ([]string, error) { + inputData, err := gpgme.NewDataBytes(blob) + if err != nil { + return nil, err + } + res, err := m.ctx.Import(inputData) + if err != nil { + return nil, err + } + keyIdentities := []string{} + for _, i := range res.Imports { + if i.Result == nil { + keyIdentities = append(keyIdentities, i.Fingerprint) + } + } + return keyIdentities, nil +} + +// SupportsSigning returns nil if the mechanism supports signing, or a SigningNotSupportedError. +func (m *gpgmeSigningMechanism) SupportsSigning() error { + return nil +} + +// Sign creates a (non-detached) signature of input using keyIdentity. +// Fails with a SigningNotSupportedError if the mechanism does not support signing. +func (m *gpgmeSigningMechanism) Sign(input []byte, keyIdentity string) ([]byte, error) { + key, err := m.ctx.GetKey(keyIdentity, true) + if err != nil { + return nil, err + } + inputData, err := gpgme.NewDataBytes(input) + if err != nil { + return nil, err + } + var sigBuffer bytes.Buffer + sigData, err := gpgme.NewDataWriter(&sigBuffer) + if err != nil { + return nil, err + } + if err = m.ctx.Sign([]*gpgme.Key{key}, inputData, sigData, gpgme.SigModeNormal); err != nil { + return nil, err + } + return sigBuffer.Bytes(), nil +} + +// Verify parses unverifiedSignature and returns the content and the signer's identity +func (m gpgmeSigningMechanism) Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error) { + signedBuffer := bytes.Buffer{} + signedData, err := gpgme.NewDataWriter(&signedBuffer) + if err != nil { + return nil, "", err + } + unverifiedSignatureData, err := gpgme.NewDataBytes(unverifiedSignature) + if err != nil { + return nil, "", err + } + _, sigs, err := m.ctx.Verify(unverifiedSignatureData, nil, signedData) + if err != nil { + return nil, "", err + } + if len(sigs) != 1 { + return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Unexpected GPG signature count %d", len(sigs))} + } + sig := sigs[0] + // This is sig.Summary == gpgme.SigSumValid except for key trust, which we handle ourselves + if sig.Status != nil || sig.Validity == gpgme.ValidityNever || sig.ValidityReason != nil || sig.WrongKeyUsage { + // FIXME: Better error reporting eventually + return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Invalid GPG signature: %#v", sig)} + } + return signedBuffer.Bytes(), sig.Fingerprint, nil +} + +// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, +// along with a short identifier of the key used for signing. +// WARNING: The short key identifier (which correponds to "Key ID" for OpenPGP keys) +// is NOT the same as a "key identity" used in other calls ot this interface, and +// the values may have no recognizable relationship if the public key is not available. +func (m gpgmeSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { + return gpgUntrustedSignatureContents(untrustedSignature) +} diff --git a/signature/mechanism_gpgme_test.go b/signature/mechanism_gpgme_test.go new file mode 100644 index 0000000000..9f5dc31bb4 --- /dev/null +++ b/signature/mechanism_gpgme_test.go @@ -0,0 +1,37 @@ +// +build !containers_image_openpgp + +package signature + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGPGMESigningMechanismClose(t *testing.T) { + // Closing an ephemeral mechanism removes the directory. + // (The non-ephemeral case is tested in the common TestGPGSigningMechanismClose) + mech, _, err := NewEphemeralGPGSigningMechanism([]byte{}) + require.NoError(t, err) + gpgMech, ok := mech.(*gpgmeSigningMechanism) + require.True(t, ok) + dir := gpgMech.ephemeralDir + assert.NotEmpty(t, dir) + _, err = os.Lstat(dir) + require.NoError(t, err) + err = mech.Close() + assert.NoError(t, err) + _, err = os.Lstat(dir) + require.Error(t, err) + assert.True(t, os.IsNotExist(err)) +} + +func TestGPGMESigningMechanismSupportsSigning(t *testing.T) { + mech, _, err := NewEphemeralGPGSigningMechanism([]byte{}) + require.NoError(t, err) + defer mech.Close() + err = mech.SupportsSigning() + assert.NoError(t, err) +} diff --git a/signature/mechanism_openpgp.go b/signature/mechanism_openpgp.go new file mode 100644 index 0000000000..e383bde3e6 --- /dev/null +++ b/signature/mechanism_openpgp.go @@ -0,0 +1,153 @@ +// +build containers_image_openpgp + +package signature + +import ( + "bytes" + "errors" + "fmt" + "io/ioutil" + "os" + "path" + "strings" + "time" + + "github.com/containers/storage/pkg/homedir" + "golang.org/x/crypto/openpgp" +) + +// A GPG/OpenPGP signing mechanism, implemented using x/crypto/openpgp. +type openpgpSigningMechanism struct { + keyring openpgp.EntityList +} + +// newGPGSigningMechanismInDirectory returns a new GPG/OpenPGP signing mechanism, using optionalDir if not empty. +// The caller must call .Close() on the returned SigningMechanism. +func newGPGSigningMechanismInDirectory(optionalDir string) (SigningMechanism, error) { + m := &openpgpSigningMechanism{ + keyring: openpgp.EntityList{}, + } + + gpgHome := optionalDir + if gpgHome == "" { + gpgHome = os.Getenv("GNUPGHOME") + if gpgHome == "" { + gpgHome = path.Join(homedir.Get(), ".gnupg") + } + } + + pubring, err := ioutil.ReadFile(path.Join(gpgHome, "pubring.gpg")) + if err != nil { + if !os.IsNotExist(err) { + return nil, err + } + } else { + _, err := m.importKeysFromBytes(pubring) + if err != nil { + return nil, err + } + } + return m, nil +} + +// newEphemeralGPGSigningMechanism returns a new GPG/OpenPGP signing mechanism which +// recognizes _only_ public keys from the supplied blob, and returns the identities +// of these keys. +// The caller must call .Close() on the returned SigningMechanism. +func newEphemeralGPGSigningMechanism(blob []byte) (SigningMechanism, []string, error) { + m := &openpgpSigningMechanism{ + keyring: openpgp.EntityList{}, + } + keyIdentities, err := m.importKeysFromBytes(blob) + if err != nil { + return nil, nil, err + } + return m, keyIdentities, nil +} + +func (m *openpgpSigningMechanism) Close() error { + return nil +} + +// importKeysFromBytes imports public keys from the supplied blob and returns their identities. +// The blob is assumed to have an appropriate format (the caller is expected to know which one). +func (m *openpgpSigningMechanism) importKeysFromBytes(blob []byte) ([]string, error) { + keyring, err := openpgp.ReadKeyRing(bytes.NewReader(blob)) + if err != nil { + k, e2 := openpgp.ReadArmoredKeyRing(bytes.NewReader(blob)) + if e2 != nil { + return nil, err // The original error -- FIXME: is this better? + } + keyring = k + } + + keyIdentities := []string{} + for _, entity := range keyring { + if entity.PrimaryKey == nil { + // Coverage: This should never happen, openpgp.ReadEntity fails with a + // openpgp.errors.StructuralError instead of returning an entity with this + // field set to nil. + continue + } + // Uppercase the fingerprint to be compatible with gpgme + keyIdentities = append(keyIdentities, strings.ToUpper(fmt.Sprintf("%x", entity.PrimaryKey.Fingerprint))) + m.keyring = append(m.keyring, entity) + } + return keyIdentities, nil +} + +// SupportsSigning returns nil if the mechanism supports signing, or a SigningNotSupportedError. +func (m *openpgpSigningMechanism) SupportsSigning() error { + return SigningNotSupportedError("signing is not supported in github.com/containers/image built with the containers_image_openpgp build tag") +} + +// Sign creates a (non-detached) signature of input using keyIdentity. +// Fails with a SigningNotSupportedError if the mechanism does not support signing. +func (m *openpgpSigningMechanism) Sign(input []byte, keyIdentity string) ([]byte, error) { + return nil, SigningNotSupportedError("signing is not supported in github.com/containers/image built with the containers_image_openpgp build tag") +} + +// Verify parses unverifiedSignature and returns the content and the signer's identity +func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error) { + md, err := openpgp.ReadMessage(bytes.NewReader(unverifiedSignature), m.keyring, nil, nil) + if err != nil { + return nil, "", err + } + if !md.IsSigned { + return nil, "", errors.New("not signed") + } + content, err := ioutil.ReadAll(md.UnverifiedBody) + if err != nil { + // Coverage: md.UnverifiedBody.Read only fails if the body is encrypted + // (and possibly also signed, but it _must_ be encrypted) and the signing + // “modification detection code” detects a mismatch. But in that case, + // we would expect the signature verification to fail as well, and that is checked + // first. Besides, we are not supplying any decryption keys, so we really + // can never reach this “encrypted data MDC mismatch” path. + return nil, "", err + } + if md.SignatureError != nil { + return nil, "", fmt.Errorf("signature error: %v", md.SignatureError) + } + if md.SignedBy == nil { + return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Invalid GPG signature: %#v", md.Signature)} + } + if md.Signature.SigLifetimeSecs != nil { + expiry := md.Signature.CreationTime.Add(time.Duration(*md.Signature.SigLifetimeSecs) * time.Second) + if time.Now().After(expiry) { + return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Signature expired on %s", expiry)} + } + } + + // Uppercase the fingerprint to be compatible with gpgme + return content, strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint)), nil +} + +// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, +// along with a short identifier of the key used for signing. +// WARNING: The short key identifier (which correponds to "Key ID" for OpenPGP keys) +// is NOT the same as a "key identity" used in other calls ot this interface, and +// the values may have no recognizable relationship if the public key is not available. +func (m openpgpSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { + return gpgUntrustedSignatureContents(untrustedSignature) +} diff --git a/signature/mechanism_openpgp_test.go b/signature/mechanism_openpgp_test.go new file mode 100644 index 0000000000..cfaf937b25 --- /dev/null +++ b/signature/mechanism_openpgp_test.go @@ -0,0 +1,28 @@ +// +build containers_image_openpgp + +package signature + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOpenpgpSigningMechanismSupportsSigning(t *testing.T) { + mech, _, err := NewEphemeralGPGSigningMechanism([]byte{}) + require.NoError(t, err) + defer mech.Close() + err = mech.SupportsSigning() + assert.Error(t, err) + assert.IsType(t, SigningNotSupportedError(""), err) +} + +func TestOpenpgpSigningMechanismSign(t *testing.T) { + mech, _, err := NewEphemeralGPGSigningMechanism([]byte{}) + require.NoError(t, err) + defer mech.Close() + _, err = mech.Sign([]byte{}, TestKeyFingerprint) + assert.Error(t, err) + assert.IsType(t, SigningNotSupportedError(""), err) +} diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index 5aee945faa..53f2b3d9f0 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -1,9 +1,12 @@ package signature +// These tests are expected to pass unmodified for _both_ mechanism_gpgme.go and mechanism_openpgp.go. + import ( "bytes" "io/ioutil" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -14,27 +17,88 @@ const ( testGPGHomeDirectory = "./fixtures" ) +func TestSigningNotSupportedError(t *testing.T) { + // A stupid test just to keep code coverage + s := "test" + err := SigningNotSupportedError(s) + assert.Equal(t, s, err.Error()) +} + func TestNewGPGSigningMechanism(t *testing.T) { // A dumb test just for code coverage. We test more with newGPGSigningMechanismInDirectory(). - _, err := NewGPGSigningMechanism() + mech, err := NewGPGSigningMechanism() assert.NoError(t, err) + mech.Close() } func TestNewGPGSigningMechanismInDirectory(t *testing.T) { // A dumb test just for code coverage. - _, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) + mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) assert.NoError(t, err) + mech.Close() // The various GPG failure cases are not obviously easy to reach. -} -func TestGPGSigningMechanismImportKeysFromBytes(t *testing.T) { - testDir, err := ioutil.TempDir("", "gpg-import-keys") + // Test that using the default directory (presumably in user’s home) + // cannot use TestKeyFingerprint. + signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") + require.NoError(t, err) + mech, err = newGPGSigningMechanismInDirectory("") + require.NoError(t, err) + defer mech.Close() + _, _, err = mech.Verify(signature) + assert.Error(t, err) + + // Similarly, using a newly created empty directory makes TestKeyFingerprint + // unavailable + emptyDir, err := ioutil.TempDir("", "signing-empty-directory") + require.NoError(t, err) + defer os.RemoveAll(emptyDir) + mech, err = newGPGSigningMechanismInDirectory(emptyDir) + require.NoError(t, err) + defer mech.Close() + _, _, err = mech.Verify(signature) + assert.Error(t, err) + + // If pubring.gpg is unreadable in the directory, either initializing + // the mechanism fails (with openpgp), or it succeeds (sadly, gpgme) and + // later verification fails. + unreadableDir, err := ioutil.TempDir("", "signing-unreadable-directory") + require.NoError(t, err) + defer os.RemoveAll(unreadableDir) + f, err := os.OpenFile(filepath.Join(unreadableDir, "pubring.gpg"), os.O_RDONLY|os.O_CREATE, 0000) + require.NoError(t, err) + f.Close() + mech, err = newGPGSigningMechanismInDirectory(unreadableDir) + if err == nil { + defer mech.Close() + _, _, err = mech.Verify(signature) + } + assert.Error(t, err) + + // Setting the directory parameter to testGPGHomeDirectory makes the key available. + mech, err = newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) - defer os.RemoveAll(testDir) + defer mech.Close() + _, _, err = mech.Verify(signature) + assert.NoError(t, err) - mech, err := newGPGSigningMechanismInDirectory(testDir) + // If we use the default directory mechanism, GNUPGHOME is respected. + origGNUPGHOME := os.Getenv("GNUPGHOME") + defer os.Setenv("GNUPGHOME", origGNUPGHOME) + os.Setenv("GNUPGHOME", testGPGHomeDirectory) + mech, err = newGPGSigningMechanismInDirectory("") require.NoError(t, err) + defer mech.Close() + _, _, err = mech.Verify(signature) + assert.NoError(t, err) +} +func TestNewEphemeralGPGSigningMechanism(t *testing.T) { + // Empty input: This is accepted anyway by GPG, just returns no keys. + mech, keyIdentities, err := NewEphemeralGPGSigningMechanism([]byte{}) + require.NoError(t, err) + defer mech.Close() + assert.Empty(t, keyIdentities) // Try validating a signature when the key is unknown. signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") require.NoError(t, err) @@ -44,31 +108,57 @@ func TestGPGSigningMechanismImportKeysFromBytes(t *testing.T) { // Successful import keyBlob, err := ioutil.ReadFile("./fixtures/public-key.gpg") require.NoError(t, err) - keyIdentities, err := mech.ImportKeysFromBytes(keyBlob) + mech, keyIdentities, err = NewEphemeralGPGSigningMechanism(keyBlob) require.NoError(t, err) + defer mech.Close() assert.Equal(t, []string{TestKeyFingerprint}, keyIdentities) - // After import, the signature should validate. content, signingFingerprint, err = mech.Verify(signature) require.NoError(t, err) assert.Equal(t, []byte("This is not JSON\n"), content) assert.Equal(t, TestKeyFingerprint, signingFingerprint) - // Two keys: just concatenate the valid input twice. - keyIdentities, err = mech.ImportKeysFromBytes(bytes.Join([][]byte{keyBlob, keyBlob}, nil)) + // Two keys: Read the binary-format pubring.gpg, and concatenate it twice. + // (Using two copies of public-key.gpg, in the ASCII-armored format, works with + // gpgmeSigningMechanism but not openpgpSigningMechanism.) + keyBlob, err = ioutil.ReadFile("./fixtures/pubring.gpg") + require.NoError(t, err) + mech, keyIdentities, err = NewEphemeralGPGSigningMechanism(bytes.Join([][]byte{keyBlob, keyBlob}, nil)) require.NoError(t, err) + defer mech.Close() assert.Equal(t, []string{TestKeyFingerprint, TestKeyFingerprint}, keyIdentities) - // Invalid input: This is accepted anyway by GPG, just returns no keys. - keyIdentities, err = mech.ImportKeysFromBytes([]byte("This is invalid")) - require.NoError(t, err) - assert.Equal(t, []string{}, keyIdentities) + // Invalid input: This is, sadly, accepted anyway by GPG, just returns no keys. + // For openpgpSigningMechanism we can detect this and fail. + mech, keyIdentities, err = NewEphemeralGPGSigningMechanism([]byte("This is invalid")) + assert.True(t, err != nil || len(keyIdentities) == 0) + if err == nil { + mech.Close() + } + assert.Empty(t, keyIdentities) // The various GPG/GPGME failures cases are not obviously easy to reach. } +func TestGPGSigningMechanismClose(t *testing.T) { + // Closing a non-ephemeral mechanism does not remove anything in the directory. + mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) + require.NoError(t, err) + err = mech.Close() + assert.NoError(t, err) + _, err = os.Lstat(testGPGHomeDirectory) + assert.NoError(t, err) + _, err = os.Lstat(filepath.Join(testGPGHomeDirectory, "pubring.gpg")) + assert.NoError(t, err) +} + func TestGPGSigningMechanismSign(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) + defer mech.Close() + + if err := mech.SupportsSigning(); err != nil { + t.Skipf("Signing not supported: %v", err) + } // Successful signing content := []byte("content") @@ -95,6 +185,7 @@ func assertSigningError(t *testing.T, content []byte, fingerprint string, err er func TestGPGSigningMechanismVerify(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) + defer mech.Close() // Successful verification signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") @@ -149,8 +240,9 @@ func TestGPGSigningMechanismVerify(t *testing.T) { } func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) { - mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) + mech, _, err := NewEphemeralGPGSigningMechanism([]byte{}) require.NoError(t, err) + defer mech.Close() // A valid signature signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index 9d914019b2..285b863007 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -5,7 +5,6 @@ package signature import ( "fmt" "io/ioutil" - "os" "strings" "github.com/pkg/errors" @@ -42,20 +41,11 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(image types.UnparsedImage, sig [ } // FIXME: move this to per-context initialization - dir, err := ioutil.TempDir("", "skopeo-signedBy-") - if err != nil { - return sarRejected, nil, err - } - defer os.RemoveAll(dir) - mech, err := newGPGSigningMechanismInDirectory(dir) - if err != nil { - return sarRejected, nil, err - } - - trustedIdentities, err := mech.ImportKeysFromBytes(data) + mech, trustedIdentities, err := NewEphemeralGPGSigningMechanism(data) if err != nil { return sarRejected, nil, err } + defer mech.Close() if len(trustedIdentities) == 0 { return sarRejected, nil, PolicyRequirementError("No public keys imported") } diff --git a/signature/signature.go b/signature/signature.go index 79174c845d..c13fd96c8b 100644 --- a/signature/signature.go +++ b/signature/signature.go @@ -252,10 +252,11 @@ func verifyAndExtractSignature(mech SigningMechanism, unverifiedSignature []byte // (including things like “✅ Verified by $authority”) func GetUntrustedSignatureInformationWithoutVerifying(untrustedSignatureBytes []byte) (*UntrustedSignatureInformation, error) { // NOTE: This should eventualy do format autodetection. - mech, err := NewGPGSigningMechanism() + mech, _, err := NewEphemeralGPGSigningMechanism([]byte{}) if err != nil { return nil, err } + defer mech.Close() untrustedContents, shortKeyIdentifier, err := mech.UntrustedSignatureContents(untrustedSignatureBytes) if err != nil { diff --git a/signature/signature_test.go b/signature/signature_test.go index 1afbfd6de5..711cd5798c 100644 --- a/signature/signature_test.go +++ b/signature/signature_test.go @@ -189,6 +189,11 @@ func TestUnmarshalJSON(t *testing.T) { func TestSign(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) + defer mech.Close() + + if err := mech.SupportsSigning(); err != nil { + t.Skipf("Signing not supported: %v", err) + } sig := newUntrustedSignature("digest!@#", "reference#@!") @@ -233,6 +238,7 @@ func TestSign(t *testing.T) { func TestVerifyAndExtractSignature(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) + defer mech.Close() type triple struct { keyIdentity string