<div>Hi Felix, list,</div><div> </div><div>Thanks for your comments.</div><div>I have incorporated your comments into the tftpd.c,</div><div>attached is the modified file.</div><div> </div><div>regards,</div><div>Ashwini<br>
<br></div><div class="gmail_quote">On Sat, Oct 26, 2013 at 7:04 PM, Felix Janda <span dir="ltr"><<a href="mailto:felix.janda@posteo.de" target="_blank">felix.janda@posteo.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<div class="HOEnZb"><div class="h5">Ashwini Sharma wrote:<br>
> Hi Rob, All,<br>
><br>
> Attached is the TFTPD implementation.<br>
> Add it to hg, if you find it fine.<br>
><br>
> Let me know for any comments/improvements.<br>
><br>
> -Ashwini<br>
<br>
</div></div>Thanks for your submission. Some comments sprinkled into the code below.<br>
<br>
Felix<br>
<br>
> /* tftpd.c - TFTP server.<br>
> *<br>
> * Copyright 2013 Ranjan Kumar <<a href="mailto:ranjankumar.bth@gmail.com">ranjankumar.bth@gmail.com</a>><br>
> * Copyright 2013 Kyungwan Han <<a href="mailto:asura321@gmail.com">asura321@gmail.com</a>><br>
> *<br>
> * No Standard.<br>
><br>
> USE_TFTPD(NEWTOY(tftpd, "rcu:", TOYFLAG_BIN))<br>
><br>
> config TFTPD<br>
> bool "tftpd"<br>
> default y<br>
> help<br>
> usage: tftpd [-cr] [-u USER] [DIR]<br>
><br>
> Transfer file from/to tftp server.<br>
><br>
> -r Prohibit upload<br>
> -c Allow file creation via upload<br>
> -u Access files as USER<br>
> */<br>
> #define FOR_tftpd<br>
> #include "toys.h"<br>
> #include "toynet.h"<br>
><br>
> GLOBALS(<br>
> char *user;<br>
> long sfd;<br>
> struct passwd *pw;<br>
> )<br>
><br>
> #define TFTPD_BLKSIZE 512 // as per RFC 1350.<br>
><br>
> // opcodes<br>
> #define TFTPD_OP_RRQ 1 // Read Request RFC 1350, RFC 2090<br>
> #define TFTPD_OP_WRQ 2 // Write Request RFC 1350<br>
> #define TFTPD_OP_DATA 3 // Data chunk RFC 1350<br>
> #define TFTPD_OP_ACK 4 // Acknowledgement RFC 1350<br>
> #define TFTPD_OP_ERR 5 // Error Message RFC 1350<br>
> #define TFTPD_OP_OACK 6 // Option acknowledgment RFC 2347<br>
><br>
> // Error Codes:<br>
> #define TFTPD_ER_NOSUCHFILE 1 // File not found<br>
> #define TFTPD_ER_ACCESS 2 // Access violation<br>
> #define TFTPD_ER_FULL 3 // Disk full or allocation exceeded<br>
> #define TFTPD_ER_ILLEGALOP 4 // Illegal TFTP operation<br>
> #define TFTPD_ER_UNKID 5 // Unknown transfer ID<br>
> #define TFTPD_ER_EXISTS 6 // File already exists<br>
> #define TFTPD_ER_UNKUSER 7 // No such user<br>
> #define TFTPD_ER_NEGOTIATE 8 // Terminate transfer due to option negotiation<br>
><br>
> /* TFTP Packet Formats<br>
> * Type Op # Format without header<br>
> * 2 bytes string 1 byte string 1 byte<br>
> * -----------------------------------------------<br>
> * RRQ/ | 01/02 | Filename | 0 | Mode | 0 |<br>
> * WRQ -----------------------------------------------<br>
> * 2 bytes 2 bytes n bytes<br>
> * ---------------------------------<br>
> * DATA | 03 | Block # | Data |<br>
> * ---------------------------------<br>
> * 2 bytes 2 bytes<br>
> * -------------------<br>
> * ACK | 04 | Block # |<br>
> * --------------------<br>
> * 2 bytes 2 bytes string 1 byte<br>
> * ----------------------------------------<br>
> * ERROR | 05 | ErrorCode | ErrMsg | 0 |<br>
> * ----------------------------------------<br>
> */<br>
<br>
Thanks for the useful comments.<br>
<br>
> static char g_buff[TFTPD_BLKSIZE];<br>
> static char g_errpkt[TFTPD_BLKSIZE];<br>
<br>
How about using toybuf and toybuf + TFTP_BLKSIZE for this?<br></blockquote><div> </div><div>Modified code as per your suggestion.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
> static void bind_and_connect(struct sockaddr *srcaddr,<br>
> struct sockaddr *dstaddr, socklen_t socklen)<br>
> {<br>
> int set = 1;<br>
><br>
> TT.sfd = xsocket(dstaddr->sa_family, SOCK_DGRAM, 0);<br>
> if (setsockopt(TT.sfd, SOL_SOCKET, SO_REUSEADDR, (const void *)&set,<br>
> sizeof(set)) < 0) perror_exit("setsockopt failed");<br>
> if (bind(TT.sfd, srcaddr, socklen)) perror_exit("bind");<br>
> if (connect(TT.sfd, dstaddr, socklen) < 0)<br>
> perror_exit("can't connect to remote host");<br>
> }<br>
<br>
This can be inlined into tftpd_main().<br></blockquote><div>inlined the function</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
> // Create and send error packet.<br>
> static void send_errpkt(struct sockaddr *dstaddr,<br>
> socklen_t socklen, char *errmsg)<br>
> {<br>
> error_msg(errmsg);<br>
> g_errpkt[1] = TFTPD_OP_ERR;<br>
> strcpy(g_errpkt + 4, errmsg);<br>
> if (sendto(TT.sfd, g_errpkt, strlen(errmsg)+5, 0, dstaddr, socklen) < 0)<br>
> perror_exit("sendto failed");<br>
> }<br>
<br>
Why does this function not set g_errpkt[3] (the error code)? In some cases<br>
before send_errpkt is called it is set up, in some cases not. In the second<br>
type of case I guess the error code of the previous packet would be used.<br>
Is that intentional?<br>
<br></blockquote><div>On an error, the server sends the error packet and shuts down. In places where the error code is not set,</div><div>it is intentional to set it as ZERO. which happens due the _static_ global declaration of the __g_errpkt__ variable.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
> // Used to send / receive packets.<br>
> static void do_action(struct sockaddr *srcaddr, struct sockaddr *dstaddr,<br>
> socklen_t socklen, char *file, int opcode, int tsize, int blksize)<br>
<br>
This is also only used once in tftpd_main() and can be inlined. That would<br>
however require quite some work.<br></blockquote><div>This is not done intentionaly, as this will result into one function being very big, of around 300 lines.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
> {<br>
> int fd, done = 0, retry_count = 12, timeout = 100, len;<br>
> uint16_t blockno = 1, pktopcode, rblockno;<br>
> char *ptr, *spkt, *rpkt;<br>
> struct pollfd pollfds[1];<br>
><br>
> spkt = xzalloc(blksize + 4);<br>
> rpkt = xzalloc(blksize + 4);<br>
> ptr = spkt+2; //point after opcode.<br>
><br>
> pollfds[0].fd = TT.sfd;<br>
> // initialize groups, setgid and setuid<br>
> if (TT.pw) {<br>
> if (change_identity(TT.pw)) perror_exit("Failed to change identity");<br>
> endgrent();<br>
> }<br>
><br>
> if (opcode == TFTPD_OP_RRQ) fd = open(file, O_RDONLY, 0666);<br>
> else fd = open(file, ((toys.optflags & FLAG_c) ?<br>
> (O_WRONLY|O_TRUNC|O_CREAT) : (O_WRONLY|O_TRUNC)) , 0666);<br>
> if (fd < 0) {<br>
> g_errpkt[3] = TFTPD_ER_NOSUCHFILE;<br>
> send_errpkt(dstaddr, socklen, "can't open file");<br>
> goto CLEAN_APP;<br>
> }<br>
> // For download -> blockno will be 1.<br>
> // 1st ACK will be from dst,which will have blockno-=1<br>
> // Create and send ACK packet.<br>
> if (blksize != TFTPD_BLKSIZE || tsize) {<br>
> pktopcode = TFTPD_OP_OACK;<br>
> // add "blksize\000blksize_val\000" in send buffer.<br>
> if (blksize != TFTPD_BLKSIZE) {<br>
> strcpy(ptr, "blksize");<br>
> ptr += strlen("blksize") + 1;<br>
> ptr += snprintf(ptr, 6, "%d", blksize) + 1;<br>
> }<br>
> if (tsize) {// add "tsize\000tsize_val\000" in send buffer.<br>
> struct stat sb;<br>
><br>
> sb.st_size = 0;<br>
> fstat(fd, &sb);<br>
> strcpy(ptr, "tsize");<br>
> ptr += strlen("tsize") + 1;<br>
> ptr += sprintf(ptr, "%lu", (unsigned long)sb.st_size)+1;<br>
> }<br>
> goto SEND_PKT;<br>
> }<br>
> // upload -> ACK 1st packet with filename, as it has blockno 0.<br>
> if (opcode == TFTPD_OP_WRQ) blockno = 0;<br>
><br>
> // Prepare DATA and/or ACK pkt and send it.<br>
> for (;;) {<br>
> int poll_ret;<br>
><br>
> retry_count = 12, timeout = 100, pktopcode = TFTPD_OP_ACK;<br>
> ptr = spkt+2;<br>
> *((uint16_t*)ptr) = htons(blockno);<br>
> blockno++;<br>
> ptr += 2;<br>
> if (opcode == TFTPD_OP_RRQ) {<br>
> pktopcode = TFTPD_OP_DATA;<br>
> len = readall(fd, ptr, blksize);<br>
> if (len < 0) {<br>
> send_errpkt(dstaddr, socklen, "read-error");<br>
> break;<br>
> }<br>
> if (len != blksize) done = 1; //last pkt.<br>
> ptr += len;<br>
> }<br>
> SEND_PKT:<br>
> // 1st ACK will be from dst, which will have blockno-=1<br>
> *((uint16_t*)spkt) = htons(pktopcode); //append send pkt's opcode.<br>
> RETRY_SEND:<br>
> if (sendto(TT.sfd, spkt, (ptr - spkt), 0, dstaddr, socklen) <0)<br>
> perror_exit("sendto failed");<br>
> // if "block size < 512", send ACK and exit.<br>
> if ((pktopcode == TFTPD_OP_ACK) && done) break;<br>
><br>
> POLL_IN:<br>
> pollfds[0].events = POLLIN;<br>
> pollfds[0].fd = TT.sfd;<br>
> poll_ret = poll(pollfds, 1, timeout);<br>
> if (poll_ret < 0 && (errno == EINTR || errno == ENOMEM)) goto POLL_IN;<br>
> if (!poll_ret) {<br>
> if (!--retry_count) {<br>
> error_msg("timeout");<br>
> break;<br>
> }<br>
> timeout += 150;<br>
> goto RETRY_SEND;<br>
> } else if (poll_ret == 1) {<br>
> len = read(pollfds[0].fd, rpkt, blksize + 4);<br>
> if (len < 0) {<br>
> send_errpkt(dstaddr, socklen, "read-error");<br>
> break;<br>
> }<br>
> if (len < 4) goto POLL_IN;<br>
> } else {<br>
> perror_msg("poll");<br>
> break;<br>
> }<br>
> // Validate receive packet.<br>
> pktopcode = ntohs(((uint16_t*)rpkt)[0]);<br>
> rblockno = ntohs(((uint16_t*)rpkt)[1]);<br>
> if (pktopcode == TFTPD_OP_ERR) {<br>
> switch(rblockno) {<br>
> case TFTPD_ER_NOSUCHFILE: error_msg("File not found"); break;<br>
> case TFTPD_ER_ACCESS: error_msg("Access violation"); break;<br>
> case TFTPD_ER_FULL: error_msg("Disk full or allocation exceeded");<br>
> break;<br>
> case TFTPD_ER_ILLEGALOP: error_msg("Illegal TFTP operation"); break;<br>
> case TFTPD_ER_UNKID: error_msg("Unknown transfer ID"); break;<br>
> case TFTPD_ER_EXISTS: error_msg("File already exists"); break;<br>
> case TFTPD_ER_UNKUSER: error_msg("No such user"); break;<br>
> case TFTPD_ER_NEGOTIATE:<br>
> error_msg("Terminate transfer due to option negotiation"); break;<br>
> default: error_msg("DATA Check failure."); break;<br>
> }<br>
<br>
How about<br>
<br>
char *message = "DATA Check failure.";<br>
<br>
if (rblockno && (rblockno < 9)) message =<br>
((char **){"File not found", "Access violation",<br>
"Disk full or allocation exceeded", "Illegal TFTP operation",<br>
"Unknown transfer ID", "File already exists",<br>
"No such user", "Terminate transfer due to option negotiation"})[rblockno - 1];<br>
error_msg(message);<br></blockquote><div>this is a good suggestion, modified the code with a little modification. As for this snippet, compiler shouts of excess </div><div>initialization for message.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
> break; // Break the for loop.<br>
> }<br>
><br>
> // if download requested by client,<br>
> // server will send data pkt and will receive ACK pkt from client.<br>
> if ((opcode == TFTPD_OP_RRQ) && (pktopcode == TFTPD_OP_ACK)) {<br>
> if (rblockno == (uint16_t) (blockno - 1)) {<br>
> if (!done) continue; // Send next chunk of data.<br>
> break;<br>
> }<br>
> }<br>
><br>
> // server will receive DATA pkt and write the data.<br>
> if ((opcode == TFTPD_OP_WRQ) && (pktopcode == TFTPD_OP_DATA)) {<br>
> if (rblockno == blockno) {<br>
> int nw = writeall(fd, &rpkt[4], len-4);<br>
> if (nw != len-4) {<br>
> g_errpkt[3] = TFTPD_ER_FULL;<br>
> send_errpkt(dstaddr, socklen, "write error");<br>
> break;<br>
> }<br>
><br>
> if (nw != blksize) done = 1;<br>
> }<br>
> continue;<br>
> }<br>
> goto POLL_IN;<br>
> } // end of loop<br>
><br>
> CLEAN_APP:<br>
> if (CFG_TOYBOX_FREE) {<br>
> free(spkt);<br>
> free(rpkt);<br>
> close(fd);<br>
> }<br>
> }<br>
><br>
> void tftpd_main(void)<br>
> {<br>
> int recvmsg_len, rbuflen, opcode, blksize = TFTPD_BLKSIZE, tsize = 0;<br>
> struct sockaddr_storage srcaddr, dstaddr;<br>
> static socklen_t socklen = sizeof(struct sockaddr_storage);<br>
<br>
Leave out the "static".<br></blockquote><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
> char *buf = g_buff;<br>
><br>
> TT.pw = NULL;<br>
> memset(&srcaddr, 0, sizeof(srcaddr));<br>
> if (getsockname(STDIN_FILENO, (struct sockaddr*)&srcaddr, &socklen)) {<br>
> toys.exithelp = 1;<br>
> error_exit(NULL);<br>
> }<br>
><br>
> if (toys.optflags & FLAG_u) {<br>
> struct passwd *pw = getpwnam(TT.user);<br>
> if (!pw) error_exit("unknown user %s", TT.user);<br>
> TT.pw = pw;<br>
> }<br>
> if (*toys.optargs) {<br>
> if (chroot(*toys.optargs))<br>
> perror_exit("can't change root directory to '%s'", *toys.optargs);<br>
> if (chdir("/")) perror_exit("can't change directory to '/'");<br>
> }<br>
><br>
> recvmsg_len = recvfrom(STDIN_FILENO, g_buff, TFTPD_BLKSIZE, 0,<br>
> (struct sockaddr*)&dstaddr, &socklen);<br>
> bind_and_connect((struct sockaddr*)&srcaddr, (struct sockaddr*)&dstaddr, socklen);<br>
> // Error condition.<br>
> if (recvmsg_len < 4 || recvmsg_len > TFTPD_BLKSIZE<br>
> || g_buff[recvmsg_len-1] != '\0') {<br>
> send_errpkt((struct sockaddr*)&dstaddr, socklen, "packet format error");<br>
> return;<br>
> }<br>
><br>
> // request is either upload or Download.<br>
> opcode = ntohs(*(uint16_t*)buf);<br>
> if (((opcode != TFTPD_OP_RRQ) && (opcode != TFTPD_OP_WRQ))<br>
> || ((opcode == TFTPD_OP_WRQ) && (toys.optflags & FLAG_r))) {<br>
> send_errpkt((struct sockaddr*)&dstaddr, socklen,<br>
> ((opcode == TFTPD_OP_WRQ) ? "write error" : "packet format error"));<br>
> return;<br>
> }<br>
><br>
> buf += 2;<br>
> if (*buf == '.' || strstr(buf, "/.")) {<br>
> send_errpkt((struct sockaddr*)&dstaddr, socklen, "dot in filename");<br>
> return;<br>
> }<br>
><br>
> buf += strlen(buf) + 1; //1 '\0'.<br>
> // As per RFC 1350, mode is case in-sensitive.<br>
> if ((buf >= (g_buff + recvmsg_len)) || (strcasecmp(buf, "octet"))) {<br>
> send_errpkt((struct sockaddr*)&dstaddr, socklen, "packet format error");<br>
> return;<br>
> }<br>
><br>
> //RFC2348. e.g. of size type: "ttype1\0ttype1_val\0...ttypeN\0ttypeN_val\0"<br>
> buf += strlen(buf) + 1;<br>
> rbuflen = g_buff + recvmsg_len - buf;<br>
> if (rbuflen) {<br>
> int jump = 0, bflag = 0;<br>
><br>
> for (; rbuflen; rbuflen -= jump, buf += jump) {<br>
> if (!bflag && !strcasecmp(buf, "blksize")) { //get blksize<br>
> errno = 0;<br>
> blksize = strtoul(buf, NULL, 10);<br>
> if (errno || blksize > 65564 || blksize < 8) blksize = TFTPD_BLKSIZE;<br>
> bflag ^= 1;<br>
> } else if (!tsize && !strcasecmp(buf, "tsize")) tsize ^= 1;<br>
><br>
> jump += strlen(buf) + 1;<br>
> }<br>
> tsize &= (opcode == TFTPD_OP_RRQ);<br>
> }<br>
><br>
> //do send / receive file.<br>
> do_action((struct sockaddr*)&srcaddr, (struct sockaddr*)&dstaddr,<br>
> socklen, g_buff + 2, opcode, tsize, blksize);<br>
> if (CFG_TOYBOX_FREE) close(STDIN_FILENO);<br>
> }<br>
</blockquote></div><br>