Opened 5 years ago

Closed 5 years ago

#13169 closed defect (fixed)

Make SSP use Windows-specifc RNG

Reported by: mikeperry Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-security, GeorgKoppen201502R, TorBrowserTeam201502R
Cc: arma, gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

According to skruffy by way of arma, SSP may not be applied on our Windows builds:
https://trac.torproject.org/projects/tor/ticket/10065#comment:31

I am not sure why not, or how skruffy checked. It looks like we are using -fstack-protector in our wrapper scripts.

Did skruffy use a tool like:
https://www.microsoft.com/en-us/download/details.aspx?id=11910

Is it possible that those tools are simply not recognizing the MinGW version of SSP?

Child Tickets

Change History (18)

comment:1 Changed 5 years ago by gk

Cc: gk added

comment:2 Changed 5 years ago by erinn

This bug is actually about this patch not getting applied:

https://gitweb.torproject.org/user/erinn/tor-browser-bundle.git/blob/17425dec3a13de51b717efeb8bdde1a4460d31fa:/gitian/patches/windows-crypto.patch

skruffy's reasoning is that since it's possible for a local user to replace the canaries, SSP is not actually enabled.

comment:3 Changed 5 years ago by mikeperry

Summary: SSP may not applied to Windows TBB?Make SSP use Windows-specifc RNG

I am not clear from the context in this patch where the canaries normally come form on Windows. It looks like it is trying to use the Windows crypto provider instead of /dev/urandom? Is that correct? Or was it failing to access /dev/urandom because it didn't exist, and then using something else?

What is this patch applying to? gcc or binutils?

comment:4 in reply to:  3 Changed 5 years ago by cypherpunks

It looks like it is trying to use the Windows crypto provider instead of /dev/urandom? Is that correct?

Yes.

What is this patch applying to? gcc or binutils?

GCC.

In Windows you can to create any directories for any disks(C:, D:, .. Z:), only system directories (Windows directory, Program files, etc) are protected. Any process with privileges of any standard user can to create C:\dev\urandom file and to fill it by any stuff.

comment:5 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201410 MikePerry201410R added

Ok, after the Firefox 31 madness calms down a bit, we should be able to apply this.

Has it been submitted to the mingw people?

comment:6 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201411 added; TorBrowserTeam201410 removed

comment:7 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201412R added

comment:8 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201412 added; TorBrowserTeam201411 removed

comment:9 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201501R added; TorBrowserTeam201412R removed

comment:10 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201501 added; TorBrowserTeam201412 removed

comment:11 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201502R added; TorBrowserTeam201501R removed

comment:12 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201502 added; TorBrowserTeam201501 removed

comment:13 Changed 5 years ago by mikeperry

Keywords: GeorgKoppen201502R added; MikePerry201410R TorBrowserTeam201502 removed

gk - You're on the mingw lists, right? Can you get this patch to them and see what they think?

comment:14 Changed 5 years ago by mikeperry

Status: newneeds_review

gk - You're on the mingw lists, right? Can you get this patch to them and see what they think?

comment:15 in reply to:  14 Changed 5 years ago by gk

Replying to mikeperry:

gk - You're on the mingw lists, right? Can you get this patch to them and see what they think?

Now, yes. http://sourceforge.net/p/mingw-w64/mailman/message/33336127/ contains the mail to the list.

comment:16 Changed 5 years ago by gk

Jacek looked at it (http://sourceforge.net/p/mingw-w64/mailman/message/33344781/) and made some suggestions which seemed reasonable and indicated that he thinks the patch is doing what it promises but could be enhanced. On the other hand he claimed to have not much knowledge about GCC/libssp and pointed to the gcc-patches mailing list (which I try next).

comment:18 Changed 5 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

The patch got merged into GCC trunk as 19fef1633156a2c7ddd267b43d08f1b245a6e1f4. Commit d4950e565f93396ebbd310c71e49576af9224d25 contains the proper backport for our use case.

Note: See TracTickets for help on using tickets.