On Thu, Sep 27, 2018 at 11:59 AM sven falempin <sven.falem...@gmail.com> wrote:
> > > On Thu, Sep 27, 2018 at 10:47 AM sven falempin <sven.falem...@gmail.com> > wrote: > >> I m not sure how this is possible but here s the data : >> >> i used the ENV to push -w 5 in my pkg_add process : >> # date >> Thu Sep 27 10:40:28 EDT 2018 >> # ps auxww | grep pkgfet >> _pkgfetc 60348 0.0 0.1 1728 5456 ?? INp Wed05PM 0:00.09 /usr/bin/ftp -w 5 >> -S session -o - https:// >> myportal.com/tar/6.3/packages/amd64/p5-LWP-MediaTypes-6.02.tgz >> # fstat -p 60348 >> _pkgfetc ftp 60348 5* internet stream tcp 0xffff8000006e8548 >> 172.16.1.35:5512 --> 92.222.70.241:443 >> >> I can see the PF state on the natting device: >> >> tcp 206.180.254.190:52251 (172.16.1.35:5512) -> 92.222.70.241:443 >> ESTABLISHED:ESTABLISHED >> age: 16:55:28 expires: 07:07:25 id: 5b7ce30b0094854a >> rule: pass out quick on em5 all flags S/SA label "READY_TO_NAT" tagged >> READY_TO_NAT >> >> this is stock 6.3 ftp which is still the same now ( >> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ftp/ ) >> >> # ktrace -p 60348 >> generate an empty 64 bit file. >> >> Any other test i could do to understand ? >> >> > Reading NC it seems that the 'poll' syscall is indeed required to timeout correctly. I m trying to find a way to recreate the problem and i am running a modified version in a loop My understanding of the tls_read(SSL_read then ssl_read) .. is that the retry is for SSL protocol error and retry which would only occur on non TCP layer. That's irrelevant in fetch.c I m just guessing the <handshake> ( at the top of ssl_read ) call will write and fix an POLLIN when write is done. This explaining trying to read again when a write is requested >.< Nevertheless each read/write code ( and even close ) shall be precede by a poll call. Bellow is the patch i m testing at the moment , i do not quite like it, it s not solving the problem, and show how the factorization of read is making the code hard to read/modify IMHO url_get should be just calling url_get_https url_get_http url_get_ftp and each of this function do only that. The _fin_ FILE* may be NULL fd may be -1 randomly in the function, it s kinda messy. And this make it difficult to nicely call poll before reading to avoid being blocked in a read indefinitely RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.167 diff -u -p -r1.167 fetch.c --- fetch.c 10 Feb 2018 06:25:16 -0000 1.167 +++ fetch.c 27 Sep 2018 20:10:52 -0000 @@ -59,6 +59,7 @@ #include <resolv.h> #ifndef NOSSL +#include <poll.h> #include <tls.h> #else /* !NOSSL */ struct tls; @@ -73,12 +74,12 @@ void abortfile(int); char hextochar(const char *); char *urldecode(const char *); char *recode_credentials(const char *_userinfo); -int ftp_printf(FILE *, struct tls *, const char *, ...) __attribute__((format(printf, 3, 4))); +int ftp_printf(FILE *, int fd, struct tls *, const char *, ...) __attribute__((format(printf, 4, 5))); char *ftp_readline(FILE *, struct tls *, size_t *); size_t ftp_read(FILE *, struct tls *, char *, size_t); #ifndef NOSSL int proxy_connect(int, char *, char *); -int SSL_vprintf(struct tls *, const char *, va_list); +int SSL_vprintf(int fd, struct tls *, const char *, va_list); char *SSL_readline(struct tls *, size_t *); #endif /* !NOSSL */ @@ -98,6 +99,90 @@ jmp_buf httpabort; static int redirect_loop; +#ifndef NOSSL +/*from netcat.c correct way to timeout a tls_call*/ +int +timeout_tls(int s, struct tls *tls_ctx, int (*func)(struct tls *)) +{ + int timeout = connect_timeout; // glu + struct pollfd pfd; + int ret; + + while ((ret = (*func)(tls_ctx)) != 0) { + if (ret == TLS_WANT_POLLIN) + pfd.events = POLLIN; + else if (ret == TLS_WANT_POLLOUT) + pfd.events = POLLOUT; + else + break; + pfd.fd = s; + if ((ret = poll(&pfd, 1, timeout)) == 1) + continue; + else if (ret == 0) { + errno = ETIMEDOUT; + ret = -1; + break; + } else + err(1, "poll failed"); + } + + return ret; +} + +/*must realloc / offset*/ +int +timeout_tls_write(int s, struct tls *tls_ctx, + const void *buf, size_t buflen) +{ + int timeout = connect_timeout; // glu + struct pollfd pfd; + int ret; + + // should check ready first => do poll write then write + while ((ret = tls_write(tls_ctx,buf,buflen)) != 0) { + if (ret == TLS_WANT_POLLIN) + pfd.events = POLLIN; + else if (ret == TLS_WANT_POLLOUT) + pfd.events = POLLOUT; + else + break; + pfd.fd = s; + if ((ret = poll(&pfd, 1, timeout)) == 1) + continue; + else if (ret == 0) { + errno = ETIMEDOUT; + ret = -1; + break; + } else + err(1, "poll failed"); + } + + return ret; +} + +// this may never timeout ! +int +timeout_tls_read(struct tls *tls_ctx, + void *buf, size_t len) { + int tret; + for (;;) { + tret = tls_read(tls_ctx, buf, len); + if (tret == TLS_WANT_POLLIN) { + if (debug) + fprintf(ttyout, "POLLIN REQUEST !\n"); + continue; + } + if (tret == TLS_WANT_POLLOUT) { + if (debug) + fprintf(ttyout, "POLLOUT REQUEST !\n"); + continue; + } + break; + } + return tret; +} +#endif + /* * Determine whether the character needs encoding, per RFC1738: * - No corresponding graphic US-ASCII. @@ -678,13 +763,13 @@ noslash: * the original URI (path). */ if (credentials) - ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n" + ftp_printf(fin,fd, tls, "GET %s HTTP/1.0\r\n" "Proxy-Authorization: Basic %s\r\n" "Host: %s\r\n%s%s\r\n\r\n", epath, credentials, proxyhost, buf ? buf : "", httpuseragent); else - ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n" + ftp_printf(fin, fd, tls, "GET %s HTTP/1.0\r\n" "Host: %s\r\n%s%s\r\n\r\n", epath, proxyhost, buf ? buf : "", httpuseragent); } else { @@ -702,7 +787,7 @@ noslash: #endif /* SMALL */ #ifndef NOSSL if (credentials) { - ftp_printf(fin, tls, + ftp_printf(fin, fd, tls, "GET /%s %s\r\nAuthorization: Basic %s\r\nHost: ", epath, restart_point ? "HTTP/1.1\r\nConnection: close" : "HTTP/1.0", @@ -711,13 +796,13 @@ noslash: credentials = NULL; } else #endif /* NOSSL */ - ftp_printf(fin, tls, "GET /%s %s\r\nHost: ", epath, + ftp_printf(fin, fd, tls, "GET /%s %s\r\nHost: ", epath, #ifndef SMALL restart_point ? "HTTP/1.1\r\nConnection: close" : #endif /* !SMALL */ "HTTP/1.0"); if (proxyhost) { - ftp_printf(fin, tls, "%s", proxyhost); + ftp_printf(fin, fd, tls, "%s", proxyhost); port = NULL; } else if (strchr(host, ':')) { /* @@ -729,10 +814,10 @@ noslash: errx(1, "Can't allocate memory."); if ((p = strchr(h, '%')) != NULL) *p = '\0'; - ftp_printf(fin, tls, "[%s]", h); + ftp_printf(fin, fd, tls, "[%s]", h); free(h); } else - ftp_printf(fin, tls, "%s", host); + ftp_printf(fin, fd, tls, "%s", host); /* * Send port number only if it's specified and does not equal @@ -741,15 +826,15 @@ noslash: */ #ifndef NOSSL if (port && strcmp(port, (ishttpsurl ? "443" : "80")) != 0) - ftp_printf(fin, tls, ":%s", port); + ftp_printf(fin, fd, tls, ":%s", port); if (restart_point) - ftp_printf(fin, tls, "\r\nRange: bytes=%lld-", + ftp_printf(fin, fd, tls, "\r\nRange: bytes=%lld-", (long long)restart_point); #else /* !NOSSL */ if (port && strcmp(port, "80") != 0) - ftp_printf(fin, tls, ":%s", port); + ftp_printf(fin, fd, tls, ":%s", port); #endif /* !NOSSL */ - ftp_printf(fin, tls, "\r\n%s%s\r\n\r\n", + ftp_printf(fin, fd, tls, "\r\n%s%s\r\n\r\n", buf ? buf : "", httpuseragent); } free(epath); @@ -1040,9 +1125,8 @@ cleanup_url_get: if (tls_session_fd != -1) dprintf(STDERR_FILENO, "tls session resumed: %s\n", tls_conn_session_resumed(tls) ? "yes" : "no"); - do { - i = tls_close(tls); - } while (i == TLS_WANT_POLLIN || i == TLS_WANT_POLLOUT); + + i = timeout_tls(fd,tls,tls_close); tls_free(tls); } free(full_host); @@ -1527,9 +1611,8 @@ ftp_read(FILE *fp, struct tls *tls, char ret = fread(buf, sizeof(char), len, fp); #ifndef NOSSL else if (tls != NULL) { - do { - tret = tls_read(tls, buf, len); - } while (tret == TLS_WANT_POLLIN || tret == TLS_WANT_POLLOUT); + tret = timeout_tls_read(tls, buf, len); + if (tret < 0) errx(1, "SSL read error: %s", tls_error(tls)); ret = (size_t)tret; @@ -1539,7 +1622,7 @@ ftp_read(FILE *fp, struct tls *tls, char } int -ftp_printf(FILE *fp, struct tls *tls, const char *fmt, ...) +ftp_printf(FILE *fp, int fd, struct tls *tls, const char *fmt, ...) { int ret; va_list ap; @@ -1549,8 +1632,9 @@ ftp_printf(FILE *fp, struct tls *tls, co if (fp != NULL) ret = vfprintf(fp, fmt, ap); #ifndef NOSSL - else if (tls != NULL) - ret = SSL_vprintf(tls, fmt, ap); + else if (tls != NULL) { + ret = SSL_vprintf(fd, tls, fmt, ap); + } #endif /* !NOSSL */ else ret = 0; @@ -1568,7 +1652,7 @@ ftp_printf(FILE *fp, struct tls *tls, co #ifndef NOSSL int -SSL_vprintf(struct tls *tls, const char *fmt, va_list ap) +SSL_vprintf(int fd, struct tls *tls, const char *fmt, va_list ap) { char *string, *buf; size_t len; @@ -1579,9 +1663,7 @@ SSL_vprintf(struct tls *tls, const char buf = string; len = ret; while (len > 0) { - ret = tls_write(tls, buf, len); - if (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT) - continue; + ret = timeout_tls_write(fd, tls, buf, len); if (ret < 0) errx(1, "SSL write error: %s", tls_error(tls)); buf += ret; @@ -1608,9 +1690,7 @@ SSL_readline(struct tls *tls, size_t *le buf = q; len *= 2; } - do { - ret = tls_read(tls, &c, 1); - } while (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT); + ret = timeout_tls_read(tls, &c, 1); if (ret < 0) errx(1, "SSL read error: %s", tls_error(tls)); -- -- --------------------------------------------------------------------------------------------------------------------- Knowing is not enough; we must apply. Willing is not enough; we must do
