gh-137335: Fix unlikely name conflicts for named pipes in multiprocessing and asyncio on Windows#137389
gh-137335: Fix unlikely name conflicts for named pipes in multiprocessing and asyncio on Windows#137389serhiy-storchaka wants to merge 5 commits intopython:mainfrom
Conversation
…processing and asyncio on Windows Since os.stat() raises an OSError for existing named pipe "\\.\pipe\...", os.path.exists() always returns False for it, and tempfile.mktemp() can return a name that matches an existing named pipe. So, tempfile.mktemp() cannot be used to generate unique names for named pipes. Instead, CreateNamedPipe() should be called in a loop with different names until it completes successfully.
serhiy-storchaka
added
needs backport to 3.13
serhiy-storchaka
changed the title
serhiy-storchaka
requested review from
1st1,
asvetlov,
gpshead,
kumaraditya303 and
willingc
as code owners
Lib/asyncio/windows_utils.py
Outdated
| address, openmode, _winapi.PIPE_WAIT, | ||
| 1, obsize, ibsize, _winapi.NMPWAIT_WAIT_FOREVER, _winapi.NULL) | ||
| while True: | ||
| address = r'\\.\pipe\python-pipe-' + random.randbytes(8).hex() |
There was a problem hiding this comment.
A comment:
we could use os.urandom(8).hex() but this would only reduce the number of loop iterations we would do until we find a suitable name. OTOH, this makes normal cases much slower (usually, we are not in the presence of an adversary that is trying to create pipes...), so we would be only doing the loop once.
Now, I'm actually worried that it if we're able to interact with the process that is creating the pipes, then we could actually recover enough samples from the underlying PRNG instance and get the original seed. But this is only if 1) there are no random calls in between and 2) we can get 624 consecutive 32-bit samples from the random source.
Condition 2) already holds because random.randbytes(8) is actually equivalent to two calls of random.randbytes(4) that are concatenated. Since reverting MT-19937 requires only consecutive 624 32-bit words, this is the same as doing 312 pipe creations where we don't have calls to random.* in between (and then inspect the named file). This could be possible in practice, especially if we're talking about the multiprocessing and the asyncio components which could have some interactivness (e.g., a server).
Therefore, I would still suggest using os.urandom(8).hex() even if it slows down the calls.
There was a problem hiding this comment.
Good catch, I did not thought about this.
Sorry for not answering immediately, I missed your comment.
|
|
||
| def pipe(*, duplex=False, overlapped=(True, True), bufsize=BUFSIZE): | ||
| """Like os.pipe() but with overlapped support and using handles not fds.""" | ||
| address = tempfile.mktemp( |
There was a problem hiding this comment.
Is getpid + unique counter not enough for avoiding conflict?
There was a problem hiding this comment.
This would be enough for avoiding unintentional conflicts. But you can easily guess the name and sabotage creation.
3 participants
Since os.stat() raises an OSError for existing named pipe
"\\.\pipe\...", os.path.exists() always returns False for it, and tempfile.mktemp() can return a name that matches an existing named pipe.So, tempfile.mktemp() cannot be used to generate unique names for named pipes. Instead, CreateNamedPipe() should be called in a loop with different names until it completes successfully.
tempfile.mktemp()for creating named pipes on Windows #137335