Bug 261298 - rc.d/ntpd: /var/db/ntpd.leap-seconds.list wrong permissions/owner
Summary: rc.d/ntpd: /var/db/ntpd.leap-seconds.list wrong permissions/owner
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Cy Schubert
URL:
Keywords: easy, needs-qa
Depends on:
Blocks:
 
Reported: 2022-01-18 06:30 UTC by Martin Waschbüsch
Modified: 2022-01-23 20:28 UTC (History)
1 user (show)

See Also:
koobs: maintainer-feedback? (cy)
koobs: mfc-stable13?
koobs: mfc-stable12?


Attachments
Circumvent login.conf umask (502 bytes, patch)
2022-01-18 07:40 UTC, Cy Schubert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Waschbüsch 2022-01-18 06:30:46 UTC
Regularly, /var/db/ntpd.leap-seconds.list ends up being owned by root and permissions of 640.
On restart of ntpd this leads to:

Jan 18 06:14:56 ns00 ntpd[30173]: leapsecond file ('/var/db/ntpd.leap-seconds.list'): open failed: Permission denied

The file should either be owned by ntpd (which ntpd runs as) or have permissions of 644.

This happens because
a) the file is owned by root:wheel and 
b) I changed umask in /etc/login.conf to 027

ntpd settings in rc.conf are:

ntpd_enable="YES"
ntpd_sync_on_start="YES"

ntpd leapfile is checked and fetched daily as specified in
/etc/periodic/daily/480.leapfile-ntpd

the logic is contained in the ntpd_fetch_leapfile() function within /etc/rc.d/ntpd

That function should take care of correct ownership of the file, so I propose to fix ownership to ntpd:ntpd there. However, there may be reasons why ownership root:wheel and permissions 644 is a better idea for some reason I have missed, thus I am not providing a patch.
Comment 1 Cy Schubert freebsd_committer freebsd_triage 2022-01-18 06:50:58 UTC
The file should be 644.

What is the output of,

ls -l /var/db/ntpd.leap-seconds.list

and

ls -lc /var/db/ntpd.leap-seconds.list

