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;
