Opened 5 months ago

Closed 4 months ago

Last modified 4 months ago

#30451 closed defect (fixed)

snowflake-client has executable stack

Reported by: boklm Owned by: cohosh
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords: TorBrowserTeam201905R, tbb-rbm
Cc: tbb-team, dcf, arlolra, cohosh, phw Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Running this command shows that the snowflake-client binary has an executable stack:

$ readelf -W -l TorBrowser/Tor/PluggableTransports/snowflake-client | grep GNU_STACK
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RWE 0x10

(RWE should be RW when the stack is not executable)

Child Tickets

Change History (15)

comment:1 Changed 4 months ago by cohosh

Owner: set to cohosh
Status: newassigned

Hmm, looking at the go linker it seems like PT_GNU_STACK should be set: https://golang.org/src/cmd/link/internal/ld/elf.go#L240

The proxy-go instances have the same problem, but the broker does not:

$ readelf -W -l broker
Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0x8

I wonder if this has something to do with CGO

comment:2 Changed 4 months ago by cohosh

Yep, it's a cgo thing.

The solution here is to add the noexecstack cgo LD flag to go-webrtc:
#cgo LDFLAGS: -L${SRCDIR}/lib -z noexecstack

However, this will currently throw an error because of golang's whitelist on linker and compiler options. This can be solved by setting the environment variable CGO_LDFLAGS_ALLOW to a regex that recognizes the -z noexecstack option.

I'll work on a patch for this.

comment:3 Changed 4 months ago by cohosh

I created a pull request for go-webrtc: https://github.com/keroserene/go-webrtc/pull/105

I've also attached a patch to this ticket, we'll have to wait until the above pull request is accepted before we can test if it works so I'll leave this as assigned until then.

comment:4 Changed 4 months ago by cohosh

Since this is a linux-specific issue I just made a patch for go-webrtc instead of waiting for a PR.

It looks successful:

$ readelf -W -l TorBrowser/Tor/PluggableTransports/snowflake-client | grep GNU_STACK
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0x10

comment:5 Changed 4 months ago by cohosh

Status: assignedneeds_review

comment:6 Changed 4 months ago by gk

Keywords: TorBrowserTeam201905R added
Status: needs_reviewneeds_information

This seems to be something to review for us? Setting the respective keyword. So, *is* this just a Linux issue or not? comment:4 seems to suggest so, but the patch touches non-Linux parts as well (like in the go config file) which confuses me.

comment:7 in reply to:  6 Changed 4 months ago by cohosh

Status: needs_informationneeds_review

Replying to gk:

This seems to be something to review for us? Setting the respective keyword. So, *is* this just a Linux issue or not? comment:4 seems to suggest so, but the patch touches non-Linux parts as well (like in the go config file) which confuses me.

That was my bad, I forgot to make the environment variable change for linux only. I uploaded a new version of the patch that should fix this.

comment:8 Changed 4 months ago by gk

Keywords: TorBrowserTeam201905 added; TorBrowserTeam201905R removed
Status: needs_reviewneeds_revision

Per chat with cohosh: I'd take the patch if there is an upcoming release anyway but I'd prefer if we can get the fix merged upstream as any additional patch we need in tor-browser-build is a bug we should try to fix. I heard there are folks in Cc to this ticket who are able to review and push the pull request. Let's try to go that route first with a new patch that just updates the go-webrtc commit once the patch landed.

comment:9 Changed 4 months ago by cohosh

Status: needs_revisionneeds_review

It turns out there's an easier way to handle this by putting the cgo directives into an environment variable. I attached a new version of the patch to this ticket.

My reasoning for putting this in projects/go/config as opposed to just projects/go-webrtc/config is that this problem will occur in all go projects that use cgo, and it also allows us to use the template build script projects/go/var/build_go_lib in go-webrtc.

comment:10 Changed 4 months ago by gk

Keywords: TorBrowserTeam201905R added; TorBrowserTeam201905 removed

comment:11 in reply to:  9 Changed 4 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to cohosh:

It turns out there's an easier way to handle this by putting the cgo directives into an environment variable. I attached a new version of the patch to this ticket.

My reasoning for putting this in projects/go/config as opposed to just projects/go-webrtc/config is that this problem will occur in all go projects that use cgo, and it also allows us to use the template build script projects/go/var/build_go_lib in go-webrtc.

Nice! Looks good to me. Applied to tor-browser-build's master (commit 24f585bf1851bfa022128a5b587b7c0940ec775c).

comment:12 Changed 4 months ago by gk

Keywords: tbb-rbm added
Note: See TracTickets for help on using tickets.