Re: tftp protocol

[jnemeth%victoria.tc.ca@localhost (John Nemeth)]

On May 29,  1:18pm, Patrick Welche wrote:
} 
} The tftpd discussion was a while ago now. In brief, I had a strange
} "opcode 768" error when tftp'ing a large file. This was because
} the block number of the transfer overflowed its 16bit integer. This
} wasn't obvious to me because the error was obscure. I propose in
} the attached patch bailing out and printing a sensible error message
} on overflow. It also wasn't obvious because our tcpdump doesn't
} know about tftp options, and I attached a patch already in this
} thread, which will become redundant once the in-tree tcpdump is
} upgraded as tcpdump has learnt about tftp options in the meantime.
} 
} The other part was how to deal with clients which use a different path
} separator to unix, and I proposed a -p option to tftpd (enclosed once
} more). (I prefer this to the one Manuel passed on which swapped all \
} for / during connection phase rather than just the filename.)

     Whilst I think the client should be fixed, this option is
non-contentious.  The only thing is I would check to see if any of the
other BSDs supports and use the same option letter.

} The number of blocks mentioned in the BUGS section was pointed out to
} be incorrect, and that is amended in the attached patch (hopefully
} correctly this time, but correct me if I'm wrong!)
} 
} We could reopen the argument on whether bailing with an error is better
} than choosing to overflow to 0 (with the "block 0" is special for option
} acknowledgement problem) or to 1 (I haven't actually seen a client do this).
} Neither of those options is "right".

     The block counter should wrap as is done by both FreeBSD and
OpenBSD.  This is most likely necessary to support downloading large
files on certain devices (i.e. Cisco devices running IOS).  I need to
further investigate this, but will most likely be implementing this
change.

} BTW that (u_short) patch I had sent in this thread is wrong, as it
} effectively enabled overflow to 0 but didn't treat the not-first block 0 in
} the same way as other blocks. I think the patch Manuel passed on did this
} correctly.
} 
} Index: tftpd.c
} ===================================================================
} RCS file: /cvsroot/src/libexec/tftpd/tftpd.c,v
} retrieving revision 1.32
} diff -u -r1.32 tftpd.c
} --- tftpd.c   16 Mar 2009 01:56:21 -0000      1.32
} +++ tftpd.c   5 Jan 2010 16:14:30 -0000
} @@ -107,6 +107,7 @@
}  static int   suppress_naks;
}  static int   logging;
}  static int   secure;
} +static char  pathsep = '\0';
}  static char  *securedir;
}  
}  struct formats;
} @@ -141,7 +142,7 @@
}  {
}  
}       syslog(LOG_ERR,
} -    "Usage: %s [-dln] [-u user] [-g group] [-s directory] [directory ...]",
} +    "Usage: %s [-dln] [-u user] [-g group] [-s directory] [-p pathsep] 
[directory ...]",
}                   getprogname());
}       exit(1);
}  }
} @@ -170,7 +171,7 @@
}       curuid = getuid();
}       curgid = getgid();
}  
} -     while ((ch = getopt(argc, argv, "dg:lns:u:")) != -1)
} +     while ((ch = getopt(argc, argv, "dg:lnp:s:u:w:")) != -1)
}               switch (ch) {
}               case 'd':
}                       debug++;
} @@ -188,6 +189,11 @@
}                       suppress_naks = 1;
}                       break;
}  
} +             case 'p':
} +                     pathsep = optarg[0];
} +                     if (optarg[1] != '\0' || optarg[0] == '\0') usage();
} +                     break;
} +
}               case 's':
}                       secure = 1;
}                       securedir = optarg;
} @@ -662,6 +668,15 @@
}                       exit(1);
}               }
}       }
} +     /*
} +      * Globally replace the path separator given in the -p option
} +      * with / to cope with clients expecting a non-unix path separator.
} +      */
} +     if (pathsep != '\0') {
} +             for (cp = filename; *cp != '\0'; ++cp) {
} +                     if (*cp == pathsep) *cp = '/';
} +             }
} +     }
}       ecode = (*pf->f_validate)(&filename, tp->th_opcode);
}       if (logging) {
}               syslog(LOG_INFO, "%s: %s request for %s: %s",
} @@ -853,7 +868,7 @@
}       case OACK:
}               return "OACK";
}       default:
} -             (void)snprintf(obuf, sizeof(obuf), "*code %d*", code);
} +             (void)snprintf(obuf, sizeof(obuf), "*code 0x%x*", code);
}               return obuf;
}       }
}  }
} @@ -882,6 +897,11 @@
}       }
}  
}       do {
} +             if (block > UINT16_MAX) {
} +                     syslog(LOG_ERR, "tftpd: block number wrapped (hint: 
increase block size)");
} +                     nak(EBADOP);
} +                     goto abort;
} +             }
}               if (block > 0) {
}                       size = readit(file, &dp, tftp_blksize, pf->f_convert);
}                       if (size < 0) {
} 

     Why are you only handling the sendfile case and not the recvfile
case?

}-- End of excerpt from Patrick Welche