Fix stls_run; sbearssl_run needs a rewrite

Signed-off-by: Laurent Bercot <ska@appnovation.com>

6 files changed, 104 insertions, 101 deletions

diff --git a/src/sbearssl/sbearssl_run.c b/src/sbearssl/sbearssl_run.c
index d339b69..c5c60ec 100644
--- a/src/sbearssl/sbearssl_run.c
+++ b/src/sbearssl/sbearssl_run.c
@@ -25,22 +25,13 @@ static int br_ssl_engine_in_isempty (br_ssl_engine_context *ctx)
   return !ctx->iomode || (ctx->iomode == 3 && !ctx->ixa && !ctx->ixb) ;
 }
 
-static void close_sendrec (br_ssl_engine_context *ctx, int *fd, int closenotify)
-{
-  if (closenotify) br_ssl_engine_close(ctx) ;
-  else
-  {
-    fd_shutdown(*fd, 1) ;
-    fd_close(*fd) ;
-    *fd = -1 ;
-  }
-}
 
 void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto, uint32_t options, unsigned int verbosity, sbearssl_handshake_cbfunc_ref cb, sbearssl_handshake_cbarg *cbarg)
 {
   iopause_fd x[4] = { { .fd = fds[0], .revents = 0 }, { .fd = fds[1], .revents = 0 }, { .fd = fds[2] }, { .fd = fds[3] } } ;
   unsigned int state = br_ssl_engine_current_state(ctx) ;
   int handshake_done = 0 ;
+  int closing = 0 ;
   tain deadline ;
 
   if (ndelay_on(x[0].fd) == -1
@@ -66,10 +57,10 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
         deadline = tain_infinite ;
       }
     }
-    else x[0].events = 0 ;
+    else x[0].events = IOPAUSE_EXCEPT ;
 
     x[1].events = x[1].fd >= 0 ? IOPAUSE_EXCEPT | (state & BR_SSL_RECVAPP ? IOPAUSE_WRITE : 0) : 0 ;
-    x[2].events = x[2].fd >= 0 && state & BR_SSL_RECVREC) ? IOPAUSE_READ : 0 ;
+    x[2].events = x[2].fd >= 0 && state & BR_SSL_RECVREC ? IOPAUSE_READ : 0 ;
     x[3].events = x[3].fd >= 0 ? IOPAUSE_EXCEPT | (state & BR_SSL_SENDREC ? IOPAUSE_WRITE : 0) : 0 ;
 
 
@@ -86,7 +77,7 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
 
    /* Flush to local */
 
-    if (x[1].revents && state & BR_SSL_RECVAPP)
+    if (x[1].revents & IOPAUSE_WRITE)
     {
       size_t len ;
       for (;;)
@@ -109,10 +100,27 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
       }
       state = br_ssl_engine_current_state(ctx) ;
     }
+    else if (x[1].revents & IOPAUSE_EXCEPT)
+    {
+      fd_close(x[1].fd) ;
+      x[1].fd = -1 ;
+      if (x[2].fd >= 0)
+      {
+        fd_shutdown(x[2].fd, 0) ;
+        fd_close(x[2].fd) ;
+        x[2].fd = -1 ;
+        if (!br_ssl_engine_in_isempty(ctx))
+        {
+          br_ssl_engine_fail(ctx, BR_ERR_IO) ;
+          state = br_ssl_engine_current_state(ctx) ;
+        }
+      }
+    }
+
 
    /* Flush to remote */
 
-    if (x[3].revents && state & BR_SSL_SENDREC)
+    if (x[3].revents & IOPAUSE_WRITE)
     {
       size_t len ;
       for (;;)
@@ -129,9 +137,33 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
         br_ssl_engine_sendrec_ack(ctx, w) ;
       }
       if (x[0].fd == -1 && !len)
-        close_sendrec(ctx, &x[3].fd, options & 1) ;
+      {
+        if (options & 1 && !closing)
+        {
+          br_ssl_engine_close(ctx) ;
+          closing = 1 ;
+        }
+        else
+        {
+          fd_shutdown(x[3].fd, 1) ;
+          fd_close(x[3].fd) ;
+          x[3].fd = -1 ;
+        }
+      }
       state = br_ssl_engine_current_state(ctx) ;
     }
