gopass must remove syscall(2) usage on OpenBSD

[jrick]

Summary

gopass built for OpenBSD from the master branch will fail to link to syscall(2) at runtime once OpenBSD removes syscall(2) from libc.

There are two usages of syscall(2) that I could find: one is due to pledge (will be fixed by https://golang.org/cl/468095), and the other is in readline, which I've opened a PR for. Unfortunately, it doesn't seem that the owner is maintaining that readline library anymore, and a switch to that other fork might be needed, unless you wish to use a module replacement that points to my PR's branch.

Steps To Reproduce

$ pwd
/home/jrick/src/gopass
$ go build
$ ./gopass
gopass:./gopass: undefined symbol 'syscall'
ld.so: gopass: lazy binding failed!
Killed 

Expected behavior

gopass executes without hitting a lazy linking error.

Environment

• OS: OpenBSD
• OS version: -current with the full syscall(2) removal patch, including removal from libc
• gopass Version: master branch
• Installation method: repo

Additional context

A working gopass can be built with patches to x/sys and readline.

$ pwd
/home/jrick/src/gopass
$ got br
master
$ got status
$ go work init
$ go work use .
$ go work use ../sys # golang.org/cl/468095
$ go work use ../readline # github.com/chzyer/readline/pull/232
$ go build
$ ./gopass

   __     _    _ _      _ _   ___   ___
 /'_ '\ /'_'\ ( '_'\  /'_' )/',__)/',__)
( (_) |( (_) )| (_) )( (_| |\__, \\__, \
'\__  |'\___/'| ,__/''\__,_)(____/(____/
( )_) |       | |
 \___/'       (_)

� Welcome to gopass!
â�  This is the built-in shell. Type 'help' for a list of commands.
gopass> ^D

[jrick]

One other additional note, this patched and working gopass still does reference syscall(2), but I could not find where, and have not seen any other linking crashes. It might be benign, or I may not have tested thoroughly enough.

$ nm -s ./gopass | grep 'U syscall' 
         U syscall

[dominikschulz]

Thank you. I think we might try switching to https://github.com/ergochat/readline if the author doesn't merge your PR soon.

[jrick]

I found one other issue performing system calls on OpenBSD, in the otp screenshot action. It's likely the only remaining reason why syscall(2) remains seen in the nm symbol output.

$ grep -R syscall.Syscall vendor/github.com/gen2brain/shm
vendor/github.com/gen2brain/shm/shm.go:	id, _, errno := syscall.Syscall(sysShmGet, uintptr(int32(key)), uintptr(int32(size)), uintptr(int32(shmFlg)))
vendor/github.com/gen2brain/shm/shm.go:	addr, _, errno := syscall.Syscall(sysShmAt, uintptr(int32(shmId)), shmAddr, uintptr(int32(shmFlg)))
vendor/github.com/gen2brain/shm/shm.go:		syscall.Syscall(sysShmDt, addr, 0, 0)
vendor/github.com/gen2brain/shm/shm.go:	result, _, errno := syscall.Syscall(sysShmDt, uintptr(unsafe.Pointer(&data[0])), 0, 0)
vendor/github.com/gen2brain/shm/shm.go:	result, _, errno := syscall.Syscall(sysShmCtl, uintptr(int32(shmId)), uintptr(int32(cmd)), uintptr(unsafe.Pointer(buf)))
$ go list all | grep github.com/gen2brain/shm             
github.com/gen2brain/shm
$ go mod why github.com/gen2brain/shm                     
# github.com/gen2brain/shm
github.com/gopasspw/gopass/pkg/otp
github.com/kbinani/screenshot
github.com/kbinani/screenshot/internal/xwindow
github.com/gen2brain/shm

$ ktrace ./gopass otp -s example.com
Abort trap (core dumped) 
$ kdump | tail    
 93081 gopass   CALL  read(3,0xc00027a819,0x1e7)
 93081 gopass   RET   read 0
 93081 gopass   CALL  close(3)
 93081 gopass   RET   close 0
 93081 gopass   CALL  kbind(0x2298d29e8,24,0xaae97868828fe3c9)
 93081 gopass   RET   kbind 0
 93081 gopass   CALL  socket(AF_UNIX,0xc001<SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK>,0)
 93081 gopass   PLDG  socket, "inet", errno 1 Operation not permitted
 93081 gopass   PSIG  SIGABRT SIG_DFL
 93081 gopass   NAMI  "gopass.core"

The reason this dies with "abort trap" is due to a pledge violation ("inet" is required, but the pledge promise is missing in this codepath).

If pledge were to be commented out:

$ ./gopass otp -s example.com        
⚠ Scanning screen n°0
gopass:./gopass: undefined symbol 'syscall'
ld.so: gopass: lazy binding failed!
Killed 

There are no shmat(2) wrappers in x/sys/unix to replace the shm package. This would need to be another addition to the x/sys module if this feature is to work at all (along with adding an inet pledge...).

I suspect nobody actually wants to add an inet pledge to a password manager, so it's probably best to completely disable this feature on OpenBSD, and use conditional compilation to avoid the lazy syscall(2) linking.

[sthen]

In most cases where you'd want to use this (fetching from a local X display) it would actually be a "unix" pledge, "inet" would only be needed if DISPLAY points to a remote machine. But then it requires shmget which is not permitted by any pledge.

I'm not really convinced pledge is adding much here anyway, firstly what's committed is incorrect ("fattr" is also needed because chmod is called), but since "exec" is also used, if you can subvert the process sufficiently you can just run some other command anyway (which is unrestricted), it might make more sense to just remove the pledge call.

[dominikschulz]

I not very familiar with OpenBSD. IIRC pledge was contributed by an OpenBSD user. We can remove it if we have a good reason to do so.

Also disabling the screenshot feature via build tags / conditional builds sounds good.

[jrick]

A decision needs to be made whether to keep pledge, and disable the screenshot feature, or leave it enabled but disable pledge entirely. As @sthen noted, we don't need "inet" to open the socket for the local display, but we do need to call shmget(), which is not allowed by any pledged program (pledge starts with nothing and enables subsets of POSIX, it doesn't simply remove parts, and nothing pledge can do can give you back shmget()).

