jail: add allow.mount.all to allow mounting any filesystem

[oshogbo]

Summary

jail: add allow.mount.all to allow mounting any filesystem

Add a new jail parameter allow.mount.all that allows mounting
any filesystem type. This is useful for build jails or development
environments where the jail root user is trusted. It is especially
useful when we want to restric the /dev visibility inside the jail,
when working on some scripts and we dont want to accidentally
overwritten other disks.

[kib]

UFS cannot be used like this. The code trusts the on-disk format, and malicious jail user could cause almost everything to kernel code, like arbitrary memory overwrite.

[oshogbo]

I see your point.

However, I have a use case where I'm building a UFS image inside a jail using md devices, etc., and I don't want to expose the entire system to that subsystem. I'd prefer to limit its visibility to disks and related resources. Not because I don't the generated filesystem, but mainly to reduce the risk of accidental user mistakes.

I was thinking this kind of flag could follow an "enable at your own risk" model.
Do you think it we can change this and add flag like a dangerous_allow_mount_ufs flag?

Docker on Linux has a similar concept with the --privileged flag, which allows users to shoot themselves in the foot if they choose to.

[kib]

Might be, instead of adding per-filesystem VFCF_JAIL flag, add one single flag allowing user-mounting anything. Then document it properly.

[oshogbo]

Introduce the single flag allowing mounting anything.

[kib]

sys/sys/jail.h

284–293

This is really awful. Can it be converted to the (long) or-ed line of symbolic names?

[jamie]

There's no need for the two-layer name "unsafe.all". If you really want both "unsafe" and "all" in the name allow.mount.unsafe_all should do. Better yet would be to keep is simple with allow.mount.all to allow all filesystems, with the understanding that such a thing might be unsafe. As it stands, there's this "allow.mount.unsafe" hierarchy, which suggests that allowing all filesystem types is unsafe, which each of those filesystem types is implicitly labeled as safe.

[oshogbo]

My thinking was that we might later discover the feature needs more granularity, and therefore introduce options like:

allow.mount.unsafe.all
allow.mount.unsafe.ufs
allow.mount.unsafe.zfs
ect.

In my opinion its more elegant.

allow.mount.all to allow all filesystems

I would prefer to keep it explicit that this is unsafe and that you are potentially shooting yourself in the foot, since allow.mount.all suggests this is a normal or recommended operation.

allow.mount.unsafe_all

If that naming is preferred, I’m fine with it.

[oshogbo]

@jamie are you ok with current state?

[jamie]

No, I still hold that the extra level adds nothing.

As far as I see, every filesystem you can mount has its security concerns; that's why they're not allowed by default. For that matter, practically every option you have for jails could be called "unsafe" in some way or another, as they are almost all used to allow things that a default "fully safe" jail doesn't have. But I don't want to turn every jail.allow.foo into jail.allow.unsafe.foo; that's as useful as every action in a program having an "are you sure?" box you need to click through, training all users to click through boxes without reading them.

If we want to allow mounting all filesystem types, allow.mount.all does the trick.

[mckusick]

This is a useful addition. I don't have an opinion on whether unsafe should be in the name.

[oshogbo]

Drop untrust keyword.

[mckusick]

Still fine with this change.

[jamie]

Only one little thing to go, so I'll call it approval. Since the parameter is allow.mount.all, the associated flag should be PR_ALLOW_MOUNT_ALL, not PR_ALLOW_MOUNT_ANY.