+    else if (x[3].revents & IOPAUSE_EXCEPT)
+    {
+      fd_shutdown(x[3].fd, 1) ;
+      fd_close(x[3].fd) ;
+      x[3].fd = -1 ;
+      if (x[0].fd >= 0)
+      {
+        fd_close(x[0].fd) ;
+        x[0].fd = -1 ;
+      }
+    }
+
 
    /* Fill from local */
 
@@ -154,7 +186,19 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
             fd_close(x[0].fd) ;
             x[0].fd = -1 ;
             if (!br_ssl_engine_sendrec_buf(ctx, &len))
-              close_sendrec(ctx, &x[3].fd, options & 1) ;
+            {
+              if (options & 1 && !closing)
+              {
+                br_ssl_engine_close(ctx) ;
+                closing = 1 ;
+              }
+              else
+              {
+                fd_shutdown(x[3].fd, 1) ;
+                fd_close(x[3].fd) ;
+                x[3].fd = -1 ;
+              }
+            }
           }
           break ;
         }
@@ -201,32 +245,6 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
       state = br_ssl_engine_current_state(ctx) ;
     }
 
-
-   /* Detect ill-timed broken pipes */
-
-    if (x[1].fd >= 0 && x[1].revents & IOPAUSE_EXCEPT && !(state & BR_SSL_RECVAPP))
-    {
-      fd_close(x[1].fd) ;
-      x[1].fd = -1 ;
-      if (x[2].fd >= 0)
-      {
-        fd_close(x[2].fd) ;
-        x[2].fd = -1 ;
-        if (!br_ssl_engine_in_isempty(ctx)) br_ssl_engine_fail(ctx, BR_ERR_IO) ;
-      }
-    }
-
-    if (x[3].fd >= 0 && x[3].revents & IOPAUSE_EXCEPT && !(state & BR_SSL_SENDREC))
-    {
-      fd_close(x[3].fd) ;
-      x[3].fd = -1 ;
-      if (x[0].fd >= 0)
-      {
-        fd_close(x[0].fd) ;
-        x[0].fd = -1 ;
-      }
-    }
-
   }  /* end of main loop */
 
   if (state & BR_SSL_CLOSED)
@@ -234,6 +252,5 @@ void sbearssl_run (br_ssl_engine_context *ctx, int const *fds, tain const *tto,
     int r = br_ssl_engine_last_error(ctx) ;
     if (r) strerr_dief4x(98, "the TLS engine closed the connection ", handshake_done ? "after" : "during", " the handshake: ", sbearssl_error_str(r)) ;
   }
-
   _exit(0) ;
 }
diff --git a/src/stls/stls_run.c b/src/stls/stls_run.c
index 2456e22..e56d123 100644
--- a/src/stls/stls_run.c
+++ b/src/stls/stls_run.c
@@ -21,7 +21,7 @@ struct stls_buffer_s
 {
   buffer b ;
   char buf[STLS_BUFSIZE] ;
-  uint8_t flags ;  /* 0x1: flush/fill wants opposite IO; 0x2: close_notify initiated */
+  uint8_t flags ;  /* 0x1: flush/fill wants opposite IO; 0x2: want close */
 } ;
 
 
@@ -121,18 +121,17 @@ static int tls_fill (struct tls *ctx, stls_buffer *b)
   r = tls_allread(ctx, v[1].iov_base, v[1].iov_len, &w) ;
   buffer_wseek(&b[1].b, w) ;
  out:
-  if (r == -1) return 1 ;
-  if (r) b[0].flags |= 1 ; else b[0].flags &= ~1 ;
-  return 0 ;
+  if (r == 1) b[0].flags |= 1 ; else b[0].flags &= ~1 ;
+  return r == -1 ;
 }
 
