On Tue, Oct 04, 2022 at 09:24:04AM +0000, Klemens Nanni wrote: > On Mon, Oct 03, 2022 at 06:43:26PM -0600, Theo de Raadt wrote: > > David Gwynne <da...@gwynne.id.au> wrote: > > > > > On Sun, Oct 02, 2022 at 06:32:04PM +0000, Klemens Nanni wrote: > > > > diskless(8) just needs tftpd(8) to deliver files, none of the possibly > > > > untrusted clients are supposed to ever write anything. > > > > > > > > Either way, even when run without -c, a single file writable by _tftpd > > > > might be enough for a malicious client to fill up the server's disk. > > > > > > > > A proper read-only mode ("stdio rpath dns inet") seems much safer. > > > > > > agreed. i'm ok with this diff, but it's worth asking if we can make the > > > default read-only and ask people to opt in for write (and create) before > > > this specific diff goes in. ie, read-only be default, '-w' to enable > > > write mode, '-c' to enable write+create? > > > > we were read-only believers a long time ago, and it seems the world has > > caught up to our way of thinking so yes maybe it is time to make it an > > option you must specify. > > I like the idea, then -c should logically imply -w. > > Feedback? OK?
Now with the important manual bits explaining the new defaul.t Index: tftpd.8 =================================================================== RCS file: /cvs/src/usr.sbin/tftpd/tftpd.8,v retrieving revision 1.8 diff -u -p -r1.8 tftpd.8 --- tftpd.8 4 Mar 2019 01:06:03 -0000 1.8 +++ tftpd.8 4 Oct 2022 09:42:16 -0000 @@ -37,7 +37,7 @@ .Nd Trivial File Transfer Protocol daemon .Sh SYNOPSIS .Nm tftpd -.Op Fl 46cdiv +.Op Fl 46cdivw .Op Fl l Ar address .Op Fl p Ar port .Op Fl r Ar socket @@ -53,11 +53,13 @@ does not require an account or password Due to the lack of authentication information, .Nm will allow only publicly readable files to be accessed. +By default files may only be read, unless the +.Fl w +option is specified. Files may be written only if they already exist and are publicly writable, unless the .Fl c -flag is specified -.Pq see below . +flag is specified. Note that this extends the concept of .Dq public to include @@ -93,6 +95,9 @@ Allow new files to be created; otherwise uploaded files must already exist. Files are created with default permissions allowing anyone to read or write to them. +.Pp +This option implies +.Fl w . .It Fl d Do not daemonize. If this option is specified, @@ -145,6 +150,8 @@ to on startup; the remote host is not expected to pass the directory as part of the file name to transfer. +.It Fl w +Allow files to be written to. .El .Sh SEE ALSO .Xr tftp 1 , Index: tftpd.c =================================================================== RCS file: /cvs/src/usr.sbin/tftpd/tftpd.c,v retrieving revision 1.48 diff -u -p -r1.48 tftpd.c --- tftpd.c 4 Oct 2022 07:05:28 -0000 1.48 +++ tftpd.c 4 Oct 2022 09:42:26 -0000 @@ -283,12 +283,13 @@ __dead void usage(void) { extern char *__progname; - fprintf(stderr, "usage: %s [-46cdiv] [-l address] [-p port] [-r socket]" + fprintf(stderr, "usage: %s [-46cdivw] [-l address] [-p port] [-r socket]" " directory\n", __progname); exit(1); } int cancreate = 0; +int canwrite = 0; int verbose = 0; int debug = 0; int iflag = 0; @@ -309,7 +310,7 @@ main(int argc, char *argv[]) int family = AF_UNSPEC; int devnull = -1; - while ((c = getopt(argc, argv, "46cdil:p:r:v")) != -1) { + while ((c = getopt(argc, argv, "46cdil:p:r:vw")) != -1) { switch (c) { case '4': family = AF_INET; @@ -318,7 +319,7 @@ main(int argc, char *argv[]) family = AF_INET6; break; case 'c': - cancreate = 1; + canwrite = cancreate = 1; break; case 'd': verbose = debug = 1; @@ -342,6 +343,9 @@ main(int argc, char *argv[]) case 'v': verbose = 1; break; + case 'w': + canwrite = 1; + break; default: usage(); /* NOTREACHED */ @@ -394,9 +398,12 @@ main(int argc, char *argv[]) if (cancreate) { if (pledge("stdio rpath wpath cpath fattr dns inet", NULL) == -1) lerr(1, "pledge"); - } else { + } else if (canwrite) { if (pledge("stdio rpath wpath fattr dns inet", NULL) == -1) lerr(1, "pledge"); + } else { + if (pledge("stdio rpath dns inet", NULL) == -1) + lerr(1, "pledge"); } event_init(); @@ -969,6 +976,9 @@ validate_access(struct tftp_client *clie int fd, wmode; const char *errstr, *filename; char rewritten[PATH_MAX]; + + if (!canwrite && mode != RRQ) + return (EACCESS); if (strcmp(requested, SEEDPATH) == 0) { char *buf;