Re: move cron socket to /var/run/cron.sock (pledge)
Theo de Raadt writes: >> Grmbl. I've hard a hard time trying to understand *why* this would be >> needed. The answer is pledge(2), who makes chmod(2) fail with EPERM >> instead of killing the process. >> >> I find this confusing. IMO pledge(2) should let the kernel do the >> appropriate security checks for chown(2). > > Cannot. pledge handles *chown() at a realistic level. > > Otherwise, we'd need pledge checks in every function reachable > by VOP_SETATTR. I'm not sure I understand the reasons, but I'll trust you on that one. Still I find this change in behavior confusing, and I hope it won't bite us in the end. I'd prefer cron not to change its gid for a weird reason, or maybe change it only around the socket chmod call, with a comment explaining why this is necessary. Otherwise, millert's diff looks good, works fine and is a very desirable improvement IMO. ok jca@ but please consider the paragraph above. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: move cron socket to /var/run/cron.sock (pledge)
> Grmbl. I've hard a hard time trying to understand *why* this would be > needed. The answer is pledge(2), who makes chmod(2) fail with EPERM > instead of killing the process. > > I find this confusing. IMO pledge(2) should let the kernel do the > appropriate security checks for chown(2). Cannot. pledge handles *chown() at a realistic level. Otherwise, we'd need pledge checks in every function reachable by VOP_SETATTR.
Re: move cron socket to /var/run/cron.sock
> There's limited backward compatibility so you can run a new crontab > with an older cron daemon. Why? That makes no sense. This kind of compat bubbles up. It should be deleted within a week, so why bother writing it...
Re: move cron socket to /var/run/cron.sock (pledge)
"Todd C. Miller" writes: > On Wed, 11 Nov 2015 23:30:48 +0100, > =?utf-8?Q?J=C3=A9r=C3=A9mie_Courr=C3=A8ges- > Anglas?= wrote: > >> "Todd C. Miller" writes: >> >> > On Wed, 11 Nov 2015 14:43:47 -0700, "Todd C. Miller" wrote: >> > >> >> There's limited backward compatibility so you can run a new crontab >> >> with an older cron daemon. >> > >> > Revised diff, I neglected to send out the cron.c changes in the >> > first one. >> >> The socket doesn't inherit the crontab group from its parent directory >> anymore. > > I was wondering if anyone would notice that. I fixed that after I > had already sent the updated diff. This versions sets cron's egid > to crontab so it can chmod the socket. Grmbl. I've hard a hard time trying to understand *why* this would be needed. The answer is pledge(2), who makes chmod(2) fail with EPERM instead of killing the process. I find this confusing. IMO pledge(2) should let the kernel do the appropriate security checks for chown(2). -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: move cron socket to /var/run/cron.sock
"Todd C. Miller" writes: > On Wed, 11 Nov 2015 14:43:47 -0700, "Todd C. Miller" wrote: > >> There's limited backward compatibility so you can run a new crontab >> with an older cron daemon. > > Revised diff, I neglected to send out the cron.c changes in the > first one. The socket doesn't inherit the crontab group from its parent directory anymore. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: move cron socket to /var/run/cron.sock
On Wed, 11 Nov 2015 23:30:48 +0100, =?utf-8?Q?J=C3=A9r=C3=A9mie_Courr=C3=A8ges-
Anglas?= wrote:
> "Todd C. Miller" writes:
>
> > On Wed, 11 Nov 2015 14:43:47 -0700, "Todd C. Miller" wrote:
> >
> >> There's limited backward compatibility so you can run a new crontab
> >> with an older cron daemon.
> >
> > Revised diff, I neglected to send out the cron.c changes in the
> > first one.
>
> The socket doesn't inherit the crontab group from its parent directory
> anymore.
I was wondering if anyone would notice that. I fixed that after I
had already sent the updated diff. This versions sets cron's egid
to crontab so it can chmod the socket. There will be a cleanup of
set_cron_cwd() in a future diff.
- todd
Index: usr.sbin/cron/client.c
===
RCS file: /cvs/src/usr.sbin/cron/client.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 client.c
--- usr.sbin/cron/client.c 11 Nov 2015 17:05:23 - 1.6
+++ usr.sbin/cron/client.c 11 Nov 2015 22:32:43 -
@@ -19,9 +19,12 @@
#include
#include
+#include
#include
#include /* for structs.h */
+#include
+#include
#include
#include
#include
@@ -92,13 +95,17 @@ void
poke_daemon(const char *spool_dir, unsigned char cookie)
{
int sock = -1;
+ const char *cronsock = CRONSOCK;
+ struct stat sb;
struct sockaddr_un s_un;
+ if (stat(cronsock, &sb) != 0)
+ cronsock = CRONSOCK_OLD;/* backwards compatibility */
+
bzero(&s_un, sizeof(s_un));
- if (snprintf(s_un.sun_path, sizeof s_un.sun_path, "%s/%s",
- CRON_SPOOL, CRONSOCK) >= sizeof(s_un.sun_path)) {
- fprintf(stderr, "%s: %s/%s: path too long\n",
- __progname, CRON_SPOOL, CRONSOCK);
+ if (strlcpy(s_un.sun_path, cronsock, sizeof(s_un.sun_path)) >=
+ sizeof(s_un.sun_path)) {
+ warnc(ENAMETOOLONG, "%s", cronsock);
return;
}
s_un.sun_family = AF_UNIX;
@@ -106,8 +113,7 @@ poke_daemon(const char *spool_dir, unsig
connect(sock, (struct sockaddr *)&s_un, sizeof(s_un)) == 0)
send(sock, &cookie, 1, MSG_NOSIGNAL);
else
- fprintf(stderr, "%s: warning, cron does not appear to be "
- "running.\n", __progname);
+ warnx("warning, cron does not appear to be running");
if (sock >= 0)
close(sock);
}
Index: usr.sbin/cron/common.c
===
RCS file: /cvs/src/usr.sbin/cron/common.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 common.c
--- usr.sbin/cron/common.c 11 Nov 2015 17:02:22 - 1.4
+++ usr.sbin/cron/common.c 11 Nov 2015 22:32:43 -
@@ -112,6 +112,7 @@ set_cron_cwd(void)
exit(EXIT_FAILURE);
}
if (grp != NULL) {
+ setegid(grp->gr_gid);
if (sb.st_gid != grp->gr_gid)
chown(AT_SPOOL, -1, grp->gr_gid);
if ((sb.st_mode & ALLPERMS) != 01770)
Index: usr.sbin/cron/cron.c
===
RCS file: /cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.68
diff -u -p -u -r1.68 cron.c
--- usr.sbin/cron/cron.c11 Nov 2015 17:19:22 - 1.68
+++ usr.sbin/cron/cron.c11 Nov 2015 22:32:43 -
@@ -431,9 +431,9 @@ open_socket(void)
exit(EXIT_FAILURE);
}
bzero(&s_un, sizeof(s_un));
- if (snprintf(s_un.sun_path, sizeof(s_un.sun_path), "%s/%s",
- CRON_SPOOL, CRONSOCK) >= sizeof(s_un.sun_path)) {
- fprintf(stderr, "%s/%s: path too long\n", CRON_SPOOL, CRONSOCK);
+ if (strlcpy(s_un.sun_path, CRONSOCK, sizeof(s_un.sun_path))
+ >= sizeof(s_un.sun_path)) {
+ fprintf(stderr, "%s: path too long\n", CRONSOCK);
log_it("CRON", "DEATH", "path too long");
exit(EXIT_FAILURE);
}
@@ -463,6 +463,7 @@ open_socket(void)
exit(EXIT_FAILURE);
}
chmod(s_un.sun_path, 0660);
+ chown(s_un.sun_path, -1, getegid());
return(sock);
}
Index: usr.sbin/cron/pathnames.h
===
RCS file: /cvs/src/usr.sbin/cron/pathnames.h,v
retrieving revision 1.20
diff -u -p -u -r1.20 pathnames.h
--- usr.sbin/cron/pathnames.h 9 Nov 2015 16:00:39 - 1.20
+++ usr.sbin/cron/pathnames.h 11 Nov 2015 22:32:43 -
@@ -50,9 +50,9 @@
/* CRONSOCK is the name of the socket used by at and
* crontab to poke cron to re-read the at and cron
* spool files while cron is asleep.
-* It lives in the spool directory.
*/
-#defineCRONSOCK".sock"
+#defineCRONSOCK"/var/run/cron.sock"
+#define
Re: move cron socket to /var/run/cron.sock
On Wed, 11 Nov 2015 14:43:47 -0700, "Todd C. Miller" wrote:
> There's limited backward compatibility so you can run a new crontab
> with an older cron daemon.
Revised diff, I neglected to send out the cron.c changes in the
first one.
- todd
Index: usr.sbin/cron/client.c
===
RCS file: /cvs/src/usr.sbin/cron/client.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 client.c
--- usr.sbin/cron/client.c 11 Nov 2015 17:05:23 - 1.6
+++ usr.sbin/cron/client.c 11 Nov 2015 21:56:18 -
@@ -19,9 +19,12 @@
#include
#include
+#include
#include
#include /* for structs.h */
+#include
+#include
#include
#include
#include
@@ -92,13 +95,17 @@ void
poke_daemon(const char *spool_dir, unsigned char cookie)
{
int sock = -1;
+ const char *cronsock = CRONSOCK;
+ struct stat sb;
struct sockaddr_un s_un;
+ if (stat(cronsock, &sb) != 0)
+ cronsock = CRONSOCK_OLD;/* backwards compatibility */
+
bzero(&s_un, sizeof(s_un));
- if (snprintf(s_un.sun_path, sizeof s_un.sun_path, "%s/%s",
- CRON_SPOOL, CRONSOCK) >= sizeof(s_un.sun_path)) {
- fprintf(stderr, "%s: %s/%s: path too long\n",
- __progname, CRON_SPOOL, CRONSOCK);
+ if (strlcpy(s_un.sun_path, cronsock, sizeof(s_un.sun_path)) >=
+ sizeof(s_un.sun_path)) {
+ warnc(ENAMETOOLONG, "%s", cronsock);
return;
}
s_un.sun_family = AF_UNIX;
@@ -106,8 +113,7 @@ poke_daemon(const char *spool_dir, unsig
connect(sock, (struct sockaddr *)&s_un, sizeof(s_un)) == 0)
send(sock, &cookie, 1, MSG_NOSIGNAL);
else
- fprintf(stderr, "%s: warning, cron does not appear to be "
- "running.\n", __progname);
+ warnx("warning, cron does not appear to be running");
if (sock >= 0)
close(sock);
}
Index: usr.sbin/cron/cron.c
===
RCS file: /cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.68
diff -u -p -u -r1.68 cron.c
--- usr.sbin/cron/cron.c11 Nov 2015 17:19:22 - 1.68
+++ usr.sbin/cron/cron.c11 Nov 2015 21:56:18 -
@@ -431,9 +431,9 @@ open_socket(void)
exit(EXIT_FAILURE);
}
bzero(&s_un, sizeof(s_un));
- if (snprintf(s_un.sun_path, sizeof(s_un.sun_path), "%s/%s",
- CRON_SPOOL, CRONSOCK) >= sizeof(s_un.sun_path)) {
- fprintf(stderr, "%s/%s: path too long\n", CRON_SPOOL, CRONSOCK);
+ if (strlcpy(s_un.sun_path, CRONSOCK, sizeof(s_un.sun_path))
+ >= sizeof(s_un.sun_path)) {
+ fprintf(stderr, "%s: path too long\n", CRONSOCK);
log_it("CRON", "DEATH", "path too long");
exit(EXIT_FAILURE);
}
Index: usr.sbin/cron/pathnames.h
===
RCS file: /cvs/src/usr.sbin/cron/pathnames.h,v
retrieving revision 1.20
diff -u -p -u -r1.20 pathnames.h
--- usr.sbin/cron/pathnames.h 9 Nov 2015 16:00:39 - 1.20
+++ usr.sbin/cron/pathnames.h 11 Nov 2015 21:56:18 -
@@ -50,9 +50,9 @@
/* CRONSOCK is the name of the socket used by at and
* crontab to poke cron to re-read the at and cron
* spool files while cron is asleep.
-* It lives in the spool directory.
*/
-#defineCRONSOCK".sock"
+#defineCRONSOCK"/var/run/cron.sock"
+#defineCRONSOCK_OLDCRON_SPOOL "/.sock"
/* cron allow/deny file. At least cron.deny must
* exist for ordinary users to run crontab.
move cron socket to /var/run/cron.sock
There's limited backward compatibility so you can run a new crontab
with an older cron daemon.
- todd
Index: usr.sbin/cron/client.c
===
RCS file: /cvs/src/usr.sbin/cron/client.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 client.c
--- usr.sbin/cron/client.c 11 Nov 2015 17:05:23 - 1.6
+++ usr.sbin/cron/client.c 11 Nov 2015 21:42:26 -
@@ -19,9 +19,12 @@
#include
#include
+#include
#include
#include /* for structs.h */
+#include
+#include
#include
#include
#include
@@ -92,13 +95,17 @@ void
poke_daemon(const char *spool_dir, unsigned char cookie)
{
int sock = -1;
+ const char *cronsock = CRONSOCK;
+ struct stat sb;
struct sockaddr_un s_un;
+ if (stat(cronsock, &sb) != 0)
+ cronsock = CRONSOCK_OLD;/* backwards compatibility */
+
bzero(&s_un, sizeof(s_un));
- if (snprintf(s_un.sun_path, sizeof s_un.sun_path, "%s/%s",
- CRON_SPOOL, CRONSOCK) >= sizeof(s_un.sun_path)) {
- fprintf(stderr, "%s: %s/%s: path too long\n",
- __progname, CRON_SPOOL, CRONSOCK);
+ if (strlcpy(s_un.sun_path, cronsock, sizeof(s_un.sun_path)) >=
+ sizeof(s_un.sun_path)) {
+ warnc(ENAMETOOLONG, "%s", cronsock);
return;
}
s_un.sun_family = AF_UNIX;
@@ -106,8 +113,7 @@ poke_daemon(const char *spool_dir, unsig
connect(sock, (struct sockaddr *)&s_un, sizeof(s_un)) == 0)
send(sock, &cookie, 1, MSG_NOSIGNAL);
else
- fprintf(stderr, "%s: warning, cron does not appear to be "
- "running.\n", __progname);
+ warnx("warning, cron does not appear to be running");
if (sock >= 0)
close(sock);
}
Index: usr.sbin/cron/pathnames.h
===
RCS file: /cvs/src/usr.sbin/cron/pathnames.h,v
retrieving revision 1.20
diff -u -p -u -r1.20 pathnames.h
--- usr.sbin/cron/pathnames.h 9 Nov 2015 16:00:39 - 1.20
+++ usr.sbin/cron/pathnames.h 11 Nov 2015 21:42:26 -
@@ -50,9 +50,9 @@
/* CRONSOCK is the name of the socket used by at and
* crontab to poke cron to re-read the at and cron
* spool files while cron is asleep.
-* It lives in the spool directory.
*/
-#defineCRONSOCK".sock"
+#defineCRONSOCK"/var/run/cron.sock"
+#defineCRONSOCK_OLDCRON_SPOOL "/.sock"
/* cron allow/deny file. At least cron.deny must
* exist for ordinary users to run crontab.
