ssh: make channel.close idempotent

channel.close closes the internal Go channels (msg, incomingRequests);
calling it twice panics with "close of closed channel". Both current
callers run on the mux loop and are serialized, so the panic is not
reachable today. Harden close by guarding it with sync.Once, so that
the connection does not become fragile to any future bug that reaches
close twice.

Change-Id: I5d8d948367538ea5561724033c4ee4daa668e221
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797060
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>

ssh/channel.go

@@ -216,6 +216,10 @@ type channel struct {
 	// packetPool has a buffer for each extended channel ID to
 	// save allocations during writes.
 	packetPool map[uint32][]byte
+
+	// closeOnce guards close so it is idempotent: closing the internal Go
+	// channels (msg, incomingRequests) more than once would panic.
+	closeOnce sync.Once
 }
 
 // writePacket sends a packet. If the packet is a channel close, it updates
@@ -393,17 +397,19 @@ func (c *channel) ReadExtended(data []byte, extended uint32) (n int, err error)
 }
 
 func (c *channel) close() {
-	c.pending.eof()
-	c.extPending.eof()
-	close(c.msg)
-	close(c.incomingRequests)
-	c.writeMu.Lock()
-	// This is not necessary for a normal channel teardown, but if
-	// there was another error, it is.
-	c.sentClose = true
-	c.writeMu.Unlock()
-	// Unblock writers.
-	c.remoteWin.close()
+	c.closeOnce.Do(func() {
+		c.pending.eof()
+		c.extPending.eof()
+		close(c.msg)
+		close(c.incomingRequests)
+		c.writeMu.Lock()
+		// This is not necessary for a normal channel teardown, but if
+		// there was another error, it is.
+		c.sentClose = true
+		c.writeMu.Unlock()
+		// Unblock writers.
+		c.remoteWin.close()
+	})
 }
 
 // responseMessageReceived is called when a success or failure message is

ssh/mux_test.go

@@ -1213,6 +1213,18 @@ func TestChannelUnexpectedResponsesDiscarded(t *testing.T) {
 	}
 }
 
+func TestChannelCloseIdempotent(t *testing.T) {
+	a, b := memPipe()
+	defer a.Close()
+	defer b.Close()
+
+	m := newMux(a)
+	ch := m.newChannel("session", channelOutbound, nil)
+
+	ch.close()
+	ch.close() // must be a no-op, not a panic
+}
+
 func TestChannelConcurrentRequests(t *testing.T) {
 	writer, reader, mux := channelPair(t)
 	defer writer.Close()