[PATCH] channel: is_local_eof|closed getters

[Jon Simons <jon@xxxxxxxxxxxxx>]

Hi,

Attached is a patch to enable ensuring that channels are truly closed after
receiving SSH_AGAIN from 'ssh_channel_close(c)'.

The problem addressed is that, given an 'ssh_channel_close(c)' that returns
SSH_AGAIN, what should be done to ensure that a CLOSE message has really been
queued to be sent back to the other side?

With this patch I believe a loop like this now ensures a proper close:

  int rc = ssh_channel_close(c);
  while (rc == SSH_AGAIN) {
    if (!ssh_channel_is_local_closed(c)) {
      rc = ssh_channel_close(c);
    } else {
      rc = ssh_blocking_flush(session, -1);
    }
  }

Before the patch, it is not possible to know whether the first channel_close
has maybe only enqueued and sent EOF, and no CLOSE message in the outgoing
session buffer, or whether a CLOSE message was enqueued but hit SSH_AGAIN
during its flush.  If a CLOSE message was enqueued but the caller issues
ssh_channel_close again, a double-close can be sent to the other side.


-Jon

From ae6f9b44fac93da95902ca1da99e5141d71a7099 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Mon, 3 Mar 2014 17:52:32 -0800
Subject: [PATCH] channel: is_local_eof|closed getters

Given an invocation to 'ssh_channel_close', upon SSH_AGAIN, there
is currently no way to know whether the SSH_AGAIN means that EOF
has been totally sent and CLOSE message bytes have been enqueued,
or whether sending and flushing EOF needs to be retried, and CLOSE
message bytes are yet to be queued.

Expose getters for 'local_eof' and the new field 'local_closed'.
Until the latter is set, it is not the case the CLOSE message
bytes have been enqueued to the outgoing session socket.

Motivation: when unable to distinguish the above ssh_channel_close
cases, clients either have to resort to missing a channel close, or
possibly sending more than one CLOSE message for a given channel.
Both of those options are bad for the case of session multiplexing
(for example, OpenSSH might indefinitely hang if it never sees a
corresponding CLOSE response for a closed channel; likewise, it
may abruptly disconnect if it receives a duplicate CLOSE or a
CLOSE for an unknown channel).

Signed-off-by: Jon Simons <jon@xxxxxxxxxxxxx>
---
 include/libssh/channels.h |  4 ++++
 src/channels.c            | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/libssh/channels.h b/include/libssh/channels.h
index 4515223..517e519 100644
--- a/include/libssh/channels.h
+++ b/include/libssh/channels.h
@@ -59,6 +59,7 @@ struct ssh_channel_struct {
     uint32_t local_channel;
     uint32_t local_window;
     int local_eof;
+    int local_close;
     uint32_t local_maxpacket;
 
     uint32_t remote_channel;
@@ -115,4 +116,7 @@ int channel_write1(ssh_channel channel, const void *data, int len);
 ssh_channel ssh_get_channel1(ssh_session session);
 #endif
 
+int ssh_channel_is_local_eof(ssh_channel c);
+int ssh_channel_is_local_closed(ssh_channel c);
+
 #endif /* CHANNELS_H_ */
diff --git a/src/channels.c b/src/channels.c
index 51e96fe..7fc95c0 100644
--- a/src/channels.c
+++ b/src/channels.c
@@ -117,6 +117,14 @@ ssh_channel ssh_channel_new(ssh_session session) {
   return channel;
 }
 
+int ssh_channel_is_local_eof(ssh_channel c) {
+	return c->local_eof;
+}
+
+int ssh_channel_is_local_closed(ssh_channel c) {
+	return c->local_close;
+}
+
 /**
  * @internal
  *
@@ -1223,6 +1231,8 @@ int ssh_channel_close(ssh_channel channel){
     channel->state=SSH_CHANNEL_STATE_CLOSED;
   }
 
+  channel->local_close = 1;
+
   rc = ssh_channel_flush(channel);
   if(rc == SSH_ERROR)
     goto error;
-- 
1.8.4.21.g992c386