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

[Windows] add _PyCreateFile with follow_symlinks support #90664

Open
eryksun opened this issue Jan 24, 2022 · 3 comments
Open

[Windows] add _PyCreateFile with follow_symlinks support #90664

eryksun opened this issue Jan 24, 2022 · 3 comments
Labels
extension-modules C modules in the Modules dir OS-windows type-feature A feature request or enhancement

Comments

@eryksun
Copy link
Contributor

eryksun commented Jan 24, 2022

BPO 46506
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-01-24.21:24:30.020>
labels = ['extension-modules', 'type-feature', 'OS-windows']
title = '[Windows] wrap CreateFile to support follow_symlinks'
updated_at = <Date 2022-01-26.23:15:08.295>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2022-01-26.23:15:08.295>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'Windows']
creation = <Date 2022-01-24.21:24:30.020>
creator = 'eryksun'
dependencies = []
files = []
hgrepos = []
issue_num = 46506
keywords = []
message_count = 3.0
messages = ['411516', '411798', '411804']
nosy_count = 5.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46506'
versions = []

@eryksun
Copy link
Contributor Author

eryksun commented Jan 24, 2022

bpo-46490 proposes to support follow_symlinks in os.utime() on Windows. Instead of duplicating the os.stat() implementation of follow_symlinks, I suggest factoring out a common _Py_CreateFile() function with two additional parameters: traverse (input) and pFileInfo (output).

The traverse parameter would indicate whether to traverse or open a name-surrogate reparse point. However, FILE_FLAG_OPEN_REPARSE_POINT should take precedence over traverse and open any type of reparse point.

The pFileInfo parameter would be an optional pointer to a struct that receives the file type (disk, char, pipe), file attributes, and reparse tag. Querying this information is required when traverse is false. There may as well be a way to return it.

Since Windows 7 hasn't been supported since 3.8, this is also an opportunity to switch to the newer CreateFile2() function [1], which is required in order to use new file flags added to the API such as FILE_FLAG_OPEN_REQUIRING_OPLOCK.

---
[1] https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfile2

@eryksun eryksun added extension-modules C modules in the Modules dir OS-windows type-feature A feature request or enhancement labels Jan 24, 2022
@eryksun
Copy link
Contributor Author

eryksun commented Jan 26, 2022

switch to the newer CreateFile2() function

Apparently we need to allow FILE_SHARE_READ if CreateFile2() is used to implement os.stat(). It's not a big deal, but this is a design flaw.

CreateFile2() always uses the kernel I/O flag FILE_DISALLOW_EXCLUSIVE [1] when it calls NtCreateFile(). The intent of this flag is to avoid exclusive read access on the first open of a file when a user lacks write permission. Thus unprivileged users aren't allowed to prevent read access to critical system files.

However, the implementation is flawed because it denies access even if the open doesn't request data access. It's also flawed because it allows exclusive read access when there are previous opens even if the previous opens have no data access.

Classically, IoCheckShareAccess() only checks for a sharing violation when an open requests read, write, or delete data access (i.e. FILE_READ_DATA, FILE_EXECUTE, FILE_WRITE_DATA, FILE_APPEND_DATA, DELETE). This is clearly explained in [MS-FSA] [2]. An open that only requests metadata access can never cause a sharing violation and poses no problem with regard to blocking access to a file.

---
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/nf-ntddk-iocreatefileex
[2] https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/8c0e3f4f-0729-49f4-a14d-7f7add593819

@eryksun
Copy link
Contributor Author

eryksun commented Jan 26, 2022

Here's an implementation of _Py_CreateFile2() and win32_xstat_impl():

