From owner-svn-src-stable-11@freebsd.org Mon Apr 16 16:33:36 2018 Return-Path: Delivered-To: svn-src-stable-11@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 61AF1F9190C; Mon, 16 Apr 2018 16:33:36 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0F56A72C09; Mon, 16 Apr 2018 16:33:36 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 0A4091FDC4; Mon, 16 Apr 2018 16:33:36 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w3GGXZEp004714; Mon, 16 Apr 2018 16:33:35 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w3GGXZxk004710; Mon, 16 Apr 2018 16:33:35 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201804161633.w3GGXZxk004710@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Mon, 16 Apr 2018 16:33:35 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r332609 - in stable/11: libexec/tftpd libexec/tftpd/tests usr.bin/tftp X-SVN-Group: stable-11 X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in stable/11: libexec/tftpd libexec/tftpd/tests usr.bin/tftp X-SVN-Commit-Revision: 332609 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Apr 2018 16:33:36 -0000 Author: asomers Date: Mon Apr 16 16:33:35 2018 New Revision: 332609 URL: https://svnweb.freebsd.org/changeset/base/332609 Log: MFC r330710, r330718-r330720 r330710: tftpd: Flush files as soon as they are fully received On an RRQ, tftpd doesn't exit as soon as it's finished receiving a file. Instead, it waits five seconds just in case the client didn't receive the server's last ACK and decides to resend the final DATA packet. Unfortunately, this created a 5 second delay from when the client thinks it's done sending the file, and when the file is available for other processes. Fix this bug by closing the file as soon as receipt is finished. PR: 157700 Reported by: Barry Mishler r330718: tftpd: Verify world-writability for WRQ when using relative paths tftpd(8) says that files may only be written if they already exist and are publicly writable. tftpd.c verifies that a file is publicly writable if it uses an absolute pathname. However, if the pathname is relative, that check is skipped. Fix it. Note that this is not a security vulnerability, because the transfer ultimately doesn't work unless the file already exists and is owned by user nobody. Also, this bug does not affect the default configuration, because the default uses the "-s" option which makes all pathnames absolute. PR: 226004 r330719: tftpd: Abort on an WRQ access violation On a WRQ (write request) tftpd checks whether the client has access permission for the file in question. If not, then the write is prevented. However, tftpd doesn't reply with an ERROR packet, nor does it abort. Instead, it tries to receive the packet anyway. The symptom is slightly different depending on the nature of the error. If the target file is nonexistent and tftpd lacks permission to create it, then tftpd will willingly receive the file, but not write it anywhere. If the file exists but is not writable, then tftpd will fail to ACK to WRQ. PR: 225996 r330720: tftpd: reject unknown opcodes If tftpd receives a command with an unknown opcode, it simply exits 1. It doesn't send an ERROR packet, and the client will hang waiting for one. Fix it. PR: 226005 Modified: stable/11/libexec/tftpd/tests/functional.c stable/11/libexec/tftpd/tftp-transfer.c stable/11/libexec/tftpd/tftpd.c stable/11/usr.bin/tftp/tftp.c Directory Properties: stable/11/ (props changed) Modified: stable/11/libexec/tftpd/tests/functional.c ============================================================================== --- stable/11/libexec/tftpd/tests/functional.c Mon Apr 16 16:32:00 2018 (r332608) +++ stable/11/libexec/tftpd/tests/functional.c Mon Apr 16 16:33:35 2018 (r332609) @@ -348,6 +348,10 @@ setup(struct sockaddr_storage *to, uint16_t idx) ATF_REQUIRE((client_s = socket(protocol, SOCK_DGRAM, 0)) > 0); break; } + + /* Clear the client's umask. Test cases will specify exact modes */ + umask(0000); + return (client_s); } @@ -671,7 +675,6 @@ TFTPD_TC_DEFINE(unknown_opcode,) { /* Looks like an RRQ or WRQ request, but with a bad opcode */ SEND_STR("\0\007foo.txt\0octet\0"); - atf_tc_expect_timeout("PR 226005 tftpd ignores bad opcodes but doesn't reject them"); RECV_ERROR(4, "Illegal TFTP operation"); } @@ -692,7 +695,6 @@ TFTPD_TC_DEFINE(w_flag,, w_flag = 1;) send_data(1, contents, contents_len); recv_ack(1); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("small.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); @@ -714,7 +716,7 @@ TFTPD_TC_DEFINE(wrq_dropped_ack,) for (i = 0; i < nitems(contents); i++) contents[i] = i; - fd = open("medium.txt", O_RDWR | O_CREAT, 0644); + fd = open("medium.txt", O_RDWR | O_CREAT, 0666); ATF_REQUIRE(fd >= 0); close(fd); @@ -730,7 +732,6 @@ TFTPD_TC_DEFINE(wrq_dropped_ack,) send_data(2, (const char*)&contents[128], 256); recv_ack(2); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); @@ -749,7 +750,7 @@ TFTPD_TC_DEFINE(wrq_dropped_data,) size_t contents_len; char buffer[1024]; - fd = open("small.txt", O_RDWR | O_CREAT, 0644); + fd = open("small.txt", O_RDWR | O_CREAT, 0666); ATF_REQUIRE(fd >= 0); close(fd); contents_len = strlen(contents) + 1; @@ -764,7 +765,6 @@ TFTPD_TC_DEFINE(wrq_dropped_data,) send_data(1, contents, contents_len); recv_ack(1); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("small.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); @@ -786,7 +786,7 @@ TFTPD_TC_DEFINE(wrq_duped_data,) for (i = 0; i < nitems(contents); i++) contents[i] = i; - fd = open("medium.txt", O_RDWR | O_CREAT, 0644); + fd = open("medium.txt", O_RDWR | O_CREAT, 0666); ATF_REQUIRE(fd >= 0); close(fd); @@ -799,7 +799,6 @@ TFTPD_TC_DEFINE(wrq_duped_data,) send_data(2, (const char*)&contents[128], 256); recv_ack(2); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); @@ -819,8 +818,6 @@ TFTPD_TC_DEFINE(wrq_eaccess,) close(fd); SEND_WRQ("empty.txt", "octet"); - atf_tc_expect_fail("PR 225996 tftpd doesn't abort on a WRQ access " - "violation"); RECV_ERROR(2, "Access violation"); } @@ -837,7 +834,6 @@ TFTPD_TC_DEFINE(wrq_eaccess_world_readable,) close(fd); SEND_WRQ("empty.txt", "octet"); - atf_tc_expect_fail("PR 226004 with relative pathnames, tftpd doesn't validate world writability"); RECV_ERROR(2, "Access violation"); } @@ -867,7 +863,6 @@ TFTPD_TC_DEFINE(wrq_medium,) send_data(2, (const char*)&contents[128], 256); recv_ack(2); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); @@ -894,14 +889,13 @@ TFTPD_TC_DEFINE(wrq_netascii,) fd = open("unix.txt", O_RDWR | O_CREAT, 0666); ATF_REQUIRE(fd >= 0); close(fd); - contents_len = strlen(contents) + 1; + contents_len = sizeof(contents); SEND_WRQ("unix.txt", "netascii"); recv_ack(0); send_data(1, contents, contents_len); recv_ack(1); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("unix.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); @@ -916,8 +910,6 @@ TFTPD_TC_DEFINE(wrq_netascii,) TFTPD_TC_DEFINE(wrq_nonexistent,) { SEND_WRQ("nonexistent.txt", "octet"); - atf_tc_expect_fail("PR 225996 tftpd doesn't abort on a WRQ access " - "violation"); RECV_ERROR(1, "File not found"); } @@ -942,7 +934,6 @@ TFTPD_TC_DEFINE(wrq_small,) send_data(1, contents, contents_len); recv_ack(1); - atf_tc_expect_fail("PR 157700 tftpd expects more data after EOF"); fd = open("small.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); Modified: stable/11/libexec/tftpd/tftp-transfer.c ============================================================================== --- stable/11/libexec/tftpd/tftp-transfer.c Mon Apr 16 16:32:00 2018 (r332608) +++ stable/11/libexec/tftpd/tftp-transfer.c Mon Apr 16 16:33:35 2018 (r332609) @@ -302,6 +302,8 @@ send_ack: gettimeofday(&(ts->tstop), NULL); } while (n_data == segsize); + write_close(); + /* Don't do late packet management for the client implementation */ if (acting_as_client) return; Modified: stable/11/libexec/tftpd/tftpd.c ============================================================================== --- stable/11/libexec/tftpd/tftpd.c Mon Apr 16 16:32:00 2018 (r332608) +++ stable/11/libexec/tftpd/tftpd.c Mon Apr 16 16:33:35 2018 (r332609) @@ -419,8 +419,7 @@ main(int argc, char *argv[]) "%s read access denied", peername); exit(1); } - } - if (tp->th_opcode == WRQ) { + } else if (tp->th_opcode == WRQ) { if (allow_wo) tftp_wrq(peer, tp->th_stuff, n - 1); else { @@ -428,7 +427,8 @@ main(int argc, char *argv[]) "%s write access denied", peername); exit(1); } - } + } else + send_error(peer, EBADOP); exit(1); } @@ -543,6 +543,10 @@ tftp_wrq(int peer, char *recvbuffer, ssize_t size) filename, errtomsg(ecode)); } + if (ecode) { + send_error(peer, ecode); + exit(1); + } tftp_recvfile(peer, mode); exit(0); } @@ -741,8 +745,12 @@ validate_access(int peer, char **filep, int mode) dirp->name, filename); if (stat(pathname, &stbuf) == 0 && (stbuf.st_mode & S_IFMT) == S_IFREG) { - if ((stbuf.st_mode & S_IROTH) != 0) { - break; + if (mode == RRQ) { + if ((stbuf.st_mode & S_IROTH) != 0) + break; + } else { + if ((stbuf.st_mode & S_IWOTH) != 0) + break; } err = EACCESS; } @@ -751,6 +759,8 @@ validate_access(int peer, char **filep, int mode) *filep = filename = pathname; else if (mode == RRQ) return (err); + else if (err != ENOTFOUND || !create_new) + return (err); } /* @@ -821,7 +831,6 @@ tftp_recvfile(int peer, const char *mode) block = 0; tftp_receive(peer, &block, &ts, NULL, 0); - write_close(); gettimeofday(&now2, NULL); if (debug&DEBUG_SIMPLE) { Modified: stable/11/usr.bin/tftp/tftp.c ============================================================================== --- stable/11/usr.bin/tftp/tftp.c Mon Apr 16 16:32:00 2018 (r332608) +++ stable/11/usr.bin/tftp/tftp.c Mon Apr 16 16:33:35 2018 (r332609) @@ -263,7 +263,6 @@ recvfile(int peer, char *port, int fd, char *name, cha tftp_receive(peer, &block, &tftp_stats, rp, n); } - write_close(); if (tftp_stats.amount > 0) printstats("Received", verbose, &tftp_stats); return;