* in the configure script) */
char *user = NULL;
char *runas = NULL;
-uid_t uid = 0;
-gid_t gid = 0;
-uid_t asuid = 0;
-gid_t asgid = 0;
-uid_t fcrontab_uid = 0;
-gid_t fcrontab_gid = 0;
-uid_t rootuid = 0;
-gid_t rootgid = 0;
+uid_t useruid = 0; /* uid of the user */
+gid_t usergid = 0; /* gid of the user */
+uid_t asuid = 0; /* uid of the user whose fcrontab we are working on */
+gid_t asgid = 0; /* gid of the user whose fcrontab we are working on*/
+uid_t fcrontab_uid = 0; /* uid of the fcron user */
+gid_t fcrontab_gid = 0; /* gid of the fcron user */
+uid_t rootuid = 0; /* uid of root */
+gid_t rootgid = 0; /* gid of root */
char need_sig = 0; /* do we need to signal fcron daemon */
{
pid_t pid = 0;
+ /* WARNING: make sure we never call die_e() or something that could call
+ * die_e() here, as die_e() would then call xexit() and we could
+ * go into a loop! */
+
if ( need_sig == 1 ) {
/* fork and exec fcronsighup */
switch ( pid = fork() ) {
case 0:
/* child */
+ if (getegid() != fcrontab_gid && setegid(fcrontab_gid) != 0) {
+ error_e("could not change egid to fcrontab_gid[%d]", fcrontab_gid);
+ exit(ERR);
+ }
execl(BINDIREX "/fcronsighup", BINDIREX "/fcronsighup",
fcronconf, NULL);
- die_e("Could not exec " BINDIREX " fcronsighup");
+
+ error_e("Could not exec " BINDIREX " fcronsighup");
+ exit(ERR);
break;
case -1:
- die_e("Could not fork (fcron has not been signaled)");
+ error_e("Could not fork (fcron has not been signaled)");
+ exit(ERR);
break;
default:
}
#ifdef HAVE_LIBPAM
+ /* we need those rights for pam to close properly */
+ if (geteuid() != fcrontab_uid && seteuid(fcrontab_uid) != 0)
+ die_e("could not change euid to %d", fcrontab_uid);
+ if (getegid() != fcrontab_gid && setegid(fcrontab_gid) != 0)
+ die_e("could not change egid to %d", fcrontab_gid);
pam_setcred(pamh, PAM_DELETE_CRED | PAM_SILENT);
pam_end(pamh, pam_close_session(pamh, PAM_SILENT));
#endif
}
-int
-open_as_user(const char *pathname, int flags, uid_t openuid, gid_t opengid)
-/* Become user and call open(), then revert back to who we were */
-{
- uid_t orig_euid = geteuid();
- gid_t orig_egid = getegid();
- int fd = -1;
-
- if (seteuid(openuid) < 0)
- die_e("open_as_user(): could not change euid to %d", uid);
- if (setegid(opengid) < 0)
- die_e("open_as_user(): could not change egid to %d", gid);
-
- fd = open(pathname, flags);
-
- /* change the effective uid/gid back to original values */
- if (seteuid(orig_euid) < 0)
- die_e("open_as_user(): could not revert to euid %d", orig_euid);
- if (setegid(orig_egid) < 0)
- die_e("open_as_user(): could not revert to egid %d", orig_egid);
-
- return fd;
-
-}
-
int
copy_src(int from, char *dest)
/* copy src file orig (already opened) to dest */
/* just in case the file was read in the past... */
lseek(from, 0, SEEK_SET);
- /* create it as fcrontab_uid (to avoid problem if user's uid changed)
- * except for root. Root requires filesystem uid root for security
- * reasons */
-#ifdef USE_SETE_ID
- if (asuid == rootuid) {
- if (seteuid(rootuid) != 0)
- die_e("seteuid(rootuid) : old source file kept");
- }
- else {
- if (seteuid(fcrontab_uid) != 0)
- error_e("seteuid(fcrontab_uid[%d])", fcrontab_uid);
- }
-#endif
-
- /* the temp file must be in the same directory as the dest file */
+ /* the temp file must be in the same directory as the dest file */
dest_path_len = strlen(dest);
strncpy(tmp_filename_str, dest, max_dest_len);
tmp_filename_index = (dest_path_len > max_dest_len) ?
max_dest_len : dest_path_len;
strcpy(&tmp_filename_str[tmp_filename_index], tmp_suffix_str);
- to_fd = open(tmp_filename_str, O_WRONLY | O_CREAT | O_TRUNC | O_SYNC,
- S_IRUSR|S_IWUSR|S_IRGRP);
+ /* create it as fcrontab_uid (to avoid problem if user's uid changed)
+ * except for root. Root requires filesystem uid root for security
+ * reasons */
+ to_fd = open_as_user(tmp_filename_str, (asuid==rootuid)?rootuid:fcrontab_uid,
+ fcrontab_gid, O_WRONLY|O_CREAT|O_TRUNC|O_SYNC, S_IRUSR|S_IWUSR|S_IRGRP);
if ( to_fd < 0 ) {
error_e("could not open file %s", tmp_filename_str);
goto exiterr;
}
-#ifdef USE_SETE_ID
- if (asuid != rootuid && seteuid(uid) != 0)
- die_e("seteuid(uid[%d]) : old source file kept", uid);
-#endif
if (asuid == rootuid ) {
if ( fchmod(to_fd, S_IWUSR | S_IRUSR) != 0 ) {
error_e("Could not fchmod %s to 600", tmp_filename_str);
close(to_fd);
to_fd = -1;
- if ( rename(tmp_filename_str, dest) < 0 ) {
+ if ( rename_as_user(tmp_filename_str, dest, useruid, fcrontab_gid) < 0 ) {
error_e("Unable to rename %s to %s : old source file kept",
tmp_filename_str, dest);
goto exiterr;
explain("removing %s's fcrontab", user);
/* remove source and formated file */
- if ( (rm_orig && remove(buf)) != 0 ) {
+ if ( (rm_orig && remove_as_user(buf, fcrontab_uid, fcrontab_gid)) != 0 ) {
if ( errno == ENOENT )
return_val = ENOENT;
else
error_e("could not remove %s", buf);
}
-#ifdef USE_SETE_ID
- if (seteuid(fcrontab_uid) != 0)
- error_e("seteuid(fcrontab_uid[%d])", fcrontab_uid);
-#endif
-
/* try to remove the temp file in case he has not
* been read by fcron daemon */
snprintf(buf, sizeof(buf), "new.%s", user);
- remove(buf);
+ remove_as_user(buf, useruid, fcrontab_gid);
/* finally create a file in order to tell the daemon
* a file was removed, and launch a signal to daemon */
snprintf(buf, sizeof(buf), "rm.%s", user);
- fd = open(buf, O_CREAT | O_TRUNC | O_EXCL, S_IRUSR | S_IWUSR);
-
-#ifdef USE_SETE_ID
- if (seteuid(uid) != 0)
- die_e("seteuid(uid[%d])", uid);
-#endif
+ fd = open_as_user(buf, fcrontab_uid, fcrontab_gid, O_CREAT|O_TRUNC|O_EXCL, S_IRUSR|S_IWUSR);
if ( fd == -1 ) {
if ( errno != EEXIST )
{
FILE *f = NULL;
int c;
+ int fd = -1;
explain("listing %s's fcrontab", user);
- if ( (f = fopen(file, "r")) == NULL ) {
- if ( errno == ENOENT ) {
+ fd = open_as_user(file, useruid, fcrontab_uid, O_RDONLY);
+ if ( fd < 0 ) {
+ if ( errno == ENOENT ) {
explain("user %s has no fcrontab.", user);
return ;
}
else
die_e("User %s could not read file \"%s\"", user, file);
}
- else {
- while ( (c = getc(f)) != EOF )
- putchar(c);
+ f = fdopen(fd, "r");
+ if ( f == NULL ) {
+ close(fd);
+ die_e("User %s could not read file \"%s\"", user, file);
+ }
- fclose(f);
+ while ( (c = getc(f)) != EOF )
+ putchar(c);
- }
+ fclose(f);
}
time_t mtime = 0;
char *tmp_str;
FILE *f = NULL, *fi = NULL;
- int file = -1;
+ int file = -1, origfd = -1;
int c;
char correction = 0;
short return_val = EXIT_OK;
}
#endif
/* copy user's fcrontab (if any) to a temp file */
- if ( (f = fopen(fcron_orig, "r")) == NULL ) {
+ origfd = open_as_user(fcron_orig, useruid, fcrontab_gid, O_RDONLY);
+ if ( origfd < 0 ) {
if ( errno != ENOENT ) {
error_e("could not open file %s", fcron_orig);
goto exiterr;
user);
}
else {
+ f = fdopen(origfd, "r");
+ if ( f == NULL ) {
+ error_e("could not fdopen file %s", fcron_orig);
+ goto exiterr;
+ }
/* copy original file to temp file */
while ( (c=getc(f)) != EOF ) {
if ( putc(c, fi) == EOF ) {
goto exiterr;
}
+ /* close the file before the user edits it */
+ close(file);
+
switch ( pid = fork() ) {
case 0:
/* child */
- if ( uid != rootuid ) {
+ if ( useruid != rootuid ) {
if (setgid(asgid) < 0) {
error_e("setgid(asgid)");
goto exiterr;
goto exiterr;
}
+ /* re-open the file that has just been edited */
+ file = open_as_user(tmp_str, useruid, usergid, O_RDONLY);
+ if ( file < 0 ) {
+ error_e("Could not open file %s", tmp_str);
+ goto exiterr;
+ }
+
#ifndef USE_SETE_ID
/* chown the file back to rootuid/rootgid */
if ( fchown(file, rootuid, rootgid) != 0 || fchmod(file, S_IRUSR|S_IWUSR) != 0 ){
end:
if ( file != -1 && close(file) != 0 )
error_e("could not close %s", tmp_str);
- if ( remove(tmp_str) != 0 )
+ if ( remove_as_user(tmp_str, useruid, fcrontab_gid) != 0 )
error_e("could not remove %s", tmp_str);
free(tmp_str);
xexit (return_val);
exiterr:
- if ( remove(tmp_str) != 0 )
+ if ( remove_as_user(tmp_str, useruid, fcrontab_gid) != 0 )
error_e("could not remove %s", tmp_str);
free(tmp_str);
if ( f != NULL )
explain("reinstalling %s's fcrontab", user);
- if ( (i = open(fcron_orig, O_RDONLY)) < 0) {
+ if ( (i = open_as_user(fcron_orig, useruid, fcrontab_gid, O_RDONLY)) < 0) {
if ( errno == ENOENT ) {
fprintf(stderr, "Could not reinstall: user %s has no fcrontab\n",
user);
usage(); break;
case 'u':
- if (uid != rootuid) {
+ if (useruid != rootuid) {
fprintf(stderr, "must be privileged to use -u\n");
xexit(EXIT_ERR);
}
else
usage();
- if (uid != rootuid) {
+ if (useruid != rootuid) {
fprintf(stderr, "must be privileged to use -u\n");
xexit(EXIT_ERR);
}
if ( list_opt + rm_opt + edit_opt + reinstall_opt == 0 )
file_opt = optind;
else {
- if (uid != rootuid) {
+ if (useruid != rootuid) {
fprintf(stderr, "must be privileged to use [user|-u user]\n");
xexit(EXIT_ERR);
}
if ( user == NULL ) {
/* get user's name using getpwuid() */
- if ( ! (pass = getpwuid(uid)) )
+ if ( ! (pass = getpwuid(useruid)) )
die_e("user \"%s\" is not in passwd file. Aborting.", USERNAME);
/* we need to strdup2 the value given by getpwuid() because we free
* file->cf_user in delete_file */
if (strrchr(argv[0],'/')==NULL) prog_name = argv[0];
else prog_name = strrchr(argv[0],'/')+1;
- uid = getuid();
- gid = getgid();
+ useruid = getuid();
+ usergid = getgid();
+
+#ifdef USE_SETE_ID
+ /* drop any suid privilege (that we use to write files) but keep sgid
+ * one for now: we need it for read_conf() and is_allowed() */
+ seteuid_safe(useruid);
+#endif
errno = 0;
if ( ! (pass = getpwnam(USERNAME)) )
fcrontab_gid = pass->pw_gid;
/* get current dir */
-#ifdef USE_SETE_ID
- if (seteuid(uid) != 0)
- die_e("Could not change euid to %d", uid);
-#endif
+ orig_dir[0] = '\0';
if ( getcwd(orig_dir, sizeof(orig_dir)) == NULL )
die_e("getcwd");
-#ifdef USE_SETE_ID
- if (seteuid(fcrontab_uid) != 0)
- die_e("Couldn't change euid to fcrontab_uid[%d]",fcrontab_uid);
-#endif
/* interpret command line options */
parseopt(argc, argv);
#ifdef USE_SETE_ID
+ /* drop any privilege we may have: we will only get them back
+ * temporarily every time we need it. */
+ seteuid_safe(useruid);
+ setegid_safe(usergid);
+#endif
#ifdef HAVE_LIBPAM
/* Open PAM session for the user and obtain any security
credentials we might need */
- /* FIXME: remove some #ifdef //////////////////////// */
- /* FIXME: should really uid=euid when calling PAM ? */
-#ifdef USE_SETE_ID
- if (seteuid(uid) != 0)
- die_e("Could not change euid to %d", uid);
-#endif
debug("username: %s", user);
retcode = pam_start("fcrontab", user, &apamconv, &pamh);
if (retcode != PAM_SUCCESS) die_pame(pamh, retcode, "Could not start PAM");
/* Close the log here, because PAM calls openlog(3) and
our log messages could go to the wrong facility */
xcloselog();
- /* FIXME: remove some #ifdef //////////////////////// */
- /* FIXME: should really uid=euid when calling PAM ? */
-#ifdef USE_SETE_ID
- if (seteuid(fcrontab_uid) != 0)
- die_e("Couldn't change euid to fcrontab_uid[%d]",fcrontab_uid);
-#endif
#endif /* USE_PAM */
- if (uid != fcrontab_uid)
- if (seteuid(fcrontab_uid) != 0)
- die_e("Couldn't change euid to fcrontab_uid[%d]",fcrontab_uid);
+#ifdef USE_SETE_ID
+ seteuid_safe(fcrontab_uid);
/* change directory */
if (chdir(fcrontabs) != 0) {
error_e("Could not chdir to %s", fcrontabs);
xexit (EXIT_ERR);
}
- /* get user's permissions */
- if (seteuid(uid) != 0)
- die_e("Could not change euid to %d", uid);
- if (setegid(fcrontab_gid) != 0)
- die_e("Could not change egid to " GROUPNAME "[%d]", fcrontab_gid);
-
+ seteuid_safe(useruid);
#else /* USE_SETE_ID */
if (setuid(rootuid) != 0 )
file[sizeof(file)-1] = '\0';
}
- fd = open_as_user(file, O_RDONLY, uid, gid);
+ fd = open_as_user(file, useruid, usergid, O_RDONLY);
if (fd < 0)
die_e("Could not open file %s", file);
if (make_file(file, fd) == OK)
}
+#ifdef USE_SETE_ID
+void
+seteuid_safe(uid_t euid)
+/* set the euid if different from the current one, and die on error */
+{
+ /* on BSD, one can only seteuid() to the real UID or saved UID,
+ * and NOT the euid, so we don't call seteuid(geteuid()),
+ * which is why we need to check if a change is needed */
+
+ if (geteuid() != euid && seteuid(euid) != 0)
+ die_e("could not change euid to %d", euid);
+
+}
+
+void
+setegid_safe(gid_t egid)
+/* set the egid if different from the current one, and die on error */
+{
+ /* on BSD, one can only setegid() to the real GID or saved GID,
+ * and NOT the egid, so we don't call setegid(getegid()),
+ * which is why we need to check if a change is needed */
+
+ if (getegid() != egid && setegid(egid) != 0)
+ die_e("could not change egid to %d", egid);
+
+}
+#endif /* def USE_SETE_ID */
+
+#ifdef USE_SETE_ID
+int
+open_as_user(const char *pathname, uid_t openuid, gid_t opengid, int flags, ...)
+/* Become user and call open(), then revert back to who we were.
+ * NOTE: when flags & O_CREAT, the 5th argument is mode_t and must be set
+ * -- it is ignored otherwise */
+{
+ uid_t orig_euid = geteuid();
+ gid_t orig_egid = getegid();
+ struct stat s;
+ int fd = -1;
+ va_list ap;
+ mode_t mode = (mode_t) 0;
+
+ if (flags & O_CREAT) {
+ va_start(ap, flags);
+ mode = va_arg(ap, mode_t);
+ va_end(ap);
+ }
+
+ seteuid_safe(openuid);
+ setegid_safe(opengid);
+
+ if (flags & O_CREAT) {
+ fd = open(pathname, flags, mode);
+ }
+ else
+ fd = open(pathname, flags);
+
+ /* change the effective uid/gid back to original values */
+ seteuid_safe(orig_euid);
+ setegid_safe(orig_egid);
+
+ /* if open() didn't fail make sure we opened a 'normal' file */
+ if ( fd >= 0 ) {
+
+ if ( fstat(fd, &s) < 0 ) {
+ error_e("open_as_user(): could not fstat %s", pathname);
+ if ( close(fd) < 0 )
+ error_e("open_as_user: could not close() %s", pathname);
+ fd = -1;
+ }
+
+ if ( ! S_ISREG(s.st_mode) || s.st_nlink != 1 ) {
+ error_e("open_as_user(): file %s is not a regular file", pathname);
+ if ( close(fd) < 0 )
+ error_e("open_as_user: could not close() %s", pathname);
+ errno = 0;
+ fd = -1;
+ }
+
+ }
+
+ return fd;
+
+}
+
+#else /* def USE_SETE_ID */
+
+int
+open_as_user(const char *pathname, uid_t openuid, gid_t opengid, int flags, ...)
+/* Become user and call open(), then revert back to who we were.
+ * As seteuid() is not available on this system attempt to similate that behavior
+ * as closely as possible.
+ * NOTE: when flags & O_CREAT, the 5th argument is mode_t and must be set
+ * -- it is ignored otherwise */
+{
+ int fd = -1;
+ struct stat s;
+ va_list ap;
+ mode_t mode = (mode_t) 0;
+
+ if (flags & O_CREAT) {
+ va_start(ap, flags);
+ mode = va_arg(ap, mode_t);
+ va_end(ap);
+ }
+
+ /* In case a flag as O_TRUNC is set, we should test if the user
+ * is allowed to open the file before we open it.
+ * There will always be a risk of race-condition between the test
+ * and the open but that's the best we can realistically do
+ * without seteuid()... */
+ stat(pathname, &s);
+ if ( ! ( s.st_mode & S_IROTH
+ || ( s.st_uid == openuid && s.st_mode & S_IRUSR )
+ || ( s.st_gid == opengid && s.st_mode & S_IRGRP ) ) ) {
+ errno = EACCES;
+ return -1;
+ }
+
+ if (flags & O_CREAT) {
+ fd = open(pathname, flags, mode);
+ }
+ else
+ fd = open(pathname, flags);
+
+ if ( fd < 0 )
+ /* we couldn't open the file */
+ return fd;
+
+ /* if open() didn't fail make sure we opened a 'normal' file */
+ if ( fstat(fd, &s) < 0 ) {
+ error_e("open_as_user(): could not fstat %s", pathname);
+ goto err;
+ }
+ if ( ! S_ISREG(s.st_mode) || s.st_nlink != 1 ) {
+ error_e("open_as_user(): file %s is not a regular file", pathname);
+ goto err;
+ }
+
+ /* we couldn't become openuid/opengid, so check manually if the user
+ * is allowed to read that file
+ * We do that again as a malicious user could have replaced the file
+ * by another one (e.g. a link) between the stat() and the open() earlier */
+ if ( ! ( s.st_mode & S_IROTH
+ || ( s.st_uid == openuid && s.st_mode & S_IRUSR )
+ || ( s.st_gid == opengid && s.st_mode & S_IRGRP ) ) ) {
+ error_e("open_as_user(): file %s does not pass the security test", pathname);
+ errno = EACCES;
+ goto err;
+ }
+
+ /* if we created a new file, change the file ownership:
+ * make it as it would be if we had seteuid()
+ * NOTE: if O_CREAT was set without O_EXCL and the file existed before
+ * then we will end up changing the ownership even if the seteuid()
+ * version of that function wouldn't have. That shouldn't break
+ * anything though. */
+ if ( (flags & O_CREAT) && fchown(fd, openuid, opengid) != 0) {
+ error_e("Could not fchown %s to uid:%d gid:%d", pathname, openuid, opengid);
+ if ( close(fd) < 0 )
+ error_e("open_as_user: could not close() %s", pathname);
+ return -1;
+ }
+
+ /* everything went ok: return the file descriptor */
+ return fd;
+
+err:
+ if ( fd >= 0 && close(fd) < 0 )
+ error_e("open_as_user: could not close() %s", pathname);
+ return = -1;
+}
+
+#endif /* def USE_SETE_ID */
+
+int
+remove_as_user(const char *pathname, uid_t removeuid, gid_t removegid)
+/* Become user and call remove(), then revert back to who we were */
+{
+ uid_t orig_euid = geteuid();
+ gid_t orig_egid = getegid();
+ int rval = -1;
+
+#ifdef USE_SETE_ID
+ seteuid_safe(removeuid);
+ setegid_safe(removegid);
+#endif /* def USE_SETE_ID */
+
+ rval = remove(pathname);
+
+#ifdef USE_SETE_ID
+ seteuid_safe(orig_euid);
+ setegid_safe(orig_egid);
+#endif /* def USE_SETE_ID */
+
+ return rval;
+}
+
+int
+rename_as_user(const char *oldpath, const char *newpath, uid_t renameuid, gid_t renamegid)
+/* Become user and call rename(), then revert back to who we were */
+{
+ uid_t orig_euid = geteuid();
+ gid_t orig_egid = getegid();
+ int rval = -1;
+
+#ifdef USE_SETE_ID
+ seteuid_safe(renameuid);
+ setegid_safe(renamegid);
+#endif /* def USE_SETE_ID */
+
+ rval = rename(oldpath, newpath);
+
+#ifdef USE_SETE_ID
+ seteuid_safe(orig_euid);
+ setegid_safe(orig_egid);
+#endif /* def USE_SETE_ID */
+
+ return rval;
+
+}
+
int
remove_blanks(char *str)
/* remove blanks at the the end of str */
init_conf();
- if ( (f = fopen(fcronconf, "r")) == NULL ) {
- if ( errno == ENOENT ) {
+ f = fopen(fcronconf, "r");
+ if ( f == NULL ) {
+ if ( errno == ENOENT ) {
if ( err_on_enoent )
die_e("Could not read %s", fcronconf);
else
projects / fcron / commitdiff