Skip to content

Commit ee29182

Browse files
Cory Crooksthatnerdyguy
authored andcommitted
gh-134587: Fix os.mkdir(mode=0o700) in Windows AppContainer
In 8af84b5 (gh-118773), which was a follow up to 81939da (gh-118488), an SDDL was used to grant a directory created via os.mkdir with mode=0o700 owner, admin, and system control. However, when running under a Windows AppContainer, objects must ALSO allow access for the AppContainer SID. This changes the logic to do a one time resolution of the SDDL string to use. If we are running under an AppContainer, then the SDDL includes the SID of the AppContainer, otherwise we use the same standard SDDL as before.
1 parent 22c8590 commit ee29182

File tree

3 files changed

+348
-4
lines changed

3 files changed

+348
-4
lines changed

Lib/test/test_os/test_os.py

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,6 +2209,238 @@ def test_win32_mkdir_700(self):
22092209
'"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"',
22102210
)
22112211

2212+
@unittest.skipUnless(os.name == 'nt', "requires Windows")
2213+
def test_win32_mkdir_700_appcontainer(self):
2214+
# gh-134587: os.mkdir(mode=0o700) must include the AppContainer SID
2215+
# in the protected DACL so that the creating process can still access
2216+
# the directory when running inside a Windows AppContainer.
2217+
import ctypes
2218+
from ctypes import wintypes
2219+
2220+
CONTAINER_NAME = "CPythonTestMkdir700"
2221+
PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES = 0x00020009
2222+
EXTENDED_STARTUPINFO_PRESENT = 0x00080000
2223+
CREATE_NO_WINDOW = 0x08000000
2224+
2225+
class SECURITY_CAPABILITIES(ctypes.Structure):
2226+
_fields_ = [
2227+
("AppContainerSid", ctypes.c_void_p),
2228+
("Capabilities", ctypes.c_void_p),
2229+
("CapabilityCount", wintypes.DWORD),
2230+
("Reserved", wintypes.DWORD),
2231+
]
2232+
2233+
class STARTUPINFOW(ctypes.Structure):
2234+
_fields_ = [
2235+
("cb", wintypes.DWORD),
2236+
("lpReserved", wintypes.LPWSTR),
2237+
("lpDesktop", wintypes.LPWSTR),
2238+
("lpTitle", wintypes.LPWSTR),
2239+
("dwX", wintypes.DWORD),
2240+
("dwY", wintypes.DWORD),
2241+
("dwXSize", wintypes.DWORD),
2242+
("dwYSize", wintypes.DWORD),
2243+
("dwXCountChars", wintypes.DWORD),
2244+
("dwYCountChars", wintypes.DWORD),
2245+
("dwFillAttribute", wintypes.DWORD),
2246+
("dwFlags", wintypes.DWORD),
2247+
("wShowWindow", wintypes.WORD),
2248+
("cbReserved2", wintypes.WORD),
2249+
("lpReserved2", ctypes.c_void_p),
2250+
("hStdInput", wintypes.HANDLE),
2251+
("hStdOutput", wintypes.HANDLE),
2252+
("hStdError", wintypes.HANDLE),
2253+
]
2254+
2255+
class STARTUPINFOEXW(ctypes.Structure):
2256+
_fields_ = [
2257+
("StartupInfo", STARTUPINFOW),
2258+
("lpAttributeList", ctypes.c_void_p),
2259+
]
2260+
2261+
class PROCESS_INFORMATION(ctypes.Structure):
2262+
_fields_ = [
2263+
("hProcess", wintypes.HANDLE),
2264+
("hThread", wintypes.HANDLE),
2265+
("dwProcessId", wintypes.DWORD),
2266+
("dwThreadId", wintypes.DWORD),
2267+
]
2268+
2269+
kernel32 = ctypes.WinDLL('kernel32', use_last_error=True)
2270+
advapi32 = ctypes.WinDLL('advapi32', use_last_error=True)
2271+
try:
2272+
userenv = ctypes.WinDLL('userenv', use_last_error=True)
2273+
except OSError:
2274+
self.skipTest("userenv.dll not available")
2275+
2276+
userenv.CreateAppContainerProfile.restype = ctypes.c_long
2277+
userenv.DeriveAppContainerSidFromAppContainerName.restype = ctypes.c_long
2278+
userenv.DeleteAppContainerProfile.restype = ctypes.c_long
2279+
2280+
# Create (or reuse existing) AppContainer profile
2281+
psid = ctypes.c_void_p()
2282+
hr = userenv.CreateAppContainerProfile(
2283+
CONTAINER_NAME, CONTAINER_NAME, CONTAINER_NAME,
2284+
None, 0, ctypes.byref(psid),
2285+
)
2286+
created_profile = (hr >= 0)
2287+
if not created_profile:
2288+
hr = userenv.DeriveAppContainerSidFromAppContainerName(
2289+
CONTAINER_NAME, ctypes.byref(psid),
2290+
)
2291+
if hr < 0:
2292+
self.skipTest(
2293+
f"Cannot create AppContainer: HRESULT {hr:#010x}"
2294+
)
2295+
2296+
try:
2297+
# Convert SID to string for icacls
2298+
sid_ptr = ctypes.c_wchar_p()
2299+
if not advapi32.ConvertSidToStringSidW(
2300+
psid, ctypes.byref(sid_ptr),
2301+
):
2302+
self.skipTest("Cannot convert AppContainer SID")
2303+
sid_str = sid_ptr.value
2304+
kernel32.LocalFree(sid_ptr)
2305+
2306+
work_dir = tempfile.mkdtemp(prefix='_test_ac_')
2307+
python_dir = os.path.dirname(os.path.abspath(sys.executable))
2308+
stdlib_dir = os.path.dirname(os.__file__)
2309+
2310+
# Directories that need AppContainer read access.
2311+
grant_rx = {python_dir, stdlib_dir}
2312+
granted = []
2313+
2314+
try:
2315+
# Grant AppContainer read+execute to the work directory
2316+
# (for the test script) and the Python installation.
2317+
subprocess.check_call(
2318+
['icacls', work_dir, '/grant',
2319+
f'*{sid_str}:(OI)(CI)RX', '/T', '/Q'],
2320+
stdout=subprocess.DEVNULL,
2321+
stderr=subprocess.DEVNULL,
2322+
)
2323+
granted.append(work_dir)
2324+
for d in grant_rx:
2325+
subprocess.check_call(
2326+
['icacls', d, '/grant',
2327+
f'*{sid_str}:(OI)(CI)RX', '/T', '/Q'],
2328+
stdout=subprocess.DEVNULL,
2329+
stderr=subprocess.DEVNULL,
2330+
)
2331+
granted.append(d)
2332+
2333+
# This script is the actual bug scenario: Using mkdtemp under
2334+
# an AppContainer, and finding it doesn't work
2335+
script = os.path.join(work_dir, '_ac_test.py')
2336+
with open(script, 'w') as f:
2337+
f.write(textwrap.dedent("""\
2338+
import os
2339+
import shutil
2340+
import tempfile
2341+
2342+
# Test what was reported in gh-134587
2343+
target = tempfile.mkdtemp(prefix='_test_ac_inner_')
2344+
try:
2345+
fpath = os.path.join(target, 'test.txt')
2346+
with open(fpath, 'w') as fp:
2347+
fp.write('ok')
2348+
with open(fpath) as fp:
2349+
assert fp.read() == 'ok', 'content mismatch'
2350+
finally:
2351+
shutil.rmtree(target, ignore_errors=True)
2352+
2353+
# Also test the root cause (mkdir with mode=0o700)
2354+
temp_dir = tempfile.gettempdir()
2355+
unique_name = next(tempfile._get_candidate_names())
2356+
other_target = os.path.join(temp_dir, '_test_ac_mkdir_' + unique_name)
2357+
os.mkdir(other_target, mode=0o700)
2358+
try:
2359+
fpath = os.path.join(other_target, 'test.txt')
2360+
with open(fpath, 'w') as fp:
2361+
fp.write('ok')
2362+
with open(fpath) as fp:
2363+
assert fp.read() == 'ok', 'content mismatch'
2364+
finally:
2365+
shutil.rmtree(other_target, ignore_errors=True)
2366+
"""))
2367+
2368+
# Set up proc-thread attribute list for AppContainer
2369+
attr_size = ctypes.c_size_t()
2370+
kernel32.InitializeProcThreadAttributeList(
2371+
None, 1, 0, ctypes.byref(attr_size),
2372+
)
2373+
attr_buf = (ctypes.c_byte * attr_size.value)()
2374+
if not kernel32.InitializeProcThreadAttributeList(
2375+
attr_buf, 1, 0, ctypes.byref(attr_size),
2376+
):
2377+
self.skipTest("InitializeProcThreadAttributeList failed")
2378+
2379+
try:
2380+
sc = SECURITY_CAPABILITIES()
2381+
sc.AppContainerSid = psid
2382+
if not kernel32.UpdateProcThreadAttribute(
2383+
attr_buf, 0,
2384+
PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES,
2385+
ctypes.byref(sc), ctypes.sizeof(sc),
2386+
None, None,
2387+
):
2388+
self.skipTest("UpdateProcThreadAttribute failed")
2389+
2390+
siex = STARTUPINFOEXW()
2391+
siex.StartupInfo.cb = ctypes.sizeof(siex)
2392+
siex.lpAttributeList = ctypes.addressof(attr_buf)
2393+
2394+
pi = PROCESS_INFORMATION()
2395+
cmd = ctypes.create_unicode_buffer(
2396+
f'"{sys.executable}" -I -S "{script}"'
2397+
)
2398+
2399+
if not kernel32.CreateProcessW(
2400+
None, cmd, None, None, False,
2401+
EXTENDED_STARTUPINFO_PRESENT | CREATE_NO_WINDOW,
2402+
None, None,
2403+
ctypes.byref(siex), ctypes.byref(pi),
2404+
):
2405+
err = ctypes.get_last_error()
2406+
self.skipTest(
2407+
f"CreateProcessW failed: error {err}"
2408+
)
2409+
2410+
try:
2411+
kernel32.WaitForSingleObject(
2412+
pi.hProcess, 30_000,
2413+
)
2414+
exit_code = wintypes.DWORD()
2415+
kernel32.GetExitCodeProcess(
2416+
pi.hProcess, ctypes.byref(exit_code),
2417+
)
2418+
self.assertEqual(
2419+
exit_code.value, 0,
2420+
"os.mkdir(mode=0o700) created a directory "
2421+
"that is inaccessible from within its "
2422+
"AppContainer (gh-134587)"
2423+
)
2424+
finally:
2425+
kernel32.CloseHandle(pi.hProcess)
2426+
kernel32.CloseHandle(pi.hThread)
2427+
finally:
2428+
kernel32.DeleteProcThreadAttributeList(attr_buf)
2429+
finally:
2430+
for d in granted:
2431+
subprocess.call(
2432+
['icacls', d, '/remove', f'*{sid_str}',
2433+
'/T', '/Q'],
2434+
stdout=subprocess.DEVNULL,
2435+
stderr=subprocess.DEVNULL,
2436+
)
2437+
shutil.rmtree(work_dir, ignore_errors=True)
2438+
finally:
2439+
if created_profile:
2440+
userenv.DeleteAppContainerProfile(CONTAINER_NAME)
2441+
if psid:
2442+
advapi32.FreeSid(psid)
2443+
22122444
def tearDown(self):
22132445
path = os.path.join(os_helper.TESTFN, 'dir1', 'dir2', 'dir3',
22142446
'dir4', 'dir5', 'dir6')
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix :func:`os.mkdir` for mode `0o700` when running in Windows AppContainer