typedef struct {
    DWORD type;
    DWORD attributes;
    DWORD reparseTag;
} _PY_CREATE_FILE_INFO;
static HANDLE
_Py_CreateFile2(LPCWSTR lpFileName,
                DWORD dwDesiredAccess,
                DWORD dwShareMode,
                DWORD dwCreationDisposition,
                LPCREATEFILE2_EXTENDED_PARAMETERS pCreateExParams,
                BOOL traverse,
                _PY_CREATE_FILE_INFO *pCreateInfo)
{
    HANDLE hFile;
    DWORD error;
    FILE_BASIC_INFO fbi;
    FILE_ATTRIBUTE_TAG_INFO fati;
    BOOL openReparsePoint = FALSE;
    BOOL traverseFailed = FALSE;
    _PY_CREATE_FILE_INFO cfi = {0};

    CREATEFILE2_EXTENDED_PARAMETERS createExParams = {
            sizeof(CREATEFILE2_EXTENDED_PARAMETERS)};
    if (!pCreateExParams) {
        pCreateExParams = &createExParams;
    }

    pCreateExParams->dwFileFlags |= FILE_FLAG_BACKUP_SEMANTICS;

    if (pCreateExParams->dwFileFlags & FILE_FLAG_OPEN_REPARSE_POINT) {
        openReparsePoint = TRUE;
    } else if (!traverse) {
        pCreateExParams->dwFileFlags |= FILE_FLAG_OPEN_REPARSE_POINT;
    }

    // Share read access if write access isn't requested because
    // CreateFile2 uses the NT I/O flag FILE_DISALLOW_EXCLUSIVE.
    if (!(dwDesiredAccess & (GENERIC_ALL | GENERIC_WRITE |
            FILE_WRITE_DATA | FILE_APPEND_DATA))) {
        dwShareMode |= FILE_SHARE_READ;
    }
call_createfile:
    hFile = CreateFile2(lpFileName, dwDesiredAccess, dwShareMode,
                dwCreationDisposition, pCreateExParams);
    error = GetLastError(); // success: 0 or ERROR_ALREADY_EXISTS
    if (hFile == INVALID_HANDLE_VALUE) {
        // bpo-37834: open an unhandled reparse point if traverse fails.
        traverseFailed = (error == ERROR_CANT_ACCESS_FILE);
        if (openReparsePoint || !(traverse && traverseFailed)) {
            return INVALID_HANDLE_VALUE;
        }
        pCreateExParams->dwFileFlags |= FILE_FLAG_OPEN_REPARSE_POINT;
        hFile = CreateFile2(lpFileName, dwDesiredAccess, dwShareMode,
                    dwCreationDisposition, pCreateExParams);
        if (hFile == INVALID_HANDLE_VALUE) {
            SetLastError(error);
            return INVALID_HANDLE_VALUE;
        }
        error = GetLastError(); // 0 or ERROR_ALREADY_EXISTS
    }

    if (!pCreateInfo && (openReparsePoint || (traverse && !traverseFailed)))
    {
        return hFile;
    }

    cfi.type = GetFileType(hFile);
    if (cfi.type == FILE_TYPE_UNKNOWN && GetLastError() != 0) {
        error = GetLastError();
        goto cleanup;
    }

    if (GetFileInformationByHandleEx(
            hFile, FileAttributeTagInfo, &fati, sizeof(fati))) {
        cfi.attributes = fati.FileAttributes;
        cfi.reparseTag = fati.ReparseTag;
    } else if (GetFileInformationByHandleEx(
                    hFile, FileBasicInfo, &fbi, sizeof(fbi))) {
        cfi.attributes = fbi.FileAttributes;
    } else {
        switch (GetLastError()) {
            case ERROR_INVALID_PARAMETER:
            case ERROR_INVALID_FUNCTION:
            case ERROR_NOT_SUPPORTED:
                // The file is not in a filesystem.
                break;
            default:
                error = GetLastError();
        }
        goto cleanup;
    }

    if (cfi.attributes & FILE_ATTRIBUTE_REPARSE_POINT) {
        if (IsReparseTagNameSurrogate(cfi.reparseTag)) {
            if (traverseFailed) {
                error = ERROR_CANT_ACCESS_FILE;
                goto cleanup;
            }
        } else if (!openReparsePoint && !traverseFailed) {
            // Always try to reparse if it's not a name surrogate.
            CloseHandle(hFile);
            traverse = TRUE;
            pCreateExParams->dwFileFlags &= ~FILE_FLAG_OPEN_REPARSE_POINT;
            goto call_createfile;
        }
    }
cleanup:
    if (error && error != ERROR_ALREADY_EXISTS) {
        CloseHandle(hFile);
        hFile = INVALID_HANDLE_VALUE;
    } else if (pCreateInfo) {
        *pCreateInfo = cfi;
    }
    SetLastError(error);
    return hFile;
}
static int
win32_xstat_impl(const wchar_t *path, struct _Py_stat_struct *result,
                 BOOL traverse)
{
    DWORD error;
    BY_HANDLE_FILE_INFORMATION fileInfo;
    _PY_CREATE_FILE_INFO cfi;
    int retval = 0;

    HANDLE hFile = _Py_CreateFile2(path, FILE_READ_ATTRIBUTES, 0,
                        OPEN_EXISTING, NULL, traverse, &cfi);

    if (hFile == INVALID_HANDLE_VALUE) {
        // Either the path doesn't exist, or the caller lacks access.
        error = GetLastError();
        switch (error) {
        case ERROR_INVALID_PARAMETER:
            // The "con" DOS device requires read or write access.
            hFile = _Py_CreateFile2(path, GENERIC_READ, FILE_SHARE_READ |
                        FILE_SHARE_WRITE | FILE_SHARE_DELETE, OPEN_EXISTING,
                        NULL, traverse, &cfi);
            if (hFile == INVALID_HANDLE_VALUE) {
                SetLastError(error);
                return -1;
            }
            break;
        case ERROR_ACCESS_DENIED:     // Cannot sync or read attributes.
        case ERROR_SHARING_VIOLATION: // It's a paging file.
            // Try reading the parent directory.
            if (!attributes_from_dir(path, &fileInfo, &cfi.reparseTag)) {
                // Cannot read the parent directory.
                SetLastError(error);
                return -1;
            }
            if (fileInfo.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) {
                if (traverse || !IsReparseTagNameSurrogate(cfi.reparseTag)) {
                    // Can't traverse, so fail.
                    SetLastError(error);
                    return -1;
                }
            }
            break;
        default:
            return -1;
        }
    }

    if (hFile != INVALID_HANDLE_VALUE) {
        if (cfi.type != FILE_TYPE_DISK) {
            memset(result, 0, sizeof(*result));
            if (cfi.attributes != INVALID_FILE_ATTRIBUTES &&
                cfi.attributes & FILE_ATTRIBUTE_DIRECTORY) {
                // e.g. "//./pipe/" or "//./mailslot/"
                result->st_mode = _S_IFDIR;
            } else if (cfi.type == FILE_TYPE_CHAR) {
                // e.g. "//./nul"
                result->st_mode = _S_IFCHR;
            } else if (cfi.type == FILE_TYPE_PIPE) {
                // e.g. "//./pipe/spam"
                result->st_mode = _S_IFIFO;
            }
            // FILE_TYPE_UNKNOWN
            // e.g. "//./mailslot/waitfor.exe/spam"
            goto cleanup;
        }

        if (!GetFileInformationByHandle(hFile, &fileInfo)) {
            switch (GetLastError()) {
            case ERROR_INVALID_PARAMETER:
            case ERROR_INVALID_FUNCTION:
            case ERROR_NOT_SUPPORTED:
                // Volumes and physical disks are block devices,
                // e.g. "//./C:" or "//./PhysicalDrive0"
                memset(result, 0, sizeof(*result));
                result->st_mode = 0x6000; // S_IFBLK
                goto cleanup;
            }
            retval = -1;
            goto cleanup;
        }
    }

    _Py_attribute_data_to_stat(&fileInfo, cfi.reparseTag, result);

    if (!(fileInfo.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) {
        // Set S_IEXEC if the filename has an extension that is
        // commonly used by files that CreateProcessW can execute.
        // A real implementation would call GetSecurityInfo,
        // OpenThreadToken / OpenProcessToken, and AccessCheck to
        // check for read, write, and execute access.
        const wchar_t *fileExtension = wcsrchr(path, '.');
        if (fileExtension) {
            if (_wcsicmp(fileExtension, L".com") == 0 ||
                _wcsicmp(fileExtension, L".exe") == 0 ||
                _wcsicmp(fileExtension, L".bat") == 0 ||
                _wcsicmp(fileExtension, L".cmd") == 0)
            {
                result->st_mode |= 0111;
            }
        }
    }
cleanup:
    if (hFile != INVALID_HANDLE_VALUE) {
        // Preserve the last error when failing.
        error = retval ? GetLastError() : 0;
        if (CloseHandle(hFile)) {
            SetLastError(error);
        } else {
            retval = -1;
        }
    }

    return retval;
}

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@eryksun eryksun changed the title [Windows] wrap CreateFile to support follow_symlinks [Windows] add _PyCreateFile with follow_symlinks support Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir OS-windows type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant