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

Always pass SECURITY_SQOS_PRESENT|SECURITY_IDENTIFICATION when opening a named pipe #42036

Closed
DemiMarie opened this issue May 16, 2017 · 6 comments · Fixed by #58216
Closed

Always pass SECURITY_SQOS_PRESENT|SECURITY_IDENTIFICATION when opening a named pipe #42036

DemiMarie opened this issue May 16, 2017 · 6 comments · Fixed by #58216
Labels
A-security Area: Security related issues (example: address space layout randomization) C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows

Comments

@DemiMarie
Copy link
Contributor

By default, opening a named pipe on Windows allows the server to impersonate the client. This is a security vulnerability. Pass SECURITY_SQOS_PRESENT|SECURITY_IDENTIFICATION to prevent this.

Since it fixes a security hole, I don’t think this needs an RFC.
@sfackler
Copy link
Member

Where do we use named pipes?

@DemiMarie
Copy link
Contributor Author

DemiMarie commented May 16, 2017 via email

@Mark-Simulacrum
Copy link
Member

@DemiMarie Could you be more clear? What exactly needs to change inside Rust? I assume we'd have to add some form of detection around file opening in windows..?

@Mark-Simulacrum Mark-Simulacrum added A-security Area: Security related issues (example: address space layout randomization) O-windows Operating system: Windows labels Jun 22, 2017
@DemiMarie
Copy link
Contributor Author

DemiMarie commented Jun 23, 2017 via email

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 27, 2017
@mattico mattico mentioned this issue Sep 14, 2017
Closed
mattico added a commit to mattico/rust that referenced this issue Sep 14, 2017
@mattico
windows: Enable default security parameters on file creation to avoid…
d55c2d7 
… named pipe exploit

Fixes rust-lang#42036

As noted in [this paper][1], the threat model for the exploit is a priveleged Rust process which accepts a file path from a malicious program. With this exploit, the malicious program can pass a named pipe to the priveleged process and gain its elevated priveleges.

The fix is to change the default OpenOptions to contain the proper security flags. [The .NET FileStream][2] has this same behavior by default. We're using the `SecurityIdentification` security level which is more permissive, but still blocks the exploit.

This is technically a breaking change. If someone were using a named pipe to impersonate a program *on purpose*, they would have to add `.security_qos_flags(0)` to their `OpenOptions` to keep working.

[1]: http://www.blakewatts.com/namedpipepaper.html
[2]: http://referencesource.microsoft.com/#mscorlib/system/io/filestream.cs,837
@pitdicker
Copy link
Contributor

@mattico can you tell anything about what potential issue you hit when trying to work on this in #44556? Or do you have WIP code of the test you where working on?

@pitdicker
Copy link
Contributor

A simple explanation of the security problem: https://flylib.com/books/en/1.287.1.242/1/.

What could be a problem with setting these flags always: the flag SECURITY_SQOS_PRESENT is the same as FILE_FLAG_OPEN_NO_RECALL. The first is supposed to be used with named pipes, the other for files and directories. So if we start to unconditionally add SECURITY_SQOS_PRESENT|SECURITY_IDENTIFICATION, it will in reality set the FILE_FLAG_OPEN_NO_RECALL in almost all cases.

Now I've never seen a system with 'hierarchical storage' where FILE_FLAG_OPEN_NO_RECALL would be necessary. It is intended for systems that combine fast store like hard drives, with slow bulk storage like tapes. File operations like indexing, searching, or backup that want to work well on such systems should open files that have the attribute FILE_ATTRIBUTE_OFFLINE with this flag. Otherwise a lot of data from tape may be put back online on hard drives. See two descriptions at The Old New Thing.

While investigation this, I found another issue with security_qos_flags: one of the values it takes is SECURITY_ANONYMOUS, whis is simply 0x0. Currently, when you set SECURITY_ANONYMOUS, it will silently be ignored unless you also happen to set SECURITY_SQOS_PRESENT (which our API is supposed to set automatically). Easy to fix though.

