Content type mismatch does not cancel image pull #20343

sanmai-NL · 2023-10-12T08:01:36Z

Issue Description

See title.

I see a possible security impact since this increases the attack surface for parser/deserializer weaknesses. The current behavior also is not in line with the fail-fast principle.

This issue occurs in practice even without a misconfiguration by a user, whenever GitLab's Container Registry is turned off or suddenly unavailable due to GitLab platform behavior, image fetching endpoints return HTML content.

Steps to reproduce the issue

Run podman build with a Containerfile with a FROM instruction that ends up sending an HTTP requesting that gets an HTML response.

Describe the results you received

time="2023-10-12T07:38:21Z" level=debug msg="Normalized platform linux/amd64 to {amd64 linux  [] }"
time="2023-10-12T07:38:21Z" level=debug msg="base for stage 0: \"${IMAGE_PREFIX}alpine:edge\""
time="2023-10-12T07:38:21Z" level=debug msg="FROM \"gitlab.com:443/blah/dependency_proxy/containers/alpine:edge AS builder\""
STEP 1/9: FROM gitlab.com:443/blah/dependency_proxy/containers/alpine:edge AS builder
...
time="2023-10-12T07:38:21Z" level=debug msg="Increasing token expiration to: 60 seconds"
time="2023-10-12T07:38:21Z" level=debug msg="GET https://gitlab.com:443/v2/blah/dependency_proxy/containers/alpine/manifests/edge"
time="2023-10-12T07:38:21Z" level=debug msg="Content-Type from manifest GET is \"text/html; charset=utf-8\""
time="2023-10-12T07:38:21Z" level=debug msg="Accessing \"gitlab.com:443/blah/dependency_proxy/containers/alpine:edge\" failed: reading manifest edge in gitlab.com:443/blah/dependency_proxy/containers/alpine: StatusCode: 404, <!DOCTYPE html>\n<html>\n<head>\n  <meta content=\"wid..."
time="2023-10-12T07:38:21Z" level=debug msg="Error pulling candidate gitlab.com:443/blah/dependency_proxy/containers/alpine:edge: initializing source docker://gitlab.com:443/blah/dependency_proxy/containers/alpine:edge: reading manifest edge in gitlab.com:443/blah/dependency_proxy/containers/alpine: StatusCode: 404, <!DOCTYPE html>\n<html>\n<head>\n  <meta content=\"wid..."

Describe the results you expected

podman build already determines the content type (see log line of 2023-10-12T07:38:21Z), and after it finds an unexpected content type, such as any subtype of text, cancel the pulling operation and raise a fault.

podman info output

I have this issue with the latest Podman image on Quay. Sorry, not able to provide more details now.

Podman in a container

Yes

Privileged Or Rootless

Rootless

Upstream Latest Release

Yes

Additional environment details

No response

Additional information

No response

The text was updated successfully, but these errors were encountered:

rhatdan · 2023-10-12T14:13:34Z

@vrothberg What do you think about adding a containers.conf setting to force failing on pulling mismatch?

vrothberg · 2023-10-13T07:43:25Z

Thanks for reaching out, @sanmai-NL !

time="2023-10-12T07:38:21Z" level=debug msg="Content-Type from manifest GET is "text/html; charset=utf-8""
time="2023-10-12T07:38:21Z" level=debug msg="Accessing "gitlab.com:443/blah/dependency_proxy/containers/alpine:edge" failed: reading manifest edge in gitlab.com:443/blah/dependency_proxy/containers/alpine: StatusCode: 404, \n\n\n <meta content="wid..."

@mtrmac is there a reason to continue pulling in that case? It is surprising at first glance but I may be missing background.

I see a possible security impact since this increases the attack surface for parser/deserializer weaknesses.

Does it really increase the attack surface? A server could very well set the content type to application/json and return the same html content.

sanmai-NL · 2023-10-13T11:57:05Z

@vrothberg Yes, it's a minor increase, I suppose. Once an attacker can alter the content but not headers of any HTTP endpoint, e.g., on a service otherwise normally intended as image registry, e.g. compromising a web app template to include a file on some unrelated path (route) on the GitLab instance, then the lack of content type filtering means an increased attack surface.

mtrmac · 2023-10-13T18:21:55Z

I don’t see anything to indicate “does not cancel image pull ”; the image pull does fail, and stop, doesn’t it?

What happens here is:

