Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New app user for .NET 8 images isn't documented as a potentially breaking change #4451

Closed
rwkarg opened this issue Feb 23, 2023 · 12 comments
Closed
Assignees
Milestone

Comments

@rwkarg
Copy link

rwkarg commented Feb 23, 2023

Describe the Bug

The new app user for .NET 8 images (#4397) is an undocumented breaking change for users that are currently creating a user named app on top of .NET 7 and earlier images.

Documented breaking changes

Steps to Reproduce

Have a Dockerfile that builds off of one of the Microsoft images (ex. dotnet/aspnet) in use with .NET 7 or earlier tags

ARG TAG_VERSION
FROM mcr.microsoft.com/dotnet/aspnet:${TAG_VERSION}

# Minimal steps to produce error
RUN groupadd -r app && \
    useradd -m -u 10005 -r -g app -d /home/app -s /sbin/nologin -c "Docker image user" app
USER app

Now Set TAG_VERSION to a .NET 8 tag (ex. 8.0.0-preview.1) and docker build

--> Container image build error since app user already exists.

Other Information

Preferred Outcome: Use a higher number user id (>10,000) and document how this addition may break existing workflows when upgrading to .NET 8 where an app user is currently being created on top of .NET 7 or earlier tags.

We currently build images off the base Microsoft images to add a non-root user (as well as other unrelated specializations not relevant to this issue) and just happen to currently use app as the user name.

We could address this by omitting the RUN step to add a user and use the provided app user, but that still runs in to an issue with our deployments to Kubernetes. The securityContext for the pods/containers specify a runAsUser that requires the user id.

The .NET 8 images create the user with id 101 which will not work for our existing deployments. This would require coordinating our .NET 8 upgrades, per service, with similar updates to our deployments to align the ids.

Additionally, recommendations are to use a high numbered user id (>10,000 is a recommendation I've seen in several places, including the security scanning tool Trivy regarding this specific vulnerability)

Blog post regarding container user colliding with a host user having the same id: Understanding how uid and gid work in Docker containers

The other way we could address this is to remove the new app user and then continue with creating our own app as before, but this is also a breaking change to our container build process that we need to address to upgrade.

@richlander
Copy link
Member

richlander commented Feb 23, 2023

Context we should consider: https://en.wikipedia.org/wiki/User_identifier#Reserved_ranges

Suggestion from some folks at MSFT:

go for an overridable ID and default it to something like 2^32-1 - SMALL_FIXED_VALUE
That way you are always far from everything else and you can allow the user to choose the ID.

Does that sound interesting and tenable?

@rwkarg
Copy link
Author

rwkarg commented Feb 24, 2023

If the image is prebuilt then it wouldn't be user modifiable but having an appropriately large value would allow us to remove one of the custom steps from our image (still need to build on top of the public image for our case to add other things like internal CA certificates).

@rwkarg
Copy link
Author

rwkarg commented Feb 24, 2023

The ideal case would be to have a non-root user be default (with a USER app in the container) and allow the few exceptional cases where a root user is explicitly desired a new image can be built on top to call USER root.

But the horse is already out of the barn and upgrading a container from root to non-root by default is a pretty big breaking change in many cases (existing file permissions, privileged operations being used, etc.).

Another option is to have a separate image that is non-root, though the mariner/chiseled images are kind of that already, so it probably doesn't make sense to have a set of 8.0.0.nonroot-preview.1 tags to mirror the existing ones.

@richlander
Copy link
Member

You have captured the situation well. It aligns with past conversations we have had.

We want to enable non-root across the whole suite of images. It will need to be opt-in (for the reason you say).

It is just a question of what it should be called and what uid it should use. We wanted to call it something general, which how we ended up with 'app'. We could switch to 'dotnet'. That felt odd to us (for a variety of reasons) but it is an option and we could more readily definitely claim we have the right to run with that image. In terms of uid, we could go with something much higher and a bit more random like: 10101010 (a byte worth).

@rwkarg
Copy link
Author

rwkarg commented Feb 24, 2023

No issue with app. It's what we happened to end up going with, too.

Other than being "large enough" and outside of the reserved ranges you posted above, I don't see any real advantage of any specific value.

Does feel like an opportunity to use something like 5031337

Or maybe net converting ASCII to decimal is 110,101,116 (both .net and dotnet would be too large).

The final piece would be documenting the breaking change for cases where there's an existing app user being defined and directing to one of the workarounds:

  • delete the .NET 8 app user before creating it in the custom image layers
  • use the existing .NET 8 app user and adjust deployment scripts, deployment manifests, file permissions, or other settings that require a specific user id

@richlander
Copy link
Member

Yup. That all sounds good.

Does feel like an opportunity to use something like 5031337

Damn! You are good.

I'll see if I can get my team to be that cheeky.

Or maybe net converting ASCII

Was thinking along the same lines.

documenting the breaking change

yup.

@richlander
Copy link
Member

Here's something reasonable.

>>> 2**16-1-1337
64198

@mthalman mthalman moved this from Backlog to On Deck in .NET Docker Mar 4, 2023
@mthalman mthalman added this to the .NET 8 milestone Mar 4, 2023
@lbussell lbussell moved this from On Deck to In Progress in .NET Docker Mar 6, 2023
@richlander
Copy link
Member

FYI: We got feedback from Canonical that the following range would be good: 60500-65519.

@lbussell
Copy link
Contributor

lbussell commented Mar 9, 2023

The UID has been changed to 64198 in #4476.

@richlander
Copy link
Member

richlander commented Mar 14, 2023

See:

$ docker run --rm mcr.microsoft.com/dotnet/aspnet:8.0-preview cat /etc/passwd | tail -n 1
app:x:64198:64198::/home/app:/bin/sh

Better?

@solvingproblemswithtechnology
Copy link

solvingproblemswithtechnology commented Jun 26, 2023

This also impacted us, although the behavior was way worse. It basically broke the docker engine in our pipelines Agents. They key point is that we was caching the nuget over compilations using --mount=type=cache,id=nuget,target=/root/.nuget/packages. Notice the path starting on /root. The agent will stuck for almost 20 minutes and after it the docker engine will be in an incosistent state and producing errors until we reboot the agent, not only the docker engine.

I was using the following Dockerfile and it happens since the preview 5:

#See https://aka.ms/customizecontainer to learn how to customize your debug container and how Visual Studio uses this Dockerfile to build your images for faster debugging.

FROM mcr.microsoft.com/dotnet/runtime-deps:8.0-preview-alpine AS base
WORKDIR /app
EXPOSE 80

RUN apk add --no-cache icu-libs krb5-libs libgcc libintl libssl1.1 libstdc++ zlib

# Pretty logs for usage with Lens or others
ENV Logging__Console__FormatterName=Simple
ENV Logging__Console__FormatterOptions__ColorBehavior=Enabled
ENV ASPNETCORE_HTTP_PORTS=80
ENV DOTNET_RUNNING_IN_CONTAINER=true
ENV DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=false

# Refactor into generic scripts out of the project
COPY ./SolutionFiles/curl-amd64 .
COPY ./SolutionFiles/warmup-startup.sh .

## Copy the csprojs preserving paths
FROM busybox:1.35 as source
WORKDIR /code
COPY . /code
RUN mkdir /csprojs; for f in */*.csproj; do cp --parents ${f} /csprojs; done;

# Restore the packages and cache it
FROM mcr.microsoft.com/dotnet/sdk:8.0-preview-alpine AS packages
WORKDIR /nugets
COPY --from=source csprojs ./
RUN --mount=type=cache,id=nuget,target=/root/.nuget/packages \
	dotnet restore "MyProject.Backend/MyProject.Backend.csproj" -r alpine-x64 -p:PublishReadyToRun=true

# Publish
FROM packages AS publish

WORKDIR /src
COPY --from=source code ./
# Merge already restored nugets into src, avoiding COPY from overwritting them
RUN (cd /nugets && tar c .) | (cd /src && tar xf -)

# PublishTrimmed and LinkDuringPublish further reduce the size, at the expense of slower compilation. Comment them if there's no issue with that.
	#-p:PublishTrimmed=true -p:LinkDuringPublish=true \ 
WORKDIR /src/MyProject.Backend
RUN --mount=type=cache,id=nuget,target=/root/.nuget/packages \
	dotnet publish "MyProject.Backend.csproj" -r alpine-x64 -c Release --sc -o /app/publish --no-restore \
		-p:PublishSingleFile=true -p:PublishReadyToRun=true \
		-p:ServerGarbageCollection=false -p:ConcurrentGarbageCollection=true

FROM base AS final
COPY --from=publish /app/publish .

ENTRYPOINT ["./MyProject.Backend"]

Leaving this here as feedback and just in case happens to another team.

@mthalman
Copy link
Member

@smartcodinghub - This doesn't seem related to the addition of the app user. The container is still set to use the root user by default. You haven't opted into it in your Dockerfile. Something else is going on here. Have you resolved the issue and what did you do to get it to work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

5 participants