Opened 12 months ago

Closed 9 months ago

#24315 closed defect (fixed)

sandbox incompatible with glibc 2.26 (openat() not handled for all our files)

Reported by: dgoulet Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: sandbox, 031-backport, 029-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

If I enable the sandbox on my system, I get killed with:

(Sandbox) Caught a bad syscall attempt (syscall openat)
/usr/lib/x86_64-linux-gnu/libasan.so.4(+0x558c0)[0x7fb5203ab8c0]
/lib/x86_64-linux-gnu/libpthread.so.0(open64+0x4e)[0x7fb51ec6667e]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x13150)[0x7fb51ec67150]
/lib/x86_64-linux-gnu/libpthread.so.0(open64+0x4e)[0x7fb51ec6667e]
/usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(evutil_open_closeonexec_+0x20)[0x7fb51fbb5540]
/usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(evutil_read_file_+0x53)[0x7fb51fbb5603]
/usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(evdns_base_load_hosts+0x8b)[0x7fb51fbc429b]
/home/dgoulet/Documents/git/tor/src/or/tor(+0x9f7adf)[0x55aa44446adf]
/home/dgoulet/Documents/git/tor/src/or/tor(do_main_loop+0x745)[0x55aa440f01d5]
/home/dgoulet/Documents/git/tor/src/or/tor(tor_run_main+0x1895)[0x55aa440f4065]
/home/dgoulet/Documents/git/tor/src/or/tor(tor_main+0x86)[0x55aa440e1fb6]
/home/dgoulet/Documents/git/tor/src/or/tor(main+0x1c)[0x55aa440df20c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7fb51db741c1]
/home/dgoulet/Documents/git/tor/src/or/tor(_start+0x2a)[0x55aa440e1c6a]

strace output:

19360 openat(AT_FDCWD, "/etc/hosts", O_RDONLY|O_CLOEXEC) = 257
19360 --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0x7f7d90ab667e, si_syscall=__NR_openat, si_arch=AUDIT_ARCH_X86_64 ---}

It is the first file being opened _after_ the seccomp sandbox has been applied. Our sandbox code only considers "open()" to touch that file:

OPEN("/etc/hosts");

My libc is 2.26.

We probably need to handle the same files with openat() as we do with open() for this.

Child Tickets

Change History (13)

comment:1 Changed 12 months ago by nickm

So, in the libevent source code at that point, it calls open, not openat. Is libc turning that open() into an openat? Did the linux kernel unify the open and openat calls?

comment:2 Changed 12 months ago by dgoulet

Hmmm that glibc source code is kind of _crazy_ but I see that __libc_open() functions in sysdeps/unix/sysv/linux/open.c is calling:

  return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag, mode);

A small test only doing open() on a file does call openat() and nothing else.

So I assume they did (well at least on glibc 2.26 Ubuntu).

comment:3 Changed 12 months ago by nickm

the change seems to have been made with glibc commit b41152d716ee9c5ba34495a54e64ea2b732139b5, which first appeared in 2.26.

comment:4 Changed 12 months ago by nickm

we can detect the specific version at runtime with gnu_get_libc_version(). Of course, it would be cleaner to call open() and somehow discover whether open() or openat() is being called.

comment:5 Changed 12 months ago by dgoulet

Owner: changed from dgoulet to nickm
Reviewer: dgoulet

comment:6 Changed 12 months ago by nickm

Status: assignedneeds_review

ticket24315_029 has some necessary changes, I hope. I've tested a client a bit, but I haven't tested in too much detail.

One point for consideration during review -- I'm not sure whether the old openat() rules need to remain at all, or if there really were any "opens" that should have not have been openats.

comment:7 Changed 12 months ago by nickm

Summary: sandbox: openat() not handled for all our filessandbox incompatible with glibc 2.26 (openat() not handled for all our files)

comment:8 Changed 12 months ago by dgoulet

Status: needs_reviewmerge_ready

Branch lgtm; but I can't test this because IMm now getting:

(Sandbox) Caught a bad syscall attempt (syscall mprotect). I'll open a ticket for that.

comment:9 Changed 12 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.1.x-final

After review and testing from dgoulet, merging this to 0.3.2 and forward; marking for possible backport.

comment:10 Changed 12 months ago by Sebastian

cypherpunks notes that #24400 applies here, too.

comment:11 Changed 10 months ago by teor

Keywords: 031-backport 029-backport added

Marking for possible backport to 0.3.1 and 0.2.9

comment:12 Changed 9 months ago by nickm

Milestone: Tor: 0.3.1.x-finalTor: 0.2.9.x-final

This has been cooking long enough without apparent trouble: merging this branch back to 0.2.9 and 0.3.1.

comment:13 Changed 9 months ago by nickm

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.