Add a special permission to the jail to adjust the host time.
This can be useful if we want to compartmentalize the NTP daemon
from the rest of the system.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 58153 Build 55041: arc lint + arc unit
Event Timeline
Comment Actions
Great. I have personally wanted that for a while.
You need to add to the granted privileges those granted by mac_ntpd(4) (see the manual page or sys/security/mac_ntpd/mac_ntpd.c). As a consequence, I'd rename the option from adjtime to just settime as it seems more appropriate with these privileges (I did that as suggested edits in some places, hopefully this should make the compilation fail until all places are fixed, except for the doc ones of course).
| sys/kern/kern_jail.c | ||
|---|---|---|
| 226 | See general comment. | |
| 4176–4177 | More privileges are needed. See general comment. | |
| 4658–4660 | See general comment. | |
| sys/sys/jail.h | ||
| 257–259 | See general comment. | |
| usr.sbin/jail/jail.8 | ||
| 660 | Suggested rephrasing. | |
| sys/kern/kern_jail.c | ||
|---|---|---|
| 4176–4177 | I have missed the PRIV_CLOCK_SETTIME, the PRIV_NETINET_REUSEPORT is already granted for jails: /*
* Allow jailed root to reuse in-use ports.
*/
case PRIV_NETINET_REUSEPORT:
return (0);We have separate priv for PRIV_NETINET_RESERVEDPORT: allow.reserved_ports. | |
Comment Actions
Oh, you're right, I hadn't checked for the PRIV_NETINET_* ones. I think that how these are handled makes sense.
Last thing I'd suggest: Since the anticipated use case is to run ntpd, I would write somewhere in the documentation that, to run it in a jail, allow.settime is needed, as well as allow.reserved_ports if using the default port. Not sure where to put that however, and doesn't necessarily have to be part of this commit. Had it not been contrib software, I would probably have chosen ntpd(8). How about a note in the description of allow.settime here, wouldn't that be great (I anticipate that administrators will remember that there is some jail parameter to activate to set time, so will look up this one)? If you follow up with, e.g., some modification to libexec/rc/rc.d/ntpd, then some update to rc.conf(5) may be a good idea also.
Comment Actions
I don't think this is good policy, as is. I like being able to delegate to the jails, but I thing it's really bad to conflate settime and adjtime. The two are different, though related, things.
| sys/kern/kern_jail.c | ||
|---|---|---|
| 4176–4177 | I don't like CLOCK_SETTIME being conflated with Adjusting time. It's a much bigger foot cannon than you need. Conflating adjtime and ntp_adjtime are likely fine, though. I'd rather see it as a separate knob. why? adjtime can only move time a little from where we are right now in a continuous manner. settime is completely arbitrary and can move the time anywhere. Depending on your use case, you may need to grant both, but once the initial time is set close to right, you never want to step it again, and that's a bit at odds with doing this in a jail. | |
| sys/kern/kern_jail.c | ||
|---|---|---|
| 4176–4177 | Yea, that's a good point. I'm happy to separate them. | |
Comment Actions
Is there any value in a virtual time, i.e. letting a jail have its own clock? Apart from a test framework, I can't think of any, but maybe someone else can. That was actually my first thought when I saw the title of this, rather than encapsulating ntpd.
Comment Actions
That would be tricky to implement. The system has a very strong sense of base timekeepers, that have errors in frequency that are corrected to produce time. To have all that nuance available would be a lot of work.
Having a fixed offset for things like CLOCK_UPTIME, CLOCK_MONOTONIC and CLOCK_REALTIME per jail wouldn't be hard, but I fear that it would impose too much of an overhead for normal operations for a feature that seems will get little use and is only really good for testing in limited environments. The one thing people would want to test, leap seconds, aren't done in a way that's conducive to simulating enough of it in the jail to actually test the system's response to it. Leap seconds are signalled and applied at the wrong layer to make is easy to adjust the offsets by a second or to signal with the normal adjtime interfaces that a leap is pending.
But regardless of the merit of the idea, the difficulty in implementing it, etc, this isn't the right place to discuss that since it's neither an alternative to this submission, nor a trivial or a not-too-difficult follow-on to the original idea.
Comment Actions
I agree that they are slightly different. However, in practice, they are so related that I think it's a bad idea to separate them. Adding a knob doesn't cost much for sure, but is one more cognitive burden for the administrator, so it has to pay off. With adjtime(), it is not possible to set an arbitrary time immediately, but in practice you can skew clocks enough so as to disrupt a network or allow most timing attacks that setting an arbitrary date would open up. Moreover, the intended use case is arguably delegating system time management to one jail, in which case both knobs will be set to the same value anyway. Finally, mac_ntpd(4) doesn't treat separately PRIV_CLOCK_SETTIME, and consistency of behavior should always be seeked unless there are compelling reasons not too (again, for better intelligibility and less cognitive burden).
So, I'm not convinced this is the right way to go, as I see it as additional administrative complication for no real practical gain.
After some pondering, I'm requesting this change: Make allow.settime be a superset of allow.adjtime (see inline comments). I don't think granting the ability to set the time to arbitrary values but not that of slightly skewing it makes sense from a security standpoint. Doing so solves my concern about easing administration of the most probable use case.
And I also suggest to defer introduction of allow.adjtime until it proves necessary in practice, although, given its low cost, I do not insist on that.
Apart from a test framework, I can't think of any personally. Maybe others will.
I agree.
| sys/kern/kern_jail.c | ||
|---|---|---|
| 4178 | Please make PR_ALLOW_SETTIME a superset of PR_ALLOW_ADJTIME (see the global comment). | |
Comment Actions
As pointed out by olce@ the settime is more distructive, so we might
want to simply group them under one ambrela. And leave the adjust time
as a sperate option for more restrictive instalations.
Comment Actions
Some manpage nit, but after that should be good to go.
| usr.sbin/jail/jail.8 | ||
|---|---|---|
| 659–669 | Descriptions of both settings are swapped. | |
| usr.sbin/jail/jail.8 | ||
|---|---|---|
| 659–669 | I'm not a native, although I don't see much difference in adjusting and calibrating. For example: man page for ntp_adjtime says (Which is allowed by allow.adjtime): The ntp_adjtime() function is used by the NTP daemon to adjust the system clock to an externally derived time. The adjtime (also `allow.adjtime): The adjtime() system call makes small adjustments to the system time Date: date – display or set date and time And the clock_settime (the allow.settime): clock_gettime, clock_settime, clock_getres – get/set/calibrate date and
timeSo maybe we should go with descriptions like: Allow privileged process in the jail to adjust global operating system data and time. And settime: Allow privileged process in the jail to set and adjust global operating system date and time. | |
| usr.sbin/jail/jail.8 | ||
|---|---|---|
| 659–669 | Maybe there's a misunderstanding. Although the diff hightlights content change for both parameters, in reality my point was just to swap the description text of both. Given the descriptions, I assumed they were swapped. For example, date cannot be used with only allow.adjtime, it clearly belongs to allow.settime as an example. ntpd, in its default operation (i.e., if the clock isn't initially too far), will only adjust time, hence can work with allow.adjtime alone. I agree action verbs may be unclear. How about saying "slowly adjusting" for allow.adjtime and just "setting" for allow.settime? | |
| usr.sbin/jail/jail.8 | ||
|---|---|---|
| 659–669 | Hadn't fully read your reformulation suggestions, I think they are good as well. | |
| usr.sbin/jail/jail.8 | ||
|---|---|---|
| 659–669 | I'm confused ;P  | |
| usr.sbin/jail/jail.8 | ||
|---|---|---|
| 659–669 | Sorry, I mixed things up... Too sleep-deprived it seems. :-) | |