Created attachment 191820 [details] Patch for exp-run jail(2) has been the old way of adding jails for nearly a decade, and it's past due for moving into COMPAT. Also, the sysctls under security.jail that went along with it are more confusing than useful for the modern jail_set(2) world, and they should go away as well. My plan is to put jail(2) under #ifdef COMPAT_FREEBSD11, and the sysctls under #ifndef BURN_BRIDGES. But I want to make sure I can find and hopefully fix any ports still using jail(2). So the included patch actually removes jail(2) and the sysctls entirely, regardless of those defines, so an exp-run should catch anything still running under the old system.
Was this reviewed? It seems some BURN_BRIDGESXXX checks are reversed?
The BURN_BRIDGESXXX should in fact be all reversed, as BURN_BRIDGES is meant to take out code when it's defined, and (just for this testing patch) I went for a way to remove all the code when BURN_BRIDGESXXX *isn't* defined. The only review the(non-testing) patch has gotten (on D226931) was a suggestion that I try an exp-run. I have made sure the test patch at least makes a good kernel and world builds, but haven't tried anything with ports myself.
Some security.jail sysctl are actively used by poudriere, what is the replacement? src/share/poudriere/include/common.sh.freebsd: [ $(sysctl -n security.jail.socket_unixiproute_only) -eq 0 ]; then src/share/poudriere/include/common.sh.freebsd: [ $(sysctl -n security.jail.allow_raw_sockets) -eq 1 ]; then src/share/poudriere/include/common.sh.freebsd: [ $(sysctl -n security.jail.chflags_allowed) -eq 1 ]; then src/share/poudriere/include/common.sh.freebsd: [ $(sysctl -n security.jail.sysvipc_allowed) -eq 1 ]; then src/share/poudriere/common.sh: [ $(sysctl -n security.jail.enforce_statfs) -eq 1 ] || \ src/share/poudriere/common.sh: [ $(sysctl -n security.jail.mount_allowed) -eq 1 ] || \ src/share/poudriere/common.sh: [ $(sysctl -n security.jail.mount_devfs_allowed) -eq 1 ] || \ src/share/poudriere/common.sh: [ $(sysctl -n security.jail.mount_nullfs_allowed) -eq 1 ] || \ src/share/poudriere/common.sh: [ $(sysctl -n security.jail.mount_tmpfs_allowed) -eq 0 ] && \
(In reply to Antoine Brodin from comment #3) There's no current replacement, but I have a plan for one: getting those parameters (and others) from jls. I'll work on that as a separate issue, which this will be dependent upon.
(In reply to Jamie Gritton from comment #4) > (In reply to Antoine Brodin from comment #3) > There's no current replacement, but I have a plan for one: getting those > parameters (and others) from jls. I'll work on that as a separate issue, > which this will be dependent upon. These sysctl lookups in poudriere are only looked up if *in a jail already*. So jls won't work. Is there a different sysctl to lookup for that list?
(In reply to Bryan Drewery from comment #5) The idea will be to have "jls -j 0" to report on the current jail (if jailed). This would allow any jail parameter to be examined (with some exclusions for security), instead of just that static list. The problem I have with the sysctls isn't the situation around reading them, but that there's a long-standing and incorrect perception that setting them affects current jails. I would keep them around read-only, but their very existence would continue that confusion.
(In reply to Jamie Gritton from comment #6) > (In reply to Bryan Drewery from comment #5) > The idea will be to have "jls -j 0" to report on the current jail (if > jailed). This would allow any jail parameter to be examined (with some > exclusions for security), instead of just that static list. > I'm not fond of this as now I'll need 2 different lookups supported and the idea of running 'jls' from a jail but with -j0 to find out the current jail's parameters feels really wrong since it is not jail 0. > The problem I have with the sysctls isn't the situation around reading them, > but that there's a long-standing and incorrect perception that setting them > affects current jails. I would keep them around read-only, but their very > existence would continue that confusion. IMHO removing them (and not even setting read-only for a release or two) violates POLA and may break a lot of other scripts. Especially given this is security related I think they need to remain read-only for at least 1 release. I don't see the harm in having the read-only sysctls. It's not like you can set other read-only sysctls, like hw.ncpus, from the host or jail and yet people don't expect it to work. If they are not read-only now then yes that's a bug that needs to be fixed.
(In reply to Bryan Drewery from comment #7) True, when you're jailed the current jail isn't JID 0. It is in fact an unknown jid, which is a difficult thing to specify. JID 0, as much as it exists, is the base system i.e. the "where I am now" view when you are not jailed, which is reasonably similar to asking for the "where I am now" view when you are jailed. But that's a separate issue. > IMHO removing them (and not even setting read-only for a release or two) > violates POLA and may break a lot of other scripts. Which is the reason it was suggested to put it inside BURN_BRIDGES, which would affect very few systems. It would make sense to reduce it to read-only first and then remove it entirely later, but there seems to be one good switch (BURN_BRIDGES) making it difficult to have an option somewhere between where we are now and where I want to go. Another option is to change where I want to go, and just make read-only the end goal. I don't consider this the optimal end result, but it may be the POLA alternative. Nonetheless, I am hoping to see an exp-run with the sysctls removed entirely, so I can gauge just how widespread their use is.
(In reply to Jamie Gritton from comment #8) > (In reply to Bryan Drewery from comment #7) > True, when you're jailed the current jail isn't JID 0. It is in fact an > unknown jid, which is a difficult thing to specify. JID 0, as much as it > exists, is the base system i.e. the "where I am now" view when you are not > jailed, which is reasonably similar to asking for the "where I am now" view > when you are jailed. But that's a separate issue. > > > IMHO removing them (and not even setting read-only for a release or two) > > violates POLA and may break a lot of other scripts. > > Which is the reason it was suggested to put it inside BURN_BRIDGES, which > would affect very few systems. It would make sense to reduce it to > read-only first and then remove it entirely later, but there seems to be one > good switch (BURN_BRIDGES) making it difficult to have an option somewhere > between where we are now and where I want to go. > > Another option is to change where I want to go, and just make read-only the > end goal. I don't consider this the optimal end result, but it may be the > POLA alternative. Thanks for keeping an open mind at least. > > Nonetheless, I am hoping to see an exp-run with the sysctls removed > entirely, so I can gauge just how widespread their use is. Yup, though I suspect not much will care about them at build time. It may be something we need to grep for.
Mmmm, I am wondering how the exp-run will fare, if poudriere uses those sysctl, and they are not there any more, what will happen?
(In reply to Mathieu Arnold from comment #10) > Mmmm, I am wondering how the exp-run will fare, if poudriere uses those > sysctl, and they are not there any more, what will happen? It's fine. Poudriere only looks them up if it is jailed itself, if security.jail.jailed is 1.
New failures on amd64: + {"origin"=>"devel/py-freebsd", "phase"=>"build", "errortype"=>"gcc4_error"} + {"origin"=>"net-mgmt/bsnmp-jails", "phase"=>"build", "errortype"=>"???"} + {"origin"=>"security/pam_jail", "phase"=>"build", "errortype"=>"bad_C++_code"} + {"origin"=>"sysutils/jailutils", "phase"=>"configure", "errortype"=>"configure_error"} + {"origin"=>"sysutils/p5-BSD-Jail-Object", "phase"=>"build", "errortype"=>"clang"} + {"origin"=>"www/uwsgi", "flavor"=>"py36", "phase"=>"build", "errortype"=>"gcc4_error"} + {"origin"=>"www/uwsgi", "phase"=>"build", "errortype"=>"gcc4_error"} New failure logs on amd64: http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/py27-freebsd-0.9.3_8.log http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/bsnmp-jails-0.10_1.log http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/pam_jail-0.4.log http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/jailutils-1.7.log http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/p5-BSD-Jail-Object-0.02_4.log http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/uwsgi-2.0.16.log http://package18.nyi.freebsd.org/data/headamd64PR226931-default/2018-04-01_06h35m59s/logs/errors/uwsgi-py36-2.0.16.log For the sysctl removals, a grep in the jail related ports may be more reliable than an exp-run.
No news for 3 years, let's reassign back to reporter. Please reassign to portmgr if any action on portmgr side is required