Both outputs are important to understanding the cause. (Else we have to guess.)
Comment 2 Martin Waschbüsch 2022-01-18 07:00:08 UTC
(In reply to Cy Schubert from comment #1)

If the file should be root-owned and 644 then the I assume the reason is the changed umask as the rc.d script and hence the updating fetch is run as root, no?

Anyway, the requested output:

root@box:~# ls -l /var/db/ntpd.leap-seconds.list
-rw-r-----  1 root  wheel  10659 Oct 22 02:20 /var/db/ntpd.leap-seconds.list

root@box:~# ls -lc /var/db/ntpd.leap-seconds.list
-rw-r-----  1 root  wheel  10659 Nov 28 03:09 /var/db/ntpd.leap-seconds.list
Comment 3 Martin Waschbüsch 2022-01-18 07:02:21 UTC
(In reply to Cy Schubert from comment #1)

Also, depending on whether I first changed the umask or first enabled ntpd, it may have been an initial fetch instead of an update? Or is the /var/db/ntpd.leap-seconds.list part of a clean install already? Just a thought.
Comment 4 Cy Schubert freebsd_committer freebsd_triage 2022-01-18 07:40:22 UTC
Created attachment 231113 [details]
Circumvent login.conf umask

This should circumvent umask in login.conf.

Though, your ctime shows that the file's metadata, including permissions, were not recently changed. They were changed Nov 28.
Comment 5 Martin Waschbüsch 2022-01-18 07:49:17 UTC
(In reply to Cy Schubert from comment #4)

Thanks! I'll give it a spin.

As for the metadata: It is true, this is not a recent thing. I've seen the error from time to time in the logs, but today was the day when I 'had enough' and tried to look into it. :-)

Btw., just out of curiosity, what is the rationale for the file to not be owned by ntpd? Are other processes using it, too?
Comment 6 Martin Waschbüsch 2022-01-18 08:42:25 UTC
(In reply to Cy Schubert from comment #4)

FWIW, the patch works as intended which is, to enforce default umask of 022 when fetching an updated leapfile.

I still cannot think of any reason the file *has* to be world-readable or owned by root, but I suppose it is way safer preserving the status quo than finding out weird and unexpected consequences later.

Anyway, thanks again!
Comment 7 Cy Schubert freebsd_committer freebsd_triage 2022-01-18 14:12:53 UTC
It needs to be owned by root to mitigate any possibility of a compromised NTP being used to alter the file and therefore alter time on the system, which BTW could be used to further compromise the system. Sad to say, ntp has had its share of CVEs. However this is true of any service. Daemons should only be allowed to own (write to) files absolutely necessary for their operation.

The principle is a sound one also for services like web services. For example, should apache be compromised and the content owned by the apache account also be altered. In that case the apache daemon might run under the apache account while the content might be owned by a wwwadm or other web content account which allows only global or group-only read access. In this case the file is conveniently owned by root.

I'll commit the patch sometime this week.
Comment 8 Martin Waschbüsch 2022-01-18 14:57:43 UTC
(In reply to Cy Schubert from comment #7)

That makes sense. I guess in my mind I was somehow mixing this up with the drift file that ntpd needs full access to. But you are right. This is strictly for looking up data that lets ntpd know how to behave and that ntpd must not be able to change.

Thank you for taking the time to elaborate!
Comment 9 Cy Schubert freebsd_committer freebsd_triage 2022-01-18 15:35:11 UTC
Thinking I may as well push this commit today with the wpa upgrade I had planned to push today and MFC it on Thursday with a bunch of ipfilter MFCs.
Comment 10 commit-hook freebsd_committer freebsd_triage 2022-01-18 15:35:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c6806434e79079f4f9419c3ba4fec37efcaa1635

commit c6806434e79079f4f9419c3ba4fec37efcaa1635
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-01-18 14:14:54 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-01-18 14:21:00 +0000

    rc.d/ntp: Ensure ntpd.leap-seconds.list is readable by ntpd

    When a use sets umask in login.conf(5) to 027 or 077 a subsequently
    fetched /var/db/ntpd.leap-seconds.list will inherit the permissions
    allowed by the umask, resulting in a file that may not be readable
    ntpd running under the ntp account. This patch adds a umask command
    to preempt the umask in login.conf(5) prior to fetching a new copy
    of the leap-seconds file.

    PR:             261298
    Reported by:    Martin Waschbusch <martin@waschbuesch.de>
    MFC after:      3 days

 libexec/rc/rc.d/ntpd | 2 ++
 1 file changed, 2 insertions(+)
Comment 11 commit-hook freebsd_committer freebsd_triage 2022-01-23 20:26:13 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=bb66b7c06e1cfd688632e11716d0064cb0385183

commit bb66b7c06e1cfd688632e11716d0064cb0385183
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-01-18 14:14:54 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-01-23 20:23:08 +0000

    rc.d/ntp: Ensure ntpd.leap-seconds.list is readable by ntpd

    When a use sets umask in login.conf(5) to 027 or 077 a subsequently
    fetched /var/db/ntpd.leap-seconds.list will inherit the permissions
    allowed by the umask, resulting in a file that may not be readable
    ntpd running under the ntp account. This patch adds a umask command
    to preempt the umask in login.conf(5) prior to fetching a new copy
    of the leap-seconds file.

    PR:             261298
    Reported by:    Martin Waschbusch <martin@waschbuesch.de>

    (cherry picked from commit c6806434e79079f4f9419c3ba4fec37efcaa1635)

 libexec/rc/rc.d/ntpd | 2 ++
 1 file changed, 2 insertions(+)
Comment 12 commit-hook freebsd_committer freebsd_triage 2022-01-23 20:27:14 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=28d0a7821adc1421b26c45b5cd8ab6532e55ba62

commit 28d0a7821adc1421b26c45b5cd8ab6532e55ba62
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-01-18 14:14:54 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-01-23 20:24:36 +0000

    rc.d/ntp: Ensure ntpd.leap-seconds.list is readable by ntpd

    When a use sets umask in login.conf(5) to 027 or 077 a subsequently
    fetched /var/db/ntpd.leap-seconds.list will inherit the permissions
    allowed by the umask, resulting in a file that may not be readable
    ntpd running under the ntp account. This patch adds a umask command
    to preempt the umask in login.conf(5) prior to fetching a new copy
    of the leap-seconds file.

    PR:             261298
    Reported by:    Martin Waschbusch <martin@waschbuesch.de>

    (cherry picked from commit c6806434e79079f4f9419c3ba4fec37efcaa1635)

 libexec/rc/rc.d/ntpd | 2 ++
 1 file changed, 2 insertions(+)
Comment 13 Cy Schubert freebsd_committer freebsd_triage 2022-01-23 20:28:23 UTC
Fixed.