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

Support unprivileged symlink creation in Windows #38921

Merged

Conversation

chris-morgan
Copy link
Member

Symlink creation on Windows has in the past basically required admin; it’s being opened up a bit in the Creators Update, so that at least people who have put their computers into Developer Mode will be able to create symlinks without special privileges. (It’s unclear from what Microsoft has said whether Developer Mode will be required in the final Creators Update release, but sadly I expect it still will be, so this still won’t be as helpful as I’d like.)

Because of compatibility concerns, they’ve hidden this new functionality behind a new flag in the CreateSymbolicLink dwFlags: SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE. So we add this flag in order to join the party.

Sources:

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@retep998
Copy link
Member

retep998 commented Jan 8, 2017

What happens when we pass this flag on older Windows when Rust is elevated? Will it error about an invalid parameter?

@chris-morgan
Copy link
Member Author

chris-morgan commented Jan 8, 2017 via email

@retep998
Copy link
Member

retep998 commented Jan 8, 2017

It would still be a good idea to test the new flag on an older Windows to make sure it doesn't break symlink creation there.

@chris-morgan
Copy link
Member Author

chris-morgan commented Jan 8, 2017 via email

@retep998
Copy link
Member

retep998 commented Jan 8, 2017

I just did a simple test. From an elevated prompt I created a symbolic link with 0 as the flag, worked fine. Then I used 2 as the flag, and it spat ERROR_INVALID_PARAMETER at me. Therefore this PR will need to specifically test for this error code and retry the operation without the new flag.

@chris-morgan chris-morgan force-pushed the windows-unprivileged-symlink-creation branch from 9eff7ba to 3357b08 Compare January 9, 2017 02:43
Symlink creation on Windows has in the past basically required admin;
it’s being opened up a bit in the Creators Update, so that at least
people who have put their computers into Developer Mode will be able to
create symlinks without special privileges. (Microsoft are being
cautious about it all; the Developer Mode requirement makes it so that
it this won’t be as helpful as I’d like, but it’s still an improvement
over requiring admin.)

Because of compatibility concerns, they’ve hidden this new functionality
behind a new flag in the CreateSymbolicLink dwFlags:
SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE. So we add this flag in
order to join the party.

Older Windows doesn’t like this new flag, though, so if we encounter
ERROR_INVALID_PARAMETER we try again without the new flag.

Sources:

- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
  is the official announcement (search for CreateSymbolicLink)

- https://news.ycombinator.com/item?id=13096354 on why the new flag.

- https://twitter.com/richturn_ms/status/818167548269051905 confirming
  that Developer Mode will still be required.
@chris-morgan chris-morgan force-pushed the windows-unprivileged-symlink-creation branch from 3357b08 to 02ae1e1 Compare January 9, 2017 08:07
@chris-morgan
Copy link
Member Author

I’m utterly astonished. It really feels like Microsoft is trying to make things hard for us.

Ah well; fallback implemented.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 10, 2017
@chris-morgan
Copy link
Member Author

r?

@BurntSushi
Copy link
Member

(Frankly I'm dubious that anyone has ever written any software in Rust that
manipulates symlinks on Windows, but ah well.)

I've certainly written my fair share of tests that do it, and this particular problem has been a pain point for me, so anything that makes it easier on us sounds good to me!

cc @rust-lang/libs

@BurntSushi
Copy link
Member

Do we have a way to write a test for this to make sure Windows versions without this flag aren't broken?

@BurntSushi
Copy link
Member

Do we have a way to write a test for this to make sure Windows versions without this flag aren't broken?

The answer is "no."

@retep998
Copy link
Member

retep998 commented Jan 23, 2017

@BurntSushi That would require fairly comprehensive testing across multiple Windows versions both with and without administrator access. We don't have the infrastructure to do that yet.

@chris-morgan
Copy link
Member Author

Any chance of getting this merged before nightly becomes beta in the next couple of days so that it can land in Rust 1.16?

@BurntSushi The problems extend to removing symlinks also: directory and file symlinks must be removed with the correct one of remove_dir or remove_file, unlike Linux (I’m guessing remove_file has to be used there, but I haven’t actually tested that assumption yet)—and whether it is a directory or file symlink isn’t exposed in the public API (it’s hidden inside the os_imp::FileType). All nasty icky issues. I have a crate for cross-platform symlink handling ready to publish which transmutes the FileType to a private copy of os_imp::FileType so that it can get at this detail. I almost didn’t bother finishing the crate because of Windows’ privilege requirements for creating symlinks.

@retep998
Copy link
Member

which transmutes the FileType to a private copy of os_imp::FileType so that it can get at this detail

Noooooooooooooooooooo. Just call the relevant Windows API functions yourself so you can get the information yourself instead of hacking it from libstd in a way that is totally unsupported and could break at any time.

@chris-morgan
Copy link
Member Author

@retep998 Easy to say… this sort of thing sprawls across quite a few private types and several Windows API calls and a couple of values that aren’t even in winapi (REPARSE_DATA_BUFFER and MAXIMUM_REPARSE_DATA_BUFFER_SIZE), so the whole thing ends up increasing my line count by about 167 lines compared to the transmuting approach, even when I’m drastically simplifying things from how they are in libstd/sys/windows/fs.rs et al.

But anyway, that’s not what this PR is about.

@chris-morgan
Copy link
Member Author

Well, we missed the boat for stable in 1.16. But still, could someone please review this? @sfackler? Anyone?

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 4, 2017

📌 Commit 02ae1e1 has been approved by alexcrichton

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 5, 2017
…ymlink-creation, r=alexcrichton

Support unprivileged symlink creation in Windows

Symlink creation on Windows has in the past basically required admin; it’s being opened up a bit in the Creators Update, so that at least people who have put their computers into Developer Mode will be able to create symlinks without special privileges. (It’s unclear from what Microsoft has said whether Developer Mode will be required in the final Creators Update release, but sadly I expect it still will be, so this *still* won’t be as helpful as I’d like.)

Because of compatibility concerns, they’ve hidden this new functionality behind a new flag in the CreateSymbolicLink dwFlags: `SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE`. So we add this flag in order to join the party.

Sources:

- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ is the official announcement (search for CreateSymbolicLink)

- https://news.ycombinator.com/item?id=13096354 on why the new flag.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 5, 2017
…ymlink-creation, r=alexcrichton

Support unprivileged symlink creation in Windows

Symlink creation on Windows has in the past basically required admin; it’s being opened up a bit in the Creators Update, so that at least people who have put their computers into Developer Mode will be able to create symlinks without special privileges. (It’s unclear from what Microsoft has said whether Developer Mode will be required in the final Creators Update release, but sadly I expect it still will be, so this *still* won’t be as helpful as I’d like.)

Because of compatibility concerns, they’ve hidden this new functionality behind a new flag in the CreateSymbolicLink dwFlags: `SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE`. So we add this flag in order to join the party.

Sources:

- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ is the official announcement (search for CreateSymbolicLink)

- https://news.ycombinator.com/item?id=13096354 on why the new flag.
bors added a commit that referenced this pull request Feb 5, 2017
@bors bors merged commit 02ae1e1 into rust-lang:master Feb 5, 2017
@chris-morgan chris-morgan deleted the windows-unprivileged-symlink-creation branch February 7, 2017 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants