droe / sslsplit

Transparent SSL/TLS interception
https://www.roe.ch/SSLsplit
BSD 2-Clause "Simplified" License
1.73k stars 327 forks source link

logpkt: handle partial write #302

Closed disaykin closed 1 year ago

disaykin commented 1 year ago

@droe Can you merge my PR?

sonertari commented 1 year ago

Looking good to me, but see the following sample code from OpenBSD write(2):

 A typical loop allowing partial writes looks like this:

 const char *buf;
 size_t bsz, off;
 ssize_t nw;
 int d;

 for (off = 0; off < bsz; off += nw)
         if ((nw = write(d, buf + off, bsz - off)) == 0 || nw == -1)
                 err(1, "write");

For some reason they quit if the written bytes is 0, contrary to the code you provided. Any idea why? Because 0 written bytes does not mean partial write? It means no write, and could cause infinite loop? If so, should we handle that case too?

Note that on both Linux and OpenBSD, write(2) reads that -1 is returned and EINTR is set before any data could be written. So retval is not 0 in that case.

disaykin commented 1 year ago

@sonertari As far as I know, a write operation can return 0 on network sockets if the remote side has closed the connection. All subsequent write attempts will return -1. Thus, the code I proposed will simply do an extra iteration before returning an error.

sonertari commented 1 year ago

I think the 0 retval does not seem like an error condition, so I think we shouldn't return error for it. So your code seems correct to me too. The following is the write() function on OpenBSD:

ssize_t
write(int fd, void *dest, size_t bcount)
{
        struct open_file *f = &files[fd];
        size_t resid;

        if ((unsigned)fd >= SOPEN_MAX || !(f->f_flags & F_WRITE)) {
                errno = EBADF;
                return (-1);
        }
        if (f->f_flags & F_RAW) {
                twiddle();
                errno = (f->f_dev->dv_strategy)(f->f_devdata, F_WRITE,
                    btodb(f->f_offset), bcount, dest, &resid);
                if (errno)
                        return (-1);
                f->f_offset += resid;
                return (resid);
        }
        resid = bcount;
        if ((errno = (f->f_ops->write)(f, dest, bcount, &resid)))
                return (-1);
        return (0);
}