Update a cancelableReader importing gpg keys

Signed-off-by: Qi Wang <[email protected]>

Author: QiWang19

image/signature/mechanism.go

@@ -4,6 +4,7 @@ package signature
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -78,7 +79,7 @@ func NewGPGSigningMechanism() (SigningMechanism, error) {
 // of these keys.
 // The caller must call .Close() on the returned SigningMechanism.
 func NewEphemeralGPGSigningMechanism(blob []byte) (SigningMechanism, []string, error) {
-	return newEphemeralGPGSigningMechanism([][]byte{blob})
+	return newEphemeralGPGSigningMechanism(context.Background(), [][]byte{blob})
 }
 
 // gpgUntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION,

image/signature/mechanism_gpgme_only.go

@@ -3,6 +3,9 @@
 package signature
 
 import (
+	"bytes"
+	"context"
+	"io"
 	"os"
 
 	"github.com/proglottis/gpgme"
@@ -12,7 +15,7 @@ import (
 // recognizes _only_ public keys from the supplied blobs, and returns the identities
 // of these keys.
 // The caller must call .Close() on the returned SigningMechanism.
-func newEphemeralGPGSigningMechanism(blobs [][]byte) (signingMechanismWithPassphrase, []string, error) {
+func newEphemeralGPGSigningMechanism(ctx context.Context, blobs [][]byte) (signingMechanismWithPassphrase, []string, error) {
 	dir, err := os.MkdirTemp("", "containers-ephemeral-gpg-")
 	if err != nil {
 		return nil, nil, err
@@ -23,34 +26,61 @@ func newEphemeralGPGSigningMechanism(blobs [][]byte) (signingMechanismWithPassph
 			os.RemoveAll(dir)
 		}
 	}()
-	ctx, err := newGPGMEContext(dir)
+	gpgmeCtx, err := newGPGMEContext(dir)
 	if err != nil {
 		return nil, nil, err
 	}
 	keyIdentities := []string{}
 	for _, blob := range blobs {
-		ki, err := importKeysFromBytes(ctx, blob)
+		ki, err := importKeysFromBytes(ctx, gpgmeCtx, blob)
 		if err != nil {
 			return nil, nil, err
 		}
 		keyIdentities = append(keyIdentities, ki...)
 	}
 
-	mech := newGPGMESigningMechanism(ctx, dir)
+	mech := newGPGMESigningMechanism(gpgmeCtx, dir)
 	removeDir = false
 	return mech, keyIdentities, nil
 }
 
+// cancelableReader wraps an io.Reader and checks context cancellation on each Read call.
+type cancelableReader struct {
+	ctx    context.Context
+	reader io.Reader
+}
+
+func (r *cancelableReader) Read(p []byte) (int, error) {
+	// Check if context is cancelled before each read
+	if err := r.ctx.Err(); err != nil {
+		return 0, err
+	}
+	n, err := r.reader.Read(p)
+	// Check again after read in case cancellation happened during the read
+	if err == nil && r.ctx.Err() != nil {
+		return n, r.ctx.Err()
+	}
+	return n, err
+}
+
 // 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 importKeysFromBytes(ctx *gpgme.Context, blob []byte) ([]string, error) {
-	inputData, err := gpgme.NewDataBytes(blob)
+// The context can be used to cancel the operation; if cancelled, the reader will return an error
+// which may allow the GPGME operation to abort (though there's no guarantee the C library will
+// respect this immediately).
+func importKeysFromBytes(ctx context.Context, gpgmeCtx *gpgme.Context, blob []byte) ([]string, error) {
+	// Create a cancelable reader that checks context on each Read call
+	reader := &cancelableReader{
+		ctx:    ctx,
+		reader: bytes.NewReader(blob),
+	}
+	inputData, err := gpgme.NewDataReader(reader)
 	if err != nil {
 		return nil, err
 	}
-	res, err := ctx.Import(inputData)
+	res, err := gpgmeCtx.Import(inputData)
 	if err != nil {
 		return nil, err
 	}

image/signature/mechanism_openpgp.go

@@ -4,6 +4,7 @@ package signature
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -63,7 +64,9 @@ func newGPGSigningMechanismInDirectory(optionalDir string) (signingMechanismWith
 // 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(blobs [][]byte) (signingMechanismWithPassphrase, []string, error) {
+// The context parameter is accepted for API consistency but is not used by this implementation.
+func newEphemeralGPGSigningMechanism(ctx context.Context, blobs [][]byte) (signingMechanismWithPassphrase, []string, error) {
+	_ = ctx // Context not used in this implementation
 	m := &openpgpSigningMechanism{
 		keyring: openpgp.EntityList{},
 	}

image/signature/mechanism_sequoia.go

@@ -3,6 +3,8 @@
 package signature
 
 import (
+	"context"
+
 	"go.podman.io/image/v5/signature/internal/sequoia"
 )
 
@@ -18,7 +20,9 @@ type sequoiaEphemeralSigningMechanism struct {
 // recognizes _only_ public keys from the supplied blobs, and returns the identities
 // of these keys.
 // The caller must call .Close() on the returned SigningMechanism.
-func newEphemeralGPGSigningMechanism(blobs [][]byte) (signingMechanismWithPassphrase, []string, error) {
+// The context parameter is accepted for API consistency but is not used by this implementation.
+func newEphemeralGPGSigningMechanism(ctx context.Context, blobs [][]byte) (signingMechanismWithPassphrase, []string, error) {
+	_ = ctx // Context not used in this implementation
 	if err := sequoia.Init(); err != nil {
 		return nil, nil, err // Coverage: This is impractical to test in-process, with the static go_sequoia_dlhandle.
 	}

image/signature/mechanism_test.go

@@ -4,6 +4,7 @@ package signature
 
 import (
 	"bytes"
+	"context"
 	"os"
 	"path/filepath"
 	"slices"
@@ -175,7 +176,7 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) {
 	require.NoError(t, err)
 	keyBlob2, err := os.ReadFile("./fixtures/public-key-2.gpg")
 	require.NoError(t, err)
-	mech, keyIdentities, err = newEphemeralGPGSigningMechanism([][]byte{keyBlob1, keyBlob2})
+	mech, keyIdentities, err = newEphemeralGPGSigningMechanism(context.Background(), [][]byte{keyBlob1, keyBlob2})
 	require.NoError(t, err)
 	defer mech.Close()
 	assert.Equal(t, []string{TestKeyFingerprint, TestKeyFingerprintWithPassphrase}, keyIdentities)

image/signature/policy_eval_signedby.go

@@ -6,13 +6,16 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"time"
 
 	digest "github.com/opencontainers/go-digest"
 	"go.podman.io/image/v5/internal/multierr"
 	"go.podman.io/image/v5/internal/private"
 	"go.podman.io/image/v5/manifest"
 )
 
+const importKeyTimeOut = 60 * time.Second
+
 func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image private.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) {
 	switch pr.KeyType {
 	case SBKeyTypeGPGKeys:
@@ -40,9 +43,58 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva
 	}
 
 	// FIXME: move this to per-context initialization
-	mech, trustedIdentities, err := newEphemeralGPGSigningMechanism(data)
-	if err != nil {
-		return sarRejected, nil, err
+	// Import the keys with a 60s timeout to avoid hanging indefinitely. see issues.redhat.com/browse/OCPBUGS-57893
+	// Create a context with timeout that can be used to cancel the operation via the cancelable reader
+	importCtx, cancel := context.WithTimeout(ctx, importKeyTimeOut)
+	defer cancel()
+
+	// Use a goroutine with timeout to handle cases where GPGME hangs in C library calls
+	// that don't read from the data source (where the cancelable reader can't help).
+	// The cancelable reader provides faster cancellation when GPGME is reading, but we still
+	// need the timeout goroutine to detect hangs and clean up resources.
+	type result struct {
+		mech signingMechanismWithPassphrase
+		keys []string
+		err  error
+	}
+	// Use a buffered channel so the goroutine can always send without blocking
+	done := make(chan result, 1)
+
+	go func() {
+		mech, keys, err := newEphemeralGPGSigningMechanism(importCtx, data)
+		done <- result{mech, keys, err}
+	}()
+
+	var mech signingMechanismWithPassphrase
+	var trustedIdentities []string
+
+	select {
+	case <-importCtx.Done():
+		// Timeout occurred. The goroutine is still running and may complete later.
+		// Spawn a cleanup goroutine to handle the result when it arrives and clean up
+		// any resources that were created, preventing leaks.
+		go func() {
+			// Wait for the operation to complete (with a reasonable upper bound
+			// to avoid waiting forever if something goes very wrong)
+			select {
+			case r := <-done:
+				// The operation completed. If a mechanism was created, close it
+				// to free file descriptors and other resources.
+				if r.mech != nil {
+					r.mech.Close()
+				}
+			case <-time.After(5 * time.Minute):
+				// If it still hasn't completed after 5 minutes, give up on cleanup.
+				// The goroutine will continue running, but at least we've tried.
+			}
+		}()
+		return sarRejected, nil, fmt.Errorf("GPG/OpenPGP key import timed out after %s", importKeyTimeOut)
+	case r := <-done:
+		mech = r.mech
+		trustedIdentities = r.keys
+		if r.err != nil {
+			return sarRejected, nil, r.err
+		}
 	}
 	defer mech.Close()
 	if len(trustedIdentities) == 0 {