From: kn <kn@openbsd.org>
Date: Tue, 4 Oct 2022 23:33:22 +0000 (+0000)
Subject: Switch default to read-only, add -w for write access (previous default)
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a5c6d06ac9db4ed3b93605b6c59da2da7cf6a388;p=openbsd

Switch default to read-only, add -w for write access (previous default)

Write access seems less often required these days and other ways to ensure
effective read-only access are mere workarounds;  worst case malicious users
can fill up the server's disk by writing to existing files.

diskless(8) only ever needs to read and running with "stdio rpath dns inet"
by default is much safer for a network daemon without any authentication.

Initially proposed as a new -R flag for read-only mode
new default suggestion dlg deraadt
"looks great" millert
OK sthen dlg
---

diff --git a/usr.sbin/tftpd/tftpd.8 b/usr.sbin/tftpd/tftpd.8
index a3aab719550..b5bfd5cbf2a 100644
--- a/usr.sbin/tftpd/tftpd.8
+++ b/usr.sbin/tftpd/tftpd.8
@@ -1,4 +1,4 @@
-.\"   $OpenBSD: tftpd.8,v 1.8 2019/03/04 01:06:03 dlg Exp $
+.\"   $OpenBSD: tftpd.8,v 1.9 2022/10/04 23:33:22 kn Exp $
 .\"
 .\" Copyright (c) 1983, 1991 The Regents of the University of California.
 .\" All rights reserved.
@@ -29,7 +29,7 @@
 .\"
 .\"	from: @(#)tftpd.8	6.7 (Berkeley) 5/13/91
 .\"
-.Dd $Mdocdate: March 4 2019 $
+.Dd $Mdocdate: October 4 2022 $
 .Dt TFTPD 8
 .Os
 .Sh NAME
@@ -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 on the remote system.
 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 ,
diff --git a/usr.sbin/tftpd/tftpd.c b/usr.sbin/tftpd/tftpd.c
index 132e3b87985..049f6f20c05 100644
--- a/usr.sbin/tftpd/tftpd.c
+++ b/usr.sbin/tftpd/tftpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tftpd.c,v 1.48 2022/10/04 07:05:28 kn Exp $	*/
+/*	$OpenBSD: tftpd.c,v 1.49 2022/10/04 23:33:22 kn Exp $	*/
 
 /*
  * Copyright (c) 2012 David Gwynne <dlg@uq.edu.au>
@@ -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();
@@ -970,6 +977,9 @@ validate_access(struct tftp_client *client, const char *requested)
 	const char	*errstr, *filename;
 	char		 rewritten[PATH_MAX];
 
+	if (!canwrite && mode != RRQ)
+		return (EACCESS);
+
 	if (strcmp(requested, SEEDPATH) == 0) {
 		char *buf;
 		if (mode != RRQ)