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.

Sign up for GitHub

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

Jump to bottom

Add non-elevated symlink support for dev mode Windows 10 #75693

Closed
vidartf mannequin opened this issue Sep 18, 2017 · 9 comments
Closed

Add non-elevated symlink support for dev mode Windows 10 #75693

vidartf mannequin opened this issue Sep 18, 2017 · 9 comments
Labels
3.8 only security fixes OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@vidartf
Copy link
Mannequin

vidartf mannequin commented Sep 18, 2017

BPO 31512
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba, @vidartf, @izbyshev
PRs
  • bpo-31512: Add non-elevated symlink support for dev mode Windows 10 #3652
    		•
    Files
  • devsymlink.patch: Proof of concept patch
    		•

    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 = <Date 2019-04-09.18:20:25.430>
created_at = <Date 2017-09-18.19:16:30.694>
labels = ['3.8', 'type-feature', 'library', 'OS-windows']
title = 'Add non-elevated symlink support for dev mode Windows 10'
updated_at = <Date 2019-04-09.18:20:25.429>
user = 'https://github.com/vidartf'

    bugs.python.org fields:

    activity = <Date 2019-04-09.18:20:25.429>
actor = 'steve.dower'
assignee = 'none'
closed = True
closed_date = <Date 2019-04-09.18:20:25.430>
closer = 'steve.dower'
components = ['Library (Lib)', 'Windows']
creation = <Date 2017-09-18.19:16:30.694>
creator = 'vidartf'
dependencies = []
files = ['47150']
hgrepos = []
issue_num = 31512
keywords = ['patch']
message_count = 9.0
messages = ['302482', '302494', '302673', '330307', '330349', '330538', '339696', '339800', '339801']
nosy_count = 7.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'vidartf', 'izbyshev']
pr_nums = ['3652']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31512'
versions = ['Python 3.8']
    @vidartf
    Copy link
    Mannequin Author

    vidartf mannequin commented Sep 18, 2017

    As explained in this Microsoft blogpost (https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/), it is possible on newer versions of Windows 10 for non-privileged users to create symlinks when the machine is in developer mode. However, to ensure backwards compatibility, this behavior requires a flag to be passed to the CreateSymbolicLink function.

    I think this is something that Python can benefit from having. It would go from "symlinks on Windows rarely work" to "symlinks on Windows work in developer mode (and rarely otherwise)".

    I've attached a proof of concept patch to enable this behavior (a Windows 10 machine with the 'Creators Update' is required to test it). In summary, it enables the flag by default, and updates enable_symlink to prevent lacking privilege from disabling symlinks on machines in developer mode.

    @vidartf vidartf mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir OS-windows type-feature A feature request or enhancement labels Sep 18, 2017
    @eryksun
    Copy link
    Contributor

    eryksun commented Sep 19, 2017

    Py_CreateSymbolicLinkW can be removed in 3.5+ because Windows XP is no longer supported and calling enable_symlink() is pointless.

    The Windows API uses some privileges simply to determine which security principals can access a capability. Whether the privilege is currently enabled or disabled in the current access token doesn't matter because functions automatically enable it for the current thread (in an impersonation token).

    In this case, CreateSymbolicLink calls RtlAcquirePrivilege to enable SeCreateSymbolicLinkPrivilege for the current thread; sets the symlink reparse point; and then reverts the current thread via RtlReleasePrivilege. It goes through these same steps whether or not the privilege is already enabled in the process, so there's no chance of a race condition between competing threads.

    Also, as a side note, the linked blog makes the following claim, which paints an incomplete picture:

    However, for Windows users, due to Windows Vista’s security
    requirements, users needed local admin rights and, importantly,
    had to run mklink in a command-line console elevated as
    administrator to create/modify symlinks.

    SeCreateSymbolicLinkPrivilege can be added to a standard user account, and it doesn't get filtered out from the user's token. So in general you do not need administrator access. However, the above is describing the case for most developers, who use a administrator account that's subject to UAC restrictions.

    @vidartf
    Copy link
    Mannequin Author

    vidartf mannequin commented Sep 21, 2017

    Thanks for the informative comments. I opened a PR based on this feedback. Would you mind checking if it conforms to what you had in mind?

    @terryjreedy terryjreedy added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Sep 4, 2018
    @vidartf
    Copy link
    Mannequin Author

    vidartf mannequin commented Nov 23, 2018

    The PR has been sitting for a while now with all previous concerns addressed. There has been a few pings on the PR without anything new happening, so I thought I would ping this issue as well: are there any other concerns about this PR, or anything else that is preventing it from being merged?

    @zooba
    Copy link
    Member

    zooba commented Nov 23, 2018

    Thanks for the ping (I don't see GitHub notifications - I get 1000s per day and it's not feasible to read them).

    I left one more comment on the PR, but then it's good to go!

    @vidartf
    Copy link
    Mannequin Author

    vidartf mannequin commented Nov 27, 2018

    Thanks! I addressed the comment, so hopefully this should be OK now.

    @eryksun
    Copy link
    Contributor

    eryksun commented Apr 9, 2019

    :ping: It seems this was ready to be merged a while ago.

    @zooba
    Copy link
    Member

    zooba commented Apr 9, 2019

    New changeset 0e10766 by Steve Dower (Vidar Tonaas Fauske) in branch 'master':
    bpo-31512: Add non-elevated symlink support for Windows (GH-3652)
    0e10766

    @zooba
    Copy link
    Member

    zooba commented Apr 9, 2019

    Done

    @zooba zooba closed this as completed Apr 9, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.8 only security fixes OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @eryksun @zooba @terryjreedy