Opened 5 years ago

Last modified 2 years ago

#14186 new defect

Try to use fchmod() first when changing permissions on an AF_UNIX socket

Reported by: andrea Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.6.2-alpha
Severity: Normal Keywords: tor-client tor-relay posix permissions
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In connection_listener_new() of connection.c, we need to change permissions of an AF_UNIX socket in one case, and on some platforms it doesn't work to use fchmod(), or so a comment claims. It'd be better to avoid the race condition by using fchmod() when possible though.

We should move this to a unix_socket_chmod() function in compat.c, perhaps, which should take a file handle, a path and a mode, and try fchmod() and then fall back to chmod() if it fails.

Point of clarification: if fchmod() fails, will it fail by returning a sensible error code or by silently not modifying the permissions? Should unix_socket_chmod() also fdstat() as needed to check that the mode is correct?

Child Tickets

Change History (14)

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.???

sticking this in 0.2.???. Please put it back if you don't agree. :)

comment:2 Changed 5 years ago by nickm

Milestone: Tor: 0.2.???Tor: 0.2.7.x-final

Tentatively move some tickets to 0.2.7

comment:3 Changed 5 years ago by nickm

Status: newassigned

comment:4 Changed 5 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:5 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:6 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:7 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:8 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:9 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:10 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:11 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:12 Changed 2 years ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:13 Changed 2 years ago by nickm

Keywords: tor-client tor-relay posix permissions added
Severity: Normal

comment:14 Changed 2 years ago by f55jwk4f

nickm, it seems like you are the one who added the comment saying fchmod doesn't work on all platforms. So exactly on which platforms do fchmod doesn't work but chmod does? On these platforms, does chmod actually prevent processes without proper permissions to access the socket? I found that some operating systems don't honor permissions on unix sockets. (https://groups.google.com/d/topic/nodejs/6wXjU1n9U3w) And Posix says "may" for this case. Linux has commit 5822b7faca709c03a59c2929005bfe9caffe6592 which says we can fchmod before bind.

Note: See TracTickets for help on using tickets.