-static int tls_close_nb (struct tls *ctx, stls_buffer *b)
+static int tls_tryclose (struct tls *ctx, stls_buffer *b)
 {
   switch (tls_close(ctx))
   {
-    case 0 : b[0].flags &= ~2 ; b[1].flags &= ~2 ; b[1].flags |= 4 ; return 1 ;
-    case TLS_WANT_POLLIN : b[0].flags &= ~2 ; b[1].flags |= 2 ; break ;
-    case TLS_WANT_POLLOUT : b[0].flags |= 2 ; b[1].flags &= ~2 ; break ;
+    case 0 : b[0].flags &= ~2 ; return 1 ;
+    case TLS_WANT_POLLIN : b[1].flags |= 1 ; break ;
+    case TLS_WANT_POLLOUT : b[0].flags |= 2 ; break ;
     default : strerr_diefu2x(98, "tls_close: ", tls_error(ctx)) ;
   }
   return 0 ;
@@ -161,7 +160,7 @@ void stls_run (struct tls *ctx, int const *fds, uint32_t options, unsigned int v
     x[0].events = x[0].fd >= 0 && buffer_isreadable(&b[0].b) ? IOPAUSE_READ : 0 ;
     x[1].events = x[1].fd >= 0 && buffer_iswritable(&b[1].b) ? IOPAUSE_WRITE : 0 ;
     x[2].events = x[2].fd >= 0 && (buffer_isreadable(&b[1].b) || (b[1].flags & 1 && buffer_iswritable(&b[0].b))) ? IOPAUSE_READ : 0 ;
-    x[3].events = x[3].fd >= 0 && (buffer_iswritable(&b[0].b) || (b[0].flags & 1 && buffer_isreadable(&b[1].b))) ? IOPAUSE_WRITE : 0 ;
+    x[3].events = x[3].fd >= 0 && (buffer_iswritable(&b[0].b) || (b[0].flags & 1 && buffer_isreadable(&b[1].b)) || b[0].flags & 2) ? IOPAUSE_WRITE : 0 ;
 
     if (iopause_g(x, 4, 0) == -1) strerr_diefu1sys(111, "iopause") ;
 
@@ -187,23 +186,14 @@ void stls_run (struct tls *ctx, int const *fds, uint32_t options, unsigned int v
     if (x[3].revents)
     {
       if (buffer_len(&b[0].b)) tls_flush(ctx, b) ;  /* normal write */
-      if ((b[0].flags & 1 && tls_fill(ctx, b))  /* peer sent close_notify and it just completed */
-       || (b[0].flags & 2 && tls_close_nb(ctx, b)))  /*  we send close_notify and it instantly succeeds */
-      {
-        if (buffer_isempty(&b[1].b)) break ;
-        fd_close(x[3].fd) ; x[3].fd = -1 ;
-        fd_close(x[2].fd) ; x[2].fd = -1 ;
-        if (x[0].fd >= 0) { fd_close(x[0].fd) ; x[0].fd = -1 ; }
-        continue ;
-      }
-      if (x[0].fd == -1 && buffer_isempty(&b[0].b))
+      if (b[0].flags & 1 && tls_fill(ctx, b))
+        strerr_dief1x(98, "tls_read returned 0 during a renegotiation?") ;
+      if (x[0].fd == -1 && buffer_isempty(&b[0].b)
+       && (!(options & 1) || tls_tryclose(ctx, b)))
       {
-        if (!(options & 1) || tls_close_nb(ctx, b))
-        {
-          fd_shutdown(x[3].fd, 1) ;
-          fd_close(x[3].fd) ;
-          x[3].fd = -1 ;
-        }
+        fd_shutdown(x[3].fd, 1) ;
+        fd_close(x[3].fd) ;
+        x[3].fd = -1 ;
       }
     }
 
@@ -221,7 +211,7 @@ void stls_run (struct tls *ctx, int const *fds, uint32_t options, unsigned int v
         x[0].fd = -1 ;
         if (buffer_isempty(&b[0].b))
         {
-          if (!(options & 1) || tls_close_nb(ctx, b))
+          if (!(options & 1) || tls_tryclose(ctx, b))
           {
             fd_shutdown(x[3].fd, 1) ;
             fd_close(x[3].fd) ;
@@ -238,38 +228,24 @@ void stls_run (struct tls *ctx, int const *fds, uint32_t options, unsigned int v
     {
       if (buffer_isreadable(&b[1].b) && tls_fill(ctx, b))
       {  /* connection closed */
+        if (options & 2 && !tls_eof_got_close_notify(ctx))
+          strerr_dief1x(98, "remote closed connection without a close_notify") ;
         fd_shutdown(x[2].fd, 0) ;
         fd_close(x[2].fd) ;
         x[2].fd = -1 ;
         if (buffer_isempty(&b[1].b))
         {
-          if (tls_eof_got_close_notify(ctx)) break ;
           fd_close(x[1].fd) ;
           x[1].fd = -1 ;
         }
-        if (options & 2)
+        if (x[3].fd >= 0 && options & 1 && tls_tryclose(ctx, b))
         {
-          if (!tls_eof_got_close_notify(ctx))
-            strerr_dief1x(98, "remote closed connection without a close_notify") ;
-          else if (x[3].fd >= 0)
-          {
-            fd_shutdown(x[3].fd, 1) ;
-            fd_close(x[3].fd) ;
-            x[3].fd = -1 ;
-          }
-        }
-      }
-      else
-      {  /* normal case */
-        if (b[1].flags & 1) tls_flush(ctx, b) ;
-        if (b[1].flags & 2 && tls_close_nb(ctx, b))
-        {
-          if (buffer_isempty(&b[1].b)) break ;
-          if (x[3].fd >= 0) { fd_close(x[3].fd) ; x[3].fd = -1 ; }
-          if (x[0].fd >= 0) { fd_close(x[0].fd) ; x[0].fd = -1 ; }
-          fd_close(x[2].fd) ; x[2].fd = -1 ;
+          fd_shutdown(x[3].fd, 1) ;
+          fd_close(x[3].fd) ;
+          x[3].fd = -1 ;
         }
       }
+      else if (b[1].flags & 1) tls_flush(ctx, b) ;
     }
   }
   _exit(0) ;
diff --git a/src/tls/s6-tlsclient.c b/src/tls/s6-tlsclient.c
index 261c8be..888c11a 100644
--- a/src/tls/s6-tlsclient.c
+++ b/src/tls/s6-tlsclient.c
@@ -13,7 +13,7 @@
 
 #define USAGE "s6-tlsclient [ options ] host port prog...\n" \
 "s6-tcpclient options: [ -q | -Q | -v ] [ -4 | -6 ] [ -d | -D ] [ -r | -R ] [ -h | -H ] [ -n | -N ] [ -t timeout ] [ -l localname ] [ -T timeoutconn ] [ -i localip ] [ -p localport ]\n" \
-"s6-tlsc options: [ -S | -s ] [ -Y | -y ] [ -K timeout ] [ -k servername ] [ -Z | -z ]"
+"s6-tlsc options: [ -S | -s ] [ -J | -j ] [ -Y | -y ] [ -K timeout ] [ -k servername ] [ -Z | -z ]"
 
 #define dieusage() strerr_dieusage(100, USAGE)
 
@@ -36,6 +36,7 @@ struct options_s
   unsigned int flagr : 1 ;
   unsigned int flagN : 1 ;
   unsigned int flagS : 1 ;
+  unsigned int flagJ : 1 ;
   unsigned int flagy : 1 ;
   unsigned int flagZ : 1 ;
   unsigned int doxy : 1 ;
@@ -59,6 +60,7 @@ struct options_s
   .flagr = 0, \
   .flagN = 0, \
   .flagS = 0, \
+  .flagJ = 0, \
   .flagy = 0, \
   .flagZ = 0, \
   .doxy = 0 \
@@ -72,7 +74,7 @@ int main (int argc, char const *const *argv)
     subgetopt l = SUBGETOPT_ZERO ;
     for (;;)
     {
-      int opt = subgetopt_r(argc, argv, "qQv46DdHhRrnNt:l:T:i:p:SsYyK:k:Zz", &l) ;
+      int opt = subgetopt_r(argc, argv, "qQv46DdHhRrnNt:l:T:i:p:SsJjYyK:k:Zz", &l) ;
       if (opt == -1) break ;
       switch (opt)
       {
@@ -109,6 +111,8 @@ int main (int argc, char const *const *argv)
         case 'p' : if (!uint160_scan(l.arg, &o.localport)) dieusage() ; break ;
         case 'S' : o.flagS = 1 ; break ;
         case 's' : o.flagS = 0 ; break ;
+        case 'J' : o.flagJ = 1 ; break ;
+        case 'j' : o.flagJ = 0 ; break ;
         case 'Y' : o.flagy = 0 ; break ;
         case 'y' : o.flagy = 1 ; break ;
         case 'K' : if (!uint0_scan(l.arg, &o.kimeout)) dieusage() ; break ;
@@ -133,7 +137,7 @@ int main (int argc, char const *const *argv)
     size_t pos = 0 ;
     unsigned int m = 0 ;
     char fmt[UINT_FMT * 4 + UINT16_FMT + IP46_FMT] ;
-    char const *newargv[31 + argc] ;
+    char const *newargv[32 + argc] ;
     newargv[m++] = S6_NETWORKING_BINPREFIX "s6-tcpclient" ;
     if (o.verbosity != 1) newargv[m++] = o.verbosity ? "-v" : "-q" ;
     if (o.flag4) newargv[m++] = "-4" ;
@@ -182,6 +186,7 @@ int main (int argc, char const *const *argv)
     newargv[m++] = *argv++ ;
     newargv[m++] = S6_NETWORKING_BINPREFIX "s6-tlsc" ;
     if (o.flagS) newargv[m++] = "-S" ;
+    if (o.flagJ) newargv[m++] = "-J" ;
     if (o.flagy) newargv[m++] = "-y" ;
     if (o.kimeout)
     {
diff --git a/src/tls/s6-tlsserver.c b/src/tls/s6-tlsserver.c
index be96f39..0c38600 100644
--- a/src/tls/s6-tlsserver.c
+++ b/src/tls/s6-tlsserver.c
@@ -15,7 +15,7 @@
 #define USAGE "s6-tlsserver [ options ] ip port prog...\n" \
 "s6-tcpserver options: [ -q | -Q | -v ] [ -1 ] [ -c maxconn ] [ -C localmaxconn ] [ -b backlog ] [ -G gidlist ] [ -g gid ] [ -u uid ] [ -U ]\n" \
 "s6-tcpserver-access options: [ -W | -w ] [ -D | -d ] [ -H | -h ] [ -R | -r ] [ -P | -p ] [ -l localname ] [ -B banner ] [ -t timeout ] [ -i rulesdir | -x rulesfile ]\n" \
-"s6-tlsd options: [ -S | -s ] [ -Y | -y ] [ -K timeout ] [ -Z | -z ] [ -k snilevel ]"
+"s6-tlsd options: [ -S | -s ] [ -J | -j ] [ -Y | -y ] [ -K timeout ] [ -Z | -z ] [ -k snilevel ]"
 
 #define dieusage() strerr_dieusage(100, USAGE)
 
@@ -45,6 +45,7 @@ struct options_s
   unsigned int flagp : 1 ;
   unsigned int rulesx : 1 ;
   unsigned int flagS : 1 ;
+  unsigned int flagJ : 1 ;
   unsigned int flagy : 1 ;
   unsigned int flagY : 1 ;
   unsigned int flagZ : 1 ;
@@ -75,6 +76,7 @@ struct options_s
   .flagp = 0, \
   .rulesx = 0, \
   .flagS = 0, \
+  .flagJ = 0, \
   .flagy = 0, \
   .flagY = 0, \
   .flagZ = 0, \
@@ -89,7 +91,7 @@ int main (int argc, char const *const *argv)
     subgetopt l = SUBGETOPT_ZERO ;
     for (;;)
     {
-      int opt = subgetopt_r(argc, argv, "qQv1c:C:b:G:g:u:UWwDdHhRrPpl:B:t:i:x:SsYyK:Zzk:", &l) ;
+      int opt = subgetopt_r(argc, argv, "qQv1c:C:b:G:g:u:UWwDdHhRrPpl:B:t:i:x:SsJjYyK:Zzk:", &l) ;
       if (opt == -1) break ;
       switch (opt)
       {
@@ -121,6 +123,8 @@ int main (int argc, char const *const *argv)
         case 'x' : o.rules = l.arg ; o.rulesx = 1 ; break ;
         case 'S' : o.flagS = 1 ; break ;
         case 's' : o.flagS = 0 ; break ;
+        case 'J' : o.flagJ = 1 ; break ;
+        case 'j' : o.flagJ = 0 ; break ;
         case 'Y' : o.flagY = 1 ; o.flagy = 0 ; break ;
         case 'y' : o.flagy = 1 ; o.flagY = 0 ; break ;
         case 'K' : if (!uint0_scan(l.arg, &o.kimeout)) dieusage() ; break ;
@@ -138,7 +142,7 @@ int main (int argc, char const *const *argv)
     size_t pos = 0 ;
     unsigned int m = 0 ;
     char fmt[UINT_FMT * 6 + UID_FMT + GID_FMT * (NGROUPS_MAX + 1)] ;
-    char const *newargv[49 + argc] ;
+    char const *newargv[50 + argc] ;
     int doaccess = o.flagw || o.flagD || !o.flagH || o.flagr || o.flagp || o.localname || o.banner || o.timeout || o.rules ;
     newargv[m++] = S6_NETWORKING_BINPREFIX "s6-tcpserver" ;
     if (o.verbosity != 1)
@@ -216,6 +220,7 @@ int main (int argc, char const *const *argv)
       newargv[m++] = fmt ;
     }
     if (o.flagS) newargv[m++] = "-S" ;
+    if (o.flagJ) newargv[m++] = "-J" ;
     if (o.flagy) newargv[m++] = "-y" ;
     else if (o.flagY) newargv[m++] = "-Y" ;
     if (o.kimeout)
diff --git a/src/tls/s6tls_prep_tlscio.c b/src/tls/s6tls_prep_tlscio.c
index 0b7ff1f..f56106f 100644
--- a/src/tls/s6tls_prep_tlscio.c
+++ b/src/tls/s6tls_prep_tlscio.c
@@ -25,9 +25,9 @@ void s6tls_prep_tlscio (char const **argv, char *buf, int const *p, uint32_t opt
     n += uint_fmt(buf + n, p[5]) ;
     buf[n++] = 0 ;
   }
-  argv[m++] = options & 4 ? "-S" : "-s" ;
-  argv[m++] = options & 2 ? "-J" : "-j" ;
-  argv[m++] = options & 1 ? "-y" : "-Y" ;
+  if (options & 4) argv[m++] = "-S" ;
+  if (options & 2) argv[m++] = "-J" ;
+  if (options & 1) argv[m++] = "-y" ;
   if (kimeout)
   {
     argv[m++] = "-K" ;
diff --git a/src/tls/s6tls_prep_tlsdio.c b/src/tls/s6tls_prep_tlsdio.c
index 59cc536..8408010 100644
--- a/src/tls/s6tls_prep_tlsdio.c
+++ b/src/tls/s6tls_prep_tlsdio.c
@@ -25,8 +25,8 @@ void s6tls_prep_tlsdio (char const **argv, char *buf, int const *p, uint32_t opt
     n += uint_fmt(buf + n, p[5]) ;
     buf[n++] = 0 ;
   }
-  argv[m++] = options & 4 ? "-S" : "-s" ;
-  argv[m++] = options & 8 ? "-J" : "-j" ;
+  if (options & 4) argv[m++] = "-S" ;
+  if (options & 8) argv[m++] = "-J" ;
   if (options & 1)
     argv[m++] = options & 2 ? "-y" : "-Y" ;
   if (kimeout)