Modules/posixmodule.c

Lines changed: 115 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6140,6 +6140,109 @@ os__path_normpath_impl(PyObject *module, path_t *path)
61406140
return result;
61416141
}
61426142

6143+
#if defined(MS_WINDOWS) && \
6144+
(defined(MS_WINDOWS_APP) || defined(MS_WINDOWS_SYSTEM))
6145+
6146+
// If running under an AppContainer, this will append an ACE to the provided
6147+
// SDDL string that grants full control to the AppContainer SID.
6148+
static LPCWSTR
6149+
sddl_append_for_appcontainer_if_necessary(LPCWSTR base_sddl) {
6150+
6151+
// Default to using the "base" SDDL, which is what we want if
6152+
// we are not running under an AppContainer
6153+
LPCWSTR resolved_sddl = base_sddl;
6154+
6155+
HANDLE hToken = NULL;
6156+
DWORD isAppContainer = 0;
6157+
DWORD returnLength = 0;
6158+
void *tokenInfo = NULL;
6159+
LPWSTR sidStr = NULL;
6160+
6161+
// Start by opening the process token, so we can query it
6162+
if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &hToken)) {
6163+
goto done;
6164+
}
6165+
6166+
// Determine if the process is running under an AppContainer
6167+
BOOL getTokenResult = GetTokenInformation(hToken, TokenIsAppContainer,
6168+
&isAppContainer, sizeof(isAppContainer),
6169+
&returnLength);
6170+
if (!getTokenResult || !isAppContainer) {
6171+
goto done;
6172+
}
6173+
6174+
// Determine the size of the buffer we need for the AppContainer SID
6175+
returnLength = 0;
6176+
GetTokenInformation(hToken, TokenAppContainerSid, NULL, 0, &returnLength);
6177+
if (!returnLength) {
6178+
goto done;
6179+
}
6180+
6181+
tokenInfo = PyMem_RawMalloc(returnLength);
6182+
if (!tokenInfo) {
6183+
goto done;
6184+
}
6185+
6186+
// Get the AppContainer SID
6187+
getTokenResult = GetTokenInformation(hToken, TokenAppContainerSid,
6188+
tokenInfo, returnLength, &returnLength);
6189+
if (!getTokenResult) {
6190+
goto done;
6191+
}
6192+
6193+
TOKEN_APPCONTAINER_INFORMATION *acInfo =
6194+
(TOKEN_APPCONTAINER_INFORMATION *)tokenInfo;
6195+
if (!acInfo->TokenAppContainer) {
6196+
goto done;
6197+
}
6198+
if (!ConvertSidToStringSidW(acInfo->TokenAppContainer, &sidStr)) {
6199+
goto done;
6200+
}
6201+
6202+
// Now that we know we are running under an AppContainer, and we have
6203+
// the AppContainer SID as a string, we can append an ACE to the provided
6204+
// SDDL
6205+
6206+
// Dynamically allocate the final buffer here. This is expected to be
6207+
// called at most once, however in the case it could be called from
6208+
// multiple threads, we are dynamically allocating the buffer here rather
6209+
// than using a static buffer (which would then require synchronization
6210+
// for that static buffer).
6211+
LPWSTR sddl_buf = PyMem_RawMalloc(sizeof(WCHAR) * 256);
6212+
6213+
int sddl_chars = _snwprintf(
6214+
sddl_buf,
6215+
256,
6216+
// Append a string that includes inheritable (OICI) entries
6217+
// that allow (A) full control (FA) to the AppContainer SID
6218+
L"%s(A;OICI;FA;;;%s)",
6219+
base_sddl,
6220+
sidStr);
6221+
6222+
if (sddl_chars >= 0 && (size_t)sddl_chars < 256) {
6223+
resolved_sddl = sddl_buf;
6224+
}
6225+
else {
6226+
PyMem_RawFree(sddl_buf);
6227+
}
6228+
6229+
done:
6230+
if (sidStr) {
6231+
LocalFree(sidStr);
6232+
}
6233+
if (tokenInfo) {
6234+
PyMem_RawFree(tokenInfo);
6235+
}
6236+
if (hToken) {
6237+
CloseHandle(hToken);
6238+
}
6239+
6240+
return resolved_sddl;
6241+
}
6242+
6243+
6244+
#endif
6245+
61436246
/*[clinic input]
61446247
os.mkdir
61456248

@@ -6173,6 +6276,10 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
61736276
int error = 0;
61746277
SECURITY_ATTRIBUTES secAttr = { sizeof(secAttr) };
61756278
SECURITY_ATTRIBUTES *pSecAttr = NULL;
6279+
#if defined(MS_WINDOWS_APP) || defined(MS_WINDOWS_SYSTEM)
6280+
// Hold the final resolved SDDL as a static, since we only need to resolve it once
6281+
static LPCWSTR resolved_sddl = NULL;
6282+
#endif
61766283
#endif
61776284
#ifdef HAVE_MKDIRAT
61786285
int mkdirat_unavailable = 0;
@@ -6191,11 +6298,15 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
61916298
if (mode == 0700 /* 0o700 */) {
61926299
ULONG sdSize;
61936300
pSecAttr = &secAttr;
6194-
// Set a discretionary ACL (D) that is protected (P) and includes
6195-
// inheritable (OICI) entries that allow (A) full control (FA) to
6196-
// SYSTEM (SY), Administrators (BA), and the owner (OW).
6301+
if (!resolved_sddl) {
6302+
// Set a discretionary ACL (D) that is protected (P) and includes
6303+
// inheritable (OICI) entries that allow (A) full control (FA) to
6304+
// SYSTEM (SY), Administrators (BA), and the owner (OW).
6305+
resolved_sddl = sddl_append_for_appcontainer_if_necessary(
6306+
L"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)");
6307+
}
61976308
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
6198-
L"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)",
6309+
resolved_sddl,
61996310
SDDL_REVISION_1,
62006311
&secAttr.lpSecurityDescriptor,
62016312
&sdSize

0 commit comments

Comments
 (0)