vendor: switch from idtools to moby/sys/user#5791
Conversation
c5768b4 to
1ba5aa4
Compare
| ContainerAdministratorSidString = "S-1-5-93-2-1" | ||
| ContainerUserSidString = "S-1-5-93-2-2" |
There was a problem hiding this comment.
I guess we need them because they were not ported to moby/sys? https://github.com/moby/moby/blob/7194b508b6766a372e42156bb3314288bd7fbd0c/pkg/idtools/idtools_windows.go#L11-L16.
@thaJeztah Is moby/sys not a good place in the meantime while waiting for a better place like hcsshim? (cc @gabriel-samfira @profnandaa @TBBle)
There was a problem hiding this comment.
Yes, we need a place for this ultimately. To my understanding (but hoping some of the Microsoft folks can confirm), these SIDs (and the related name) are "special" as they are ones that are available by default in containers.
What I don't know though, is where is that documented ? They were introduced in Moby during the implementation of container support for Windows; if they are to be considered special, then they could use an official definition; e.g. to be documented as part of the OCI specifications, and possibly the OCI could define them as consts (for convenience).
There was a problem hiding this comment.
I think this is when the consts where introduced moby/moby#35521 (comment), but not sure where they originated from and/or if there's a definition elsewhere;
moby/moby#35521 (comment) also had this part;
sddlString := system.SddlAdministratorsLocalSystem
sddlString += "(A;OICI;GRGWGXRCWDSD;;;" + identity.SID + ")"
``There was a problem hiding this comment.
I would guess that they were never actually documented, but probably should have been added to one of the two official lists of "Well known SIDs". (The lists should all match, but it appears some updates were only done in one or the other). (Also, another official list exists, and more abound outside the MS documentation.)
My suspicion is that domain S-1-5-93 "User Manager" was reserved internally in MS but never made it out into the lists, and is used directly during the base contain image generation process. The two users S-1-5-93-1 and S-1-5-93-2 are similarly possibly "arbitrary" user numbers used during image creation from a Microsoft-reserved pool. (Any final segment less than 1000 is reserved to MS and is generally used for built-in users and groups in regular domains, e.g., Well-known RID 500 is the "Administrator" user account.)
So the most-formal definition we have AFAICT is probably the contents of the Windows base image and some kind of promise that the MS base image build process won't change those values.
> docker run --rm -it --isolation=process mcr.microsoft.com/windows/nanoserver:ltsc2022 reg query "HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList
...
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\S-1-5-18
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\S-1-5-19
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\S-1-5-20
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\S-1-5-93-2-1
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\S-1-5-93-2-2(The first three accounts are the Well-Known SIDs for Local System, Local Service, and Network Service respectively)
> docker run --rm -it --isolation=process --user "ContainerAdministrator" mcr.microsoft.com/powershell whoami /user /groups
USER INFORMATION
----------------
User Name SID
=================================== ============
user manager\containeradministrator S-1-5-93-2-1
GROUP INFORMATION
-----------------
Group Name Type SID Attributes
==================================== ================ ============ ===============================================================
Mandatory Label\High Mandatory Level Label S-1-16-12288
Everyone Well-known group S-1-1-0 Mandatory group, Enabled by default, Enabled group
BUILTIN\Users Alias S-1-5-32-545 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\SERVICE Well-known group S-1-5-6 Mandatory group, Enabled by default, Enabled group
CONSOLE LOGON Well-known group S-1-2-1 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\Authenticated Users Well-known group S-1-5-11 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\This Organization Well-known group S-1-5-15 Mandatory group, Enabled by default, Enabled group
LOCAL Well-known group S-1-2-0 Mandatory group, Enabled by default, Enabled group
BUILTIN\Administrators Alias S-1-5-32-544 Mandatory group, Enabled by default, Enabled group, Group owner
Unknown SID type S-1-5-93-0 Mandatory group, Enabled by default, Enabled group> docker run --rm -it --isolation=process --user "ContainerUser" mcr.microsoft.com/powershell whoami /user /groups
USER INFORMATION
----------------
User Name SID
========================== ============
user manager\containeruser S-1-5-93-2-2
GROUP INFORMATION
-----------------
Group Name Type SID Attributes
====================================== ================ ============ ==================================================
Mandatory Label\Medium Mandatory Level Label S-1-16-8192
Everyone Well-known group S-1-1-0 Mandatory group, Enabled by default, Enabled group
BUILTIN\Users Alias S-1-5-32-545 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\INTERACTIVE Well-known group S-1-5-4 Mandatory group, Enabled by default, Enabled group
CONSOLE LOGON Well-known group S-1-2-1 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\Authenticated Users Well-known group S-1-5-11 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\This Organization Well-known group S-1-5-15 Mandatory group, Enabled by default, Enabled group
LOCAL Well-known group S-1-2-0 Mandatory group, Enabled by default, Enabled group
Unknown SID type S-1-5-93-0 Mandatory group, Enabled by default, Enabled groupThat "Unknown SID type" suggests that even the Windows built-in tools don't have a registration for S-1-5-93-* as well-known SIDs.
Poking around a bit more given the above knowledge, things with prefix S-1-5-93 appear to be an otherwise-undocumented instance of "Virtual Accounts" (class name "User Manager" I guess?) rather than "Well-Known SIDS", so the knowledge of what lives in the S-1-5-93 SID tree is possibly hard-coded into HCS and/or other parts of the OS, rather than being part of the image-creation process as I surmised earlier. Virtual Accounts are significantly less documented than Well-Known SIDs according to that link, but a few other instances are known.
Also worth noting, those SIDs do not exist outside the container, i.e. on the host, per
(New-Object System.Security.Principal.SecurityIdentifier("S-1-5-93-2-1")).Translate([System.Security.Principal.NTAccount])which works in a container but fails on the host.
So there may not exist public documentation for those SIDs, I suspect the docker source is the canonical public reference, and they would probably be hard to change at this point anyway.
(Side-note: Using WMI tools such as WMIC or Get-CimInstance inside the container reports a normal-looking set of user accounts for a blank PC, "Administrator", "DefaultAccount", "Guest" and "WDAGUtilityAccount", which suggests that however ContainerUser and ContainerAdmin are set up, they're not visible (or perhaps simply ignored due to their format) by WMI. The registry profile list suggests they've never been logged in with and are presumably non-functional.)
Anyway, I guess hcsshim would be the right place to document the SIDs and expected behaviour of the S-1-5-93 tree, assuming I'm right and that tree is owned by the Windows Containers team.
Alternatively, another Virtual Account class has a fixed SID documented in the Well-Known SIDs list (S-1-5-83-0) so adding it there would be reasonable, but that might also probably mean adding them to Win32 APIs like CreateWellKnownSid.
moby/moby#35521 (comment) also had this part;
sddlString := system.SddlAdministratorsLocalSystem sddlString += "(A;OICI;GRGWGXRCWDSD;;;" + identity.SID + ")" ``
This part is reasonably explicable, as system.SddlAdministratorsLocalSystem is using published known values ("D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)" which are permission grants for SDDL_BUILTIN_ADMINSTRATORS=>S-1-5-32-544 and SDDL_LOCAL_SYSTEM=>S-1-5-18 respectively.) identity.SID here is actually the SID passed down from the Dockerfile COPY or ADD instruction's context.
There was a problem hiding this comment.
The ContainerAdministrator and ContainerUser are built into HCS. They are not really documented anywhere. We can treat these 2 accounts as always present in all Windows containers with ContainerAdministrator being part of the Administrators group. It always has elevated privileges and can in theory modify files not owned by itself, even when mounted as a volume from the host.
The two constants can live anywhere in terms of code, and moby/sys makes sense TBH. These two users are used throughout the code. We default to ContainerUser when no USER is specified in a Dockerfile and we default to ContainerAdministrator when we need to run with elevated privileges (as is the case in readUser when we try to identify the SID of the USER when we create folders and files via COPY - the alternative was to parse the SAM file which is not at all documented and that is generally frowned upon).
So @TBBle is correct.
There was a problem hiding this comment.
I've created moby/sys#184 to add this constant directly to moby/sys/user instead of including it here. I'll update these usages once that is merged.
There was a problem hiding this comment.
I've added a reference to this discussion in the comments for these constants. I talked with @thaJeztah yesterday and we both thought a local constant would be good for now. It would be nice if these constants were also defined in hcsshim or somewhere in the Microsoft documentation so we had something to reference and we could either transition to using that constant or we can at least reference where these constants come from with some guarantee that they won't change.
This would at least prevent extending the scope of moby/sys until this is a bit more well documented on the Windows side of things since moby/sys would have to maintain the compatibility with the API and this util package is something that can be considered to have no API contract.
1ba5aa4 to
40d62e4
Compare
40d62e4 to
4f9825a
Compare
Convert usages of `github.com/docker/docker/pkg/idtools` to `github.com/moby/sys/user` in order to break the dependency between buildkit and docker. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
4f9825a to
66016a8
Compare
tonistiigi
left a comment
There was a problem hiding this comment.
This has been in review for some time. Can we get this in and create a follow-up issue to track the fate of these windows constants with the comments from this PR.
Convert usages of
github.com/docker/docker/pkg/idtoolstogithub.com/moby/sys/userin order to break the dependency betweenbuildkit and docker.