diff --git a/copy/single.go b/copy/single.go index ba414a22d6..6f8e1f826c 100644 --- a/copy/single.go +++ b/copy/single.go @@ -11,6 +11,7 @@ import ( "slices" "strings" "sync" + "time" "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/internal/image" @@ -74,7 +75,13 @@ 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. + // Create a timeout context for GPG key import operations to prevent indefinite hanging. + // This timeout is passed down to the cancellable reader which will interrupt operations + // if they take too long, preventing file descriptor leaks. + const importTimeout = 60 * time.Second + policyCtx, cancel := context.WithTimeout(ctx, importTimeout) + 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/signature/mechanism.go b/signature/mechanism.go index 1d3fe0fdc9..d84704abb5 100644 --- a/signature/mechanism.go +++ b/signature/mechanism.go @@ -4,6 +4,7 @@ package signature import ( "bytes" + "context" "errors" "fmt" "io" @@ -65,7 +66,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([][]byte{blob}, context.Background()) } // gpgUntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, diff --git a/signature/mechanism_gpgme.go b/signature/mechanism_gpgme.go index 2b2a7ad866..a666f3a282 100644 --- a/signature/mechanism_gpgme.go +++ b/signature/mechanism_gpgme.go @@ -5,8 +5,10 @@ package signature import ( "bytes" + "context" "errors" "fmt" + "io" "os" "github.com/containers/image/v5/signature/internal" @@ -36,7 +38,9 @@ func newGPGSigningMechanismInDirectory(optionalDir string) (signingMechanismWith // 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 is used for cancellation - when it times out or is cancelled, the +// cancellable reader will interrupt key import operations. +func newEphemeralGPGSigningMechanism(blobs [][]byte, ctx context.Context) (signingMechanismWithPassphrase, []string, error) { dir, err := os.MkdirTemp("", "containers-ephemeral-gpg-") if err != nil { return nil, nil, err @@ -47,17 +51,18 @@ func newEphemeralGPGSigningMechanism(blobs [][]byte) (signingMechanismWithPassph os.RemoveAll(dir) } }() - ctx, err := newGPGMEContext(dir) + gpgmeCtx, err := newGPGMEContext(dir) if err != nil { return nil, nil, err } mech := &gpgmeSigningMechanism{ - ctx: ctx, + ctx: gpgmeCtx, ephemeralDir: dir, } + keyIdentities := []string{} for _, blob := range blobs { - ki, err := mech.importKeysFromBytes(blob) + ki, err := mech.importKeysFromBytes(blob, ctx) if err != nil { return nil, nil, err } @@ -95,12 +100,41 @@ func (m *gpgmeSigningMechanism) Close() error { return nil } +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 (m *gpgmeSigningMechanism) importKeysFromBytes(blob []byte) ([]string, error) { - inputData, err := gpgme.NewDataBytes(blob) +// Uses a cancellable reader to prevent indefinite hanging and file descriptor leaks. +// The cancellable reader will return an error when cancellation is detected, causing Import to fail. +// The context is used for cancellation - when it times out or is cancelled, the reader will +// interrupt the operation. +func (m *gpgmeSigningMechanism) importKeysFromBytes(blob []byte, ctx context.Context) ([]string, error) { + // Create a cancellable reader that wraps the blob data + + reader := &cancelableReader{ + ctx: ctx, + reader: bytes.NewReader(blob), + } + + inputData, err := gpgme.NewDataReader(reader) if err != nil { return nil, err } diff --git a/signature/mechanism_openpgp.go b/signature/mechanism_openpgp.go index bd1979aefb..52fd5ebacb 100644 --- a/signature/mechanism_openpgp.go +++ b/signature/mechanism_openpgp.go @@ -5,6 +5,7 @@ package signature import ( "bytes" + "context" "errors" "fmt" "io" @@ -64,7 +65,8 @@ 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 compatibility but not used in this implementation. +func newEphemeralGPGSigningMechanism(blobs [][]byte, ctx context.Context) (signingMechanismWithPassphrase, []string, error) { m := &openpgpSigningMechanism{ keyring: openpgp.EntityList{}, } diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index ef67db6b99..68f83628df 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -4,6 +4,7 @@ package signature import ( "bytes" + "context" "os" "path/filepath" "testing" @@ -150,7 +151,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([][]byte{keyBlob1, keyBlob2}, context.Background()) require.NoError(t, err) defer mech.Close() assert.Equal(t, []string{TestKeyFingerprint, TestKeyFingerprintWithPassphrase}, keyIdentities) diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index e5c9329185..e3951d15b5 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -41,7 +41,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(data, ctx) if err != nil { return sarRejected, nil, err }