Page MenuHomeFreeBSD
Differential D23022 Diff 66304 usr.sbin/inetd/builtins.c

Changeset View

Standalone View

View Options

usr.sbin/inetd/builtins.c

Show All 33 Lines
#include <sys/param.h> #include <sys/param.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/sysctl.h> #include <sys/sysctl.h>
#include <sys/ucred.h> #include <sys/ucred.h>
#include <sys/uio.h> #include <sys/uio.h>
#include <sys/utsname.h> #include <sys/utsname.h>
#include <capsicum_helpers.h>
#include <ctype.h> #include <ctype.h>
#include <err.h> #include <err.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <limits.h> #include <limits.h>
#include <pwd.h> #include <pwd.h>
#include <signal.h> #include <signal.h>
#include <stdlib.h> #include <stdlib.h>
Show All 20 Lines
static void machtime_dg(int, struct servtab *); static void machtime_dg(int, struct servtab *);
static void machtime_stream(int, struct servtab *); static void machtime_stream(int, struct servtab *);
static char ring[128]; static char ring[128];
static char *endring; static char *endring;
struct biltin biltins[] = { struct biltin biltins[] = {
/* Echo received data */ /* Echo received data */
{ "echo", SOCK_STREAM, 1, -1, echo_stream }, { "echo", SOCK_STREAM, true, -1, echo_stream, true },
markjUnsubmitted
Done
Inline Actions

I'd consider combining bi_fork and bi_capenter into a bi_flags variable. That'd make it easier to read the table.

markj: I'd consider combining bi_fork and bi_capenter into a bi_flags variable. That'd make it easier…
{ "echo", SOCK_DGRAM, 0, 1, echo_dg }, { "echo", SOCK_DGRAM, false, 1, echo_dg, false },
/* Internet /dev/null */ /* Internet /dev/null */
{ "discard", SOCK_STREAM, 1, -1, discard_stream }, { "discard", SOCK_STREAM, true, -1, discard_stream, true },
{ "discard", SOCK_DGRAM, 0, 1, discard_dg }, { "discard", SOCK_DGRAM, false, 1, discard_dg, false },
markjUnsubmitted
Not Done
Inline Actions

Why do we not support cap mode for any of the UDP services? Seems like discard_dg should be trivial.

markj: Why do we not support cap mode for any of the UDP services? Seems like discard_dg should be…
kevansAuthorUnsubmitted
Done
Inline Actions

It's less about it being a UDP service and more about not forking for it -- I generally declined to touch any that we currently don't fork off in the first iteration.

kevans: It's less about it being a UDP service and more about not forking for it -- I generally…
/* Return 32 bit time since 1900 */ /* Return 32 bit time since 1900 */
{ "time", SOCK_STREAM, 0, -1, machtime_stream }, { "time", SOCK_STREAM, false, -1, machtime_stream,
{ "time", SOCK_DGRAM, 0, 1, machtime_dg }, false },
{ "time", SOCK_DGRAM, false, 1, machtime_dg, false },
/* Return human-readable time */ /* Return human-readable time */
{ "daytime", SOCK_STREAM, 0, -1, daytime_stream }, { "daytime", SOCK_STREAM, false, -1, daytime_stream, false },
{ "daytime", SOCK_DGRAM, 0, 1, daytime_dg }, { "daytime", SOCK_DGRAM, false, 1, daytime_dg, false },
/* Familiar character generator */ /* Familiar character generator */
{ "chargen", SOCK_STREAM, 1, -1, chargen_stream }, { "chargen", SOCK_STREAM, true, -1, chargen_stream, true },
{ "chargen", SOCK_DGRAM, 0, 1, chargen_dg }, { "chargen", SOCK_DGRAM, false, 1, chargen_dg, false },
{ "tcpmux", SOCK_STREAM, 1, -1, (bi_fn_t *)tcpmux }, /*
* tcpmux cap_enter is effectively ignored; we explicitly check if the
* service is tcpmux, then we invoke it manually and proceed as if the
* service it resolved to was selected.
*/
{ "tcpmux", SOCK_STREAM, true, -1, (bi_fn_t *)tcpmux,
false },
{ "auth", SOCK_STREAM, 1, -1, ident_stream }, { "auth", SOCK_STREAM, true, -1, ident_stream, false },
{ NULL, 0, 0, 0, NULL } { NULL, 0, 0, 0, NULL, false }
}; };
/* /*
* RFC864 Character Generator Protocol. Generates character data without * RFC864 Character Generator Protocol. Generates character data without
* any regard for input. * any regard for input.
*/ */
static void static void
▲ Show 20 Lines • Show All 88 Lines • ▼ Show 20 Lines
static void static void
daytime_dg(int s, struct servtab *sep) daytime_dg(int s, struct servtab *sep)
{ {
char buffer[256]; char buffer[256];
time_t now; time_t now;
struct sockaddr_storage ss; struct sockaddr_storage ss;
socklen_t size; socklen_t size;
now = time((time_t *) 0); now = time((time_t *) 0);
oshogboUnsubmitted
Not Done
Inline Actions

Is any of those time can be run in the capability program?
If so does we need caph_cache_tzdata ?

oshogbo: Is any of those time can be run in the capability program? If so does we need caph_cache_tzdata…
kevansAuthorUnsubmitted
Done
Inline Actions

Nope- those ones are confined to builtins that don't fork

kevans: Nope- those ones are confined to builtins that don't fork
size = sizeof(ss); size = sizeof(ss);
if (recvfrom(s, buffer, sizeof(buffer), 0, if (recvfrom(s, buffer, sizeof(buffer), 0,
(struct sockaddr *)&ss, &size) < 0) (struct sockaddr *)&ss, &size) < 0)
return; return;
if (check_loop((struct sockaddr *)&ss, sep)) if (check_loop((struct sockaddr *)&ss, sep))
return; return;
▲ Show 20 Lines • Show All 401 Lines • ▼ Show 20 Lines if (fflag) {
* Therefore, we open the file we have permissions to open * Therefore, we open the file we have permissions to open
* and if it's not a regular file, we close it and end up * and if it's not a regular file, we close it and end up
* returning the user's real username. * returning the user's real username.
*/ */
if (asprintf(&p, "%s/.fakeid", pw->pw_dir) == -1) if (asprintf(&p, "%s/.fakeid", pw->pw_dir) == -1)
iderror(lport, fport, s, ID_UNKNOWN); iderror(lport, fport, s, ID_UNKNOWN);
fakeid_fd = open(p, O_RDONLY | O_NONBLOCK); fakeid_fd = open(p, O_RDONLY | O_NONBLOCK);
free(p); free(p);
if (caph_limit_stream(fakeid_fd, CAPH_READ) == -1)
goto fakeid_fail;
if (fakeid_fd == -1 || fstat(fakeid_fd, &sb) == -1 || if (fakeid_fd == -1 || fstat(fakeid_fd, &sb) == -1 ||
markjUnsubmitted
Done
Inline Actions

It reads a bit strangely to check fakeid_fd == -1 after calling caph_limit_stream(). You might consider extending the existing if-statement instead, i.e.

if (fakeid_fd == -1 ||
    caph_limit_stream(...) == -1 ||
    fstat(...) == -1 ||
    ...
markj: It reads a bit strangely to check fakeid_fd == -1 after calling caph_limit_stream(). You might…
!S_ISREG(sb.st_mode)) !S_ISREG(sb.st_mode))
goto fakeid_fail; goto fakeid_fail;
markjUnsubmitted
Not Done
Inline Actions

Why caph_limit_stream()? fakeid_fd isn't wrapped with a FILE. I'm not sure why it's useful to limit rights here at all (fakeid_fd doesn't get inherited by children?), but cap_rights_limit(fakeid_fd, CAP_READ) would make more sense.

markj: Why `caph_limit_stream()`? fakeid_fd isn't wrapped with a `FILE`. I'm not sure why it's useful…
kevansAuthorUnsubmitted
Done
Inline Actions

Fair point; dropped entirely

kevans: Fair point; dropped entirely
if ((ssize = read(fakeid_fd, buf, sizeof(buf) - 1)) < 0) if ((ssize = read(fakeid_fd, buf, sizeof(buf) - 1)) < 0)
goto fakeid_fail; goto fakeid_fail;
buf[ssize] = '\0'; buf[ssize] = '\0';
/* /*
* Usually, the file will have the desired identity * Usually, the file will have the desired identity
* in the form "identity\n". Allow for leading white * in the form "identity\n". Allow for leading white
▲ Show 20 Lines • Show All 185 Lines • Show Last 20 Lines