There is a debug log line which includes the contents of the Content-Type header. That debug log line exists exactly to immediately surface similar situations, where the server just isn’t EDIT implementing a protocol at all. But the header is not actually used at that point.
Afterwards, a HTTP status check finds status 404, and returns an “image not found” failure. The error is correctly detected and correctly handled, it seems to me.

And this order of operations is, I think, correct: The code could check for status 404 first, and terminate without logging the Content-Type value, but that would be very confusing exactly in the relevant situations: users could think that they have correctly deployed a registry on that URL, and that the contents of the registry are incorrect and need to be changed on the registry. The Content-Type indication here suggests that it’s the setup of the server, not the contents, that needs fixing.

If anything, I think the code should be parsing the values more, to return different “a registry exists, but the image does not” vs. “this is not a registry at all” errors.

github-actions · 2023-11-13T00:07:01Z

A friendly reminder that this issue had no activity for 30 days.

sanmai-NL · 2023-11-13T09:35:33Z

@mtrmac I read the log line at time="2023-10-12T07:38:21Z" as, now I'm reading the response data even if it makes no sense because the response code indicated an error.

I'm not suggesting to not log the content type, just not the data/contents.

Can you explain your last sentence? You needn't parse data to discern an unreachable registry from an invalid endpoint.

mtrmac · 2023-11-13T18:31:40Z

“an unreachable registry” does not play a role here; “this is not a registry at all” (but a website) is not the same thing as “an unreachable registry”.

mtrmac · 2023-11-13T18:43:19Z

As a general matter, “reading and logging data from a server increases an attack surface”… is not untrue but it’s also not something I worry about. It is inherently happening every time a server reports an error text and that error text is surfaced to the user (in fact that’s much worse, because the text passes through Unicode layout and rendering, and the text is human-comprehensible and it could convince the human to violate the security policy of the system).

There’s some “background level” of risk inherent in using client-server applications and it’s just not practical to avoid it without compromising other things users care about.

In fact, the response body even goes through two JSON parse operations before it becomes this “unexpected response, here’s the prefix of the body” error message. In that context, it just doesn’t make sense to me to worry about the “increased attack surface” of response[:50] + "..." and fmt.Errorf("…%s…").

I’ll make that even worse: with containers/image#2186 , the text goes through an extra quoting stage. Does that increase an attack surface? Yes, but also, no.

sanmai-NL added the kind/bug Categorizes issue or PR as related to a bug. label Oct 12, 2023

sanmai-NL changed the title ~~Content type mismatch does not preempt image pull~~ Content type mismatch does not cancel image pull Oct 12, 2023

github-actions bot added the stale-issue label Nov 13, 2023

rhatdan removed the stale-issue label Nov 13, 2023

mtrmac mentioned this issue Nov 13, 2023

Quote the response body in an error message containers/image#2186

Merged

mtrmac closed this as completed Nov 13, 2023

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Feb 12, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Content type mismatch does not cancel image pull #20343

Content type mismatch does not cancel image pull #20343

sanmai-NL commented Oct 12, 2023 •

edited

rhatdan commented Oct 12, 2023

vrothberg commented Oct 13, 2023

sanmai-NL commented Oct 13, 2023

mtrmac commented Oct 13, 2023 •

edited

github-actions bot commented Nov 13, 2023

sanmai-NL commented Nov 13, 2023 •

edited

mtrmac commented Nov 13, 2023

mtrmac commented Nov 13, 2023

Content type mismatch does not cancel image pull #20343

Content type mismatch does not cancel image pull #20343

Comments

sanmai-NL commented Oct 12, 2023 • edited

Issue Description

Steps to reproduce the issue

Describe the results you received

Describe the results you expected

podman info output

Podman in a container

Privileged Or Rootless

Upstream Latest Release

Additional environment details

Additional information

rhatdan commented Oct 12, 2023

vrothberg commented Oct 13, 2023

sanmai-NL commented Oct 13, 2023

mtrmac commented Oct 13, 2023 • edited

github-actions bot commented Nov 13, 2023

sanmai-NL commented Nov 13, 2023 • edited

mtrmac commented Nov 13, 2023

mtrmac commented Nov 13, 2023

sanmai-NL commented Oct 12, 2023 •

edited

mtrmac commented Oct 13, 2023 •

edited

sanmai-NL commented Nov 13, 2023 •

edited