Opened 4 years ago
Closed 4 years ago
#17986 closed enhancement (implemented)
Use SecureZeroMemory in memwipe on Windows
Reported by: | teor | Owned by: | |
---|---|---|---|
Priority: | Medium | Milestone: | Tor: 0.2.8.x-final |
Component: | Core Tor/Tor | Version: | |
Severity: | Minor | Keywords: | windows |
Cc: | Actual Points: | ||
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
In #7419, we use explicit_bzero/memset_s in memwipe when available.
Windows has SecureZeroMemory with the same semantics in Windows.h.
https://msdn.microsoft.com/en-us/library/windows/desktop/aa366877.aspx
As long as we build our Windows binaries with headers that contain SecureZeroMemory, the code will work on any Windows version:
"This function is defined as the RtlSecureZeroMemory function (see WinBase.h). The implementation of RtlSecureZeroMemory is provided inline and can be used on any version of Windows (see WinNT.h.)"
Child Tickets
Change History (10)
comment:1 Changed 4 years ago by
Summary: | Use SecureZeroMemory for Wind → Use SecureZeroMemory in memwipe on Windows |
---|
comment:2 Changed 4 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 4 years ago by
Looks great, I don't have Windows so can't compile it, but it seems simple enough, let's get it tested, then merged.
This is going to require a manual merge with #7419 that preserves all the different cases.
(Edit: explain *why* I didn't compile it)
comment:4 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Merged into master after #7419, with a tiny amount of cleanup.
comment:5 Changed 4 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Jenkins says:
../tor/src/common/crypto.c: In function 'memwipe': 22:31:43 ../tor/src/common/crypto.c:2965:3: error: implicit declaration of function 'SecureZeroMemory' [-Werror=implicit-function-declaration] 22:31:43 ../tor/src/common/crypto.c:2965:3: error: nested extern declaration of 'SecureZeroMemory' [-Werror=nested-externs]
https://jenkins.torproject.org/job/tor-ci-windows/1795/consoleFull
Microsoft says we need to include Windows.h (which includes WinBase.h) to get this function.
https://msdn.microsoft.com/en-us/library/windows/desktop/aa366877.aspx
comment:7 Changed 4 years ago by
Replying to nickm:
Strange! Aren't we already including windows.h ?
MinGW's WinBase.h doesn't contain SecureZeroMemory.
(Their other memory setting functions #define down to memset, so this is probably just as well.)
We'll need to AC_CHECK_FUNC for SecureZeroMemory in configure, like in #7419. (Do we have to include windows.h explicitly, or will configure pick it up automatically?)
comment:8 Changed 4 years ago by
Watch out: AC_CHECK_FUNC doesn't check whether the function is declared at all; it just checks whether it's linkable. We might also need AC_CHECK_FUNC_DECL. Either way we should probably include windows.h.
comment:9 Changed 4 years ago by
The function is defined inline in the header, there's no linker symbol for it. So we need AC_CHECK_FUNC_DECL and *not* AC_CHECK_FUNC.
comment:10 Changed 4 years ago by
Resolution: | → implemented |
---|---|
Status: | reopened → closed |
I've tried to fix it with 1d6dd288e1c084a5118785899cca910e8c69fbb1; let's see if that works
https://github.com/rl1987/tor/compare/securezeromemory