(And also, for this screenshot feature to work in a pledge-disabled gopass, it will need to make the proper libc calls instead of syscall(2). That will need more additions to x/sys.)

[AnomalRoil]

If that's the case, then disabling the screenshot feature on OpenBSD with a build tag sounds like the more logical thing to do anyway.

I wish I knew if our users even use the feature. I sure do, but maybe I'm the only one, especially since it requires CGO on Darwin 😅

[dominikschulz]

@AnomalRoil Could you maybe take care of disabling the screenshot feature on OpenBSD? You did contribute this feature in the first place.

I will take care of switching out readline and the x/sys change should eventually land upstream - nothing we can do here to speed that up.

[dominikschulz]

Unfortunately switching out the readline lib is not entirely trivial, so I'd wait a bit if they still merge that PR. If not I'd first consider a replacement rule.

[AnomalRoil]

@jrick Do you know if NetBSD and FreeBSD are also concerned by the same issue?

[jrick]

they are not, this is something happening first (and only?) in openbsd

[dominikschulz]

Ok, so the ergochat/readline folks are amazing. I have asked for a ClearScreen method. They immediately replied, put up a PR and even tested it with my gopass PR. 🤯

Looks like we should be able to sort this out within a few days. Thanks a lot everyone, great effort 👍

[jrick]

A new version of x/sys was tagged a few days back (v0.14.0) that fixes the pledge calls. With just that module updated over current master branch, gopass is working properly again on OpenBSD.

edit: oh and "fattr" also needs to be added to the pledge list still. https://man.openbsd.org/pledge#fattr

[jrick]

Here I can get it to hit the pledge violation after setting incorrect permissions in my store.

$ ./gopass fsck 
Checking password store integrity ...

[] Permissions too wide: /home/jrick/.local/share/gopass/stores/root/websites/example.com/example.gpg (-rw-rw-rw-)

[]   Fixing permissions from -rw-rw-rw- to -rw-------
Abort trap (core dumped) 

Adding fattr fixes this (but hits some other issue regarding git after the fact. gopass git status says nothing to commit, working tree clean, but it looks like it's trying to commit changes under this state)

$ ./gopass fsck 
Checking password store integrity ...

[] Permissions too wide: /home/jrick/.local/share/gopass/stores/root/websites/example.com/example.gpg (-rw-rw-rw-)

[]   Fixing permissions from -rw-rw-rw- to -rw-------
] 1 / 2 [Gooooooooooooooooooooooooooooooooooooooooooooooooopass                                                      ]  50.00% 
❌ fsck failed on root store: failed to commit changes to git: exit status 1: 

Error: fsck found errors: failed to commit changes to git: exit status 1: 

[dominikschulz]

I think we fixed all relevant issues here, correct? Otherwise feel free to re-open.