diff --git a/image/copy/single.go b/image/copy/single.go index 588ad9ab3a..7cef1d3ecd 100644 --- a/image/copy/single.go +++ b/image/copy/single.go @@ -12,6 +12,7 @@ import ( "slices" "strings" "sync" + "time" digest "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -30,6 +31,8 @@ import ( chunkedToc "go.podman.io/storage/pkg/chunked/toc" ) +const policyEvaluationTimeout = 60 * time.Second + // imageCopier tracks state specific to a single image (possibly an item of a manifest list) type imageCopier struct { c *copier @@ -75,7 +78,11 @@ func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.Unpar // Please keep this policy check BEFORE reading any other information about the image. // (The multiImage check above only matches the MIME type, which we have received anyway. // Actual parsing of anything should be deferred.) - if allowed, err := c.policyContext.IsRunningImageAllowed(ctx, unparsedImage); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. + // Apply a timeout to avoid hanging indefinitely on GPG key import operations. see issues.redhat.com/browse/OCPBUGS-57893 + // The timeout context will be used by the cancelable reader to potentially abort GPGME operations. + policyCtx, cancel := context.WithTimeout(ctx, policyEvaluationTimeout) + defer cancel() + if allowed, err := c.policyContext.IsRunningImageAllowed(policyCtx, unparsedImage); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. return copySingleImageResult{}, fmt.Errorf("Source image rejected: %w", err) } src, err := image.FromUnparsedImage(ctx, c.options.SourceCtx, unparsedImage) diff --git a/image/signature/mechanism.go b/image/signature/mechanism.go index 897fc49971..e90e68552e 100644 --- a/image/signature/mechanism.go +++ b/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, diff --git a/image/signature/mechanism_gpgme_only.go b/image/signature/mechanism_gpgme_only.go index 0f971ac6a4..ca1fe38a74 100644 --- a/image/signature/mechanism_gpgme_only.go +++ b/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 } diff --git a/image/signature/mechanism_openpgp.go b/image/signature/mechanism_openpgp.go index 2f1b99d18c..aafbcfa3b7 100644 --- a/image/signature/mechanism_openpgp.go +++ b/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{}, } diff --git a/image/signature/mechanism_sequoia.go b/image/signature/mechanism_sequoia.go index 0a6f002f27..8af815e20f 100644 --- a/image/signature/mechanism_sequoia.go +++ b/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. } diff --git a/image/signature/mechanism_test.go b/image/signature/mechanism_test.go index 445b9ed4cf..bf23a90d8c 100644 --- a/image/signature/mechanism_test.go +++ b/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) diff --git a/image/signature/policy_eval_signedby.go b/image/signature/policy_eval_signedby.go index 149545da8f..e9cad3225c 100644 --- a/image/signature/policy_eval_signedby.go +++ b/image/signature/policy_eval_signedby.go @@ -40,7 +40,7 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva } // FIXME: move this to per-context initialization - mech, trustedIdentities, err := newEphemeralGPGSigningMechanism(data) + mech, trustedIdentities, err := newEphemeralGPGSigningMechanism(ctx, data) if err != nil { return sarRejected, nil, err }