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 library loading #3493

Merged
merged 1 commit into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 40 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,44 @@ fn main() {
}
let target = env::var("TARGET").unwrap();
println!("cargo:rustc-env=TARGET={target}");

// Set linker options specific to Windows MSVC.
let target_os = env::var("CARGO_CFG_TARGET_OS");
let target_env = env::var("CARGO_CFG_TARGET_ENV");
if !(target_os.as_deref() == Ok("windows") && target_env.as_deref() == Ok("msvc")) {
return;
}

// # Only search system32 for DLLs
//
// This applies to DLLs loaded at load time. However, this setting is ignored
// before Windows 10 RS1 (aka 1601).
// https://learn.microsoft.com/en-us/cpp/build/reference/dependentloadflag?view=msvc-170
println!("cargo:cargo:rustc-link-arg-bin=rustup-init=/DEPENDENTLOADFLAG:0x800");

// # Delay load
//
// Delay load dlls that are not "known DLLs"[1].
// Known DLLs are always loaded from the system directory whereas other DLLs
// are loaded from the application directory. By delay loading the latter
// we can ensure they are instead loaded from the system directory.
// [1]: https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order#factors-that-affect-searching
//
// This will work on all supported Windows versions but it relies on
// us using `SetDefaultDllDirectories` before any libraries are loaded.
// See also: src/bin/rustup-init.rs
let delay_load_dlls = ["bcrypt", "powrprof", "secur32"];
for dll in delay_load_dlls {
println!("cargo:rustc-link-arg-bin=rustup-init=/delayload:{dll}.dll");
}
// When using delayload, it's necessary to also link delayimp.lib
// https://learn.microsoft.com/en-us/cpp/build/reference/dependentloadflag?view=msvc-170
println!("cargo:rustc-link-arg-bin=rustup-init=delayimp.lib");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT delayimp.lib is not mentioned on the linked page. Do you have a reference for this/why this is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, see the documentation for the delayload linker option. I've added a comment that links to it.


// # Turn linker warnings into errors
//
// Rust hides linker warnings meaning mistakes may go unnoticed.
// Turning them into errors forces them to be displayed (and the build to fail).
// If we do want to ignore specific warnings then `/IGNORE:` should be used.
println!("cargo:cargo:rustc-link-arg-bin=rustup-init=/WX");
}
21 changes: 21 additions & 0 deletions src/bin/rustup-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ use rustup::is_proxyable_tools;
use rustup::utils::utils;

fn main() {
#[cfg(windows)]
pre_rustup_main_init();

let process = OSProcess::default();
with(process.into(), || match maybe_trace_rustup() {
Err(e) => {
Expand Down Expand Up @@ -163,3 +166,21 @@ fn do_recursion_guard() -> Result<()> {

Ok(())
}

/// Windows pre-main security mitigations.
///
/// This attempts to defend against malicious DLLs that may sit alongside
/// rustup-init in the user's download folder.
#[cfg(windows)]
pub fn pre_rustup_main_init() {
use winapi::um::libloaderapi::{SetDefaultDllDirectories, LOAD_LIBRARY_SEARCH_SYSTEM32};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe orthogonal, but should we start using windows-sys here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised there's not an issue for that already but I do think that's beyond the scope of this PR.

// Default to loading delay loaded DLLs from the system directory.
// For DLLs loaded at load time, this relies on the `delayload` linker flag.
// This is only necessary prior to Windows 10 RS1. See build.rs for details.
unsafe {
let result = SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for this function seem to say that it's not directly available on Win7:

Windows 7, Windows Server 2008 R2, Windows Vista and Windows Server 2008: To call this function in an application, use the GetProcAddress function to retrieve its address from Kernel32.dll. KB2533623 must be installed on the target platform.

So... how did this call work? Are we not setting up the build in a Win7 mode? (until we actually drop that support)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this works in my Win7 VM. Presumably the advice to use GetProcAddress is because the old SDK libraries didn't include them and they require an OS update to work. You get an error message if the OS update is not installed but it's quite easy to find answers on the web pointing to the necessary update. I think it's ok to require users to have installed security updates, especially for such an old OS.

But yes, Windows 7 hasn't been tested in CI in a long time. The same goes for all rustlang repos I'm aware of.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the Rust 1.72 blog post had this text:

In a future release we're planning to increase the minimum supported Windows version to 10. The accepted proposal in compiler MCP 651 is that Rust 1.75 will be the last to officially support Windows 7, 8, and 8.1. When Rust 1.76 is released in February 2024, only Windows 10 and later will be supported as tier-1 targets. This change will apply both as a host compiler and as a compilation target.

How do you think those changes (5 months from now) should interact with this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't change anything until rust's minimum supported version is Windows 10 RS1 (aka 1607). I can add a similar comment to the one in the build script and note how they're related.

// SetDefaultDllDirectories should never fail if given valid arguments.
// But just to be safe and to catch mistakes, assert that it succeeded.
assert_ne!(result, 0);
}
}