Now I don't care much for working well with hierarchical storage. But we should not break it without having some sort of workaround available.

  • We can say that if user code contains custom_flags it should be able to handle Windows-specific oddities such as this one. We could make it so that even custom_flags(0) is enough to disable automatic sqos flags. But it doesn't feel great to just drop a security measure when the user could be doing something unrelated, such as the buffering hint custom_flags(FILE_FLAG_RANDOM_ACCESS).
  • Only set SECURITY_SQOS_PRESENT|SECURITY_IDENTIFICATION when we know what we are trying to open is a named pipe. I think all named pipes start with \\.\pipe\, and we know the filename when setting the flags. But I am not sure if this really works, or if it is possible for an unprivileged process to create a symbolic link to one in the object manager.

I don't know if I'll get to making a PR and doing the required testing, just writing down what I've found so far.

@pitdicker pitdicker mentioned this issue Feb 6, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Feb 23, 2019
@Centril
Rollup merge of rust-lang#58216 - pitdicker:sqos_flags, r=alexcrichton
8c66221 
 Set secure flags when opening a named pipe on Windows

Fixes rust-lang#42036, see also the previous attempt in rust-lang#44556.

Whether this is correct depends on if it is somehow possible to create a symlink to a named pipe, outside the named pipe filesystem (NPFS). But as far as I can tell that should be impossible.

Also fixes that `security_qos_flags(SECURITY_ANONYMOUS)` does not set the `SECURITY_SQOS_PRESENT` flag, and the incorrect documentation about the default value of `security_qos_flags`.
Centril added a commit to Centril/rust that referenced this issue Feb 23, 2019
@Centril
Rollup merge of rust-lang#58216 - pitdicker:sqos_flags, r=alexcrichton
e018763 
 Set secure flags when opening a named pipe on Windows

Fixes rust-lang#42036, see also the previous attempt in rust-lang#44556.

Whether this is correct depends on if it is somehow possible to create a symlink to a named pipe, outside the named pipe filesystem (NPFS). But as far as I can tell that should be impossible.

Also fixes that `security_qos_flags(SECURITY_ANONYMOUS)` does not set the `SECURITY_SQOS_PRESENT` flag, and the incorrect documentation about the default value of `security_qos_flags`.
Centril added a commit to Centril/rust that referenced this issue Feb 23, 2019
@Centril
Rollup merge of rust-lang#58216 - pitdicker:sqos_flags, r=alexcrichton
249e103 
 Set secure flags when opening a named pipe on Windows

Fixes rust-lang#42036, see also the previous attempt in rust-lang#44556.

Whether this is correct depends on if it is somehow possible to create a symlink to a named pipe, outside the named pipe filesystem (NPFS). But as far as I can tell that should be impossible.

Also fixes that `security_qos_flags(SECURITY_ANONYMOUS)` does not set the `SECURITY_SQOS_PRESENT` flag, and the incorrect documentation about the default value of `security_qos_flags`.
bors added a commit that referenced this issue Mar 2, 2019
@bors
Auto merge of #58216 - pitdicker:sqos_flags, r=alexcrichton
fab272e 
 Set secure flags when opening a named pipe on Windows

Fixes #42036, see also the previous attempt in #44556.

Whether this is correct depends on if it is somehow possible to create a symlink to a named pipe, outside the named pipe filesystem (NPFS). But as far as I can tell that should be impossible.

Also fixes that `security_qos_flags(SECURITY_ANONYMOUS)` does not set the `SECURITY_SQOS_PRESENT` flag, and the incorrect documentation about the default value of `security_qos_flags`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-security Area: Security related issues (example: address space layout randomization) C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows
Projects
None yet
Milestone
No milestone
4 participants
@sfackler @Mark-Simulacrum @pitdicker @DemiMarie