On Fri, Oct 20, 2017 at 12:04:41PM +0000, Jeremie Courreges-Anglas wrote:
> On Fri, Oct 20 2017, Sebastien Marie <sema...@online.fr> wrote:
> > On Thu, Oct 19, 2017 at 08:58:12PM +0200, Jan Klemkow wrote:
> >> + char nfilename[PATH_MAX];
> >> +
> >> + snprintf(nfilename, sizeof nfilename, "%s/%s",
> >> + getip(&client->ss), filename);
> >
> > - filename has PATH_MAX length
> > - getip(&client->ss) could have NI_MAXHOST length
>
> INET6_ADDRSTRLEN since getip() calls getnameinfo(NI_NUMERICHOST), but
> your point stands.
>
> > so nfilename could be larger than PATH_MAX (sizeof nfilename).
> >
> > I assume the return of snprintf() need to be checked. if truncation
> > occured, a warning should be issued and nfilename discarded (just
> > calling tftp_open(client, filename)) ?
>
> I think we should refuse the request if truncated.
done
> >> + if (access(nfilename, R_OK) == 0)
> >> + tftp_open(client, nfilename);
> >> + else
> >> + tftp_open(client, filename);
> >> + }
>
> Here we look up the same file in both the client-specific subdirectory
> and the default directory. Should we instead look only in the
> client-specific directory if the latter exist?
Common files should be found in the default directory. But, host
specific files could be overwritten if they exist in the subdirectory.
The diff below should address all comments.
Thanks,
Jan
Index: tftpd.8
===================================================================
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.8,v
retrieving revision 1.5
diff -u -p -r1.5 tftpd.8
--- tftpd.8 18 Jul 2015 05:32:56 -0000 1.5
+++ tftpd.8 21 Oct 2017 18:41:15 -0000
@@ -37,7 +37,7 @@
.Nd DARPA Trivial File Transfer Protocol daemon
.Sh SYNOPSIS
.Nm tftpd
-.Op Fl 46cdv
+.Op Fl 46cdiv
.Op Fl l Ar address
.Op Fl p Ar port
.Op Fl r Ar socket
@@ -100,6 +100,16 @@ If this option is specified,
.Nm
will run in the foreground and log
the client IP, type of request, and filename to stderr.
+.It Fl i
+.Nm
+tries to rewrite the requested filename with a prefix of
+the client's IP address.
+If the rewritten path exists
+.Nm
+will serve this file.
+If not, it will serve the original filename.
+This option can not be combined with
+.Fl r .
.It Fl l Ar address
Listen on the specified address.
By default
@@ -126,6 +136,8 @@ before the TFTP request will continue.
By default
.Nm
does not use filename rewriting.
+This option can not be combined with
+.Fl i .
.It Fl v
Log the client IP, type of request, and filename.
.It Ar directory
Index: tftpd.c
===================================================================
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.39
diff -u -p -r1.39 tftpd.c
--- tftpd.c 26 May 2017 17:38:46 -0000 1.39
+++ tftpd.c 21 Oct 2017 19:49:04 -0000
@@ -282,7 +282,7 @@ __dead void
usage(void)
{
extern char *__progname;
- fprintf(stderr, "usage: %s [-46cdv] [-l address] [-p port] [-r socket]"
+ fprintf(stderr, "usage: %s [-46cdiv] [-l address] [-p port] [-r socket]"
" directory\n", __progname);
exit(1);
}
@@ -290,6 +290,7 @@ usage(void)
int cancreate = 0;
int verbose = 0;
int debug = 0;
+int iflag = 0;
int
main(int argc, char *argv[])
@@ -307,7 +308,7 @@ main(int argc, char *argv[])
int family = AF_UNSPEC;
int devnull = -1;
- while ((c = getopt(argc, argv, "46cdl:p:r:v")) != -1) {
+ while ((c = getopt(argc, argv, "46cdil:p:r:v")) != -1) {
switch (c) {
case '4':
family = AF_INET;
@@ -321,6 +322,11 @@ main(int argc, char *argv[])
case 'd':
verbose = debug = 1;
break;
+ case 'i':
+ if (rewrite != NULL)
+ usage();
+ iflag = 1;
+ break;
case 'l':
addr = optarg;
break;
@@ -328,6 +334,8 @@ main(int argc, char *argv[])
port = optarg;
break;
case 'r':
+ if (iflag == 1)
+ usage();
rewrite = optarg;
break;
case 'v':
@@ -903,7 +911,20 @@ again:
if (rwmap != NULL)
rewrite_map(client, filename);
- else
+ else if (iflag) {
+ char nfilename[PATH_MAX];
+
+ if (snprintf(nfilename, sizeof(nfilename), "%s/%s",
+ getip(&client->ss), filename) > PATH_MAX - 1) {
+ ecode = EBADOP;
+ goto error;
+ }
+
+ if (access(nfilename, R_OK) == 0)
+ tftp_open(client, nfilename);
+ else
+ tftp_open(client, filename);
+ } else
tftp_open(client, filename);
return;
