journal: explicitly name the abstract unix socket

[champtar]

File based unix socket are part of the mount namespace.
Abstract unix socket are part of the network namespace, meaning processes
inside hostNetwork containers can read/write to host abstract unix socket,
so depending of the usage this can be an attack vector.

While looking at a Kubernetes cluster, it's hard to find out why
kubelet / kube-controller / kube-scheduler / calico have a randomly named
abstract unix socket, so name it explicitly.

[lucab]

Thanks for the patch. However, while I understand the rationale behind this PR, it is a purely-ergonomic change that has a nasty side-effect of draining entropy in init(), and it also exposes the socket name to squatting.

[champtar]

Hi @lucab

abstract unix socket are a security risk with hostNetwork containers, see https://research.nccgroup.com/2020/11/30/technical-advisory-containerd-containerd-shim-api-exposed-to-host-network-containers-cve-2020-15257/
Being able to quickly determine that a program is safe is really important IMO.

crypto/rand uses getrandom(), which once seeded never block. There is a lot of misconceptions around /dev/(u)random and getrandom(), but using it doesn't make your system less secure (https://www.2uo.de/myths-about-urandom/)

Now for the socket name squatting, I add a 16 bytes random string, so I'm not sure how you want that to collide.

Creating a temporary file based unix socket is a problem on read only filesystem.
Maybe we could Dial() journalSocket, but init() will fail if it's not present

[champtar]

@lucab can I do anything to convince you or should we just close this ?

[lucab]

While I do see the appeal of having a known-prefix for this abstract socket, I'm still unconvinced this outweighs the drawbacks from this change.

The referenced CVE is caused by a (basically unauthenticated) RPC server on a local socket. But the abstract socket usage here is neither a server nor some kind of unauthenticated RPC: it is only used for sending log messages to journald. A prefix does not make the library any safer in that regard, as that security scenario does not apply here.

The entropy draining is a real concern, but unrelated to security, no misconceptions there. This library is used by early-boot processes and this kind of logic in other libraries has already caused dead-locking issues due to starvation/blocking, see census-instrumentation/opencensus-go#1228 for a recent example.

Finally, while it's a fair point that chances of collisions are low if we drain enough entropy (but see point above), the current code guarantees no collision as the kernel picks a known-free address for us.
I guess that what we'd really like here is a "autobind using given prefix" primitive for abstract sockets, which the Linux kernel does not currently provide (to the best of my knowledge).