Send unsupported error on extended packets.

Following the rules outlined here:

https://tools.ietf.org/html/draft-ietf-secsh-filexfer-extensions-00

Return an SSH_FXP_STATUS with appropriate status error for extended
packets that we do not support.

packet-typing.go

@@ -127,7 +127,9 @@ func makePacket(p rxPacket) (requestPacket, error) {
 		return nil, errors.Errorf("unhandled packet type: %s", p.pktType)
 	}
 	if err := pkt.UnmarshalBinary(p.pktBytes); err != nil {
-		return nil, err
+		// Return partially unpacked packet to allow callers to return
+		// error messages appropriately with necessary id() method.
+		return pkt, err
 	}
 	return pkt, nil
 }

packet.go

@@ -874,10 +874,18 @@ type sshFxpExtendedPacket struct {
 	}
 }
 
-func (p sshFxpExtendedPacket) id() uint32     { return p.ID }
-func (p sshFxpExtendedPacket) readonly() bool { return p.SpecificPacket.readonly() }
+func (p sshFxpExtendedPacket) id() uint32 { return p.ID }
+func (p sshFxpExtendedPacket) readonly() bool {
+	if p.SpecificPacket == nil {
+		return true
+	}
+	return p.SpecificPacket.readonly()
+}
 
 func (p sshFxpExtendedPacket) respond(svr *Server) error {
+	if p.SpecificPacket == nil {
+		return nil
+	}
 	return p.SpecificPacket.respond(svr)
 }
 
@@ -897,7 +905,7 @@ func (p *sshFxpExtendedPacket) UnmarshalBinary(b []byte) error {
 	case "[email protected]":
 		p.SpecificPacket = &sshFxpExtendedPacketPosixRename{}
 	default:
-		return errUnknownExtendedPacket
+		return errors.Wrapf(errUnknownExtendedPacket, "packet type %v", p.SpecificPacket)
 	}
 
 	return p.SpecificPacket.UnmarshalBinary(bOrig)

request-server.go

@@ -128,9 +128,14 @@ func (rs *RequestServer) Serve() error {
 
 		pkt, err = makePacket(rxPacket{fxp(pktType), pktBytes})
 		if err != nil {
-			debug("makePacket err: %v", err)
-			rs.conn.Close() // shuts down recvPacket
-			break
+			switch errors.Cause(err) {
+			case errUnknownExtendedPacket:
+				rs.serverConn.sendError(pkt, ErrSshFxOpUnsupported)
+			default:
+				debug("makePacket err: %v", err)
+				rs.conn.Close() // shuts down recvPacket
+				break
+			}
 		}
 
 		pktChan <- pkt

server.go

@@ -134,7 +134,7 @@ func (svr *Server) sftpServerWorker(pktChan chan requestPacket) error {
 		case *sshFxpOpenPacket:
 			readonly = pkt.readonly()
 		case *sshFxpExtendedPacket:
-			readonly = pkt.SpecificPacket.readonly()
+			readonly = pkt.readonly()
 		}
 
 		// If server is operating read-only and a write operation is requested,
@@ -304,9 +304,18 @@ func (svr *Server) Serve() error {
 
 		pkt, err = makePacket(rxPacket{fxp(pktType), pktBytes})
 		if err != nil {
-			debug("makePacket err: %v", err)
-			svr.conn.Close() // shuts down recvPacket
-			break
+			switch errors.Cause(err) {
+			case errUnknownExtendedPacket:
+				if err := svr.serverConn.sendError(pkt, ErrSshFxOpUnsupported); err != nil {
+					debug("failed to send err packet: %v", err)
+					svr.conn.Close() // shuts down recvPacket
+					break
+				}
+			default:
+				debug("makePacket err: %v", err)
+				svr.conn.Close() // shuts down recvPacket
+				break
+			}
 		}
 
 		pktChan <- pkt

server_test.go

@@ -198,24 +198,29 @@ func (p sshFxpTestBadExtendedPacket) MarshalBinary() ([]byte, error) {
 }
 
 // test that errors are sent back when we request an invalid extended packet operation
+// this validates the following rfc draft is followed https://tools.ietf.org/html/draft-ietf-secsh-filexfer-extensions-00
 func TestInvalidExtendedPacket(t *testing.T) {
 	client, server := clientServerPair(t)
 	defer client.Close()
 	defer server.Close()
 
 	badPacket := sshFxpTestBadExtendedPacket{client.nextID(), "thisDoesn'tExist", "foobar"}
-	_, _, err := client.clientConn.sendPacket(badPacket)
-	if err == nil {
-		t.Fatal("expected error from bad packet")
+	typ, data, err := client.clientConn.sendPacket(badPacket)
+	if err != nil {
+		t.Fatalf("unexpected error from sendPacket: %s", err)
 	}
-
-	// try to stat a file; the client should have shut down.
-	filePath := "/etc/passwd"
-	_, err = client.Stat(filePath)
-	if err == nil {
-		t.Fatal("expected error from closed connection")
+	if typ != ssh_FXP_STATUS {
+		t.Fatalf("received non-FPX_STATUS packet: %v", typ)
 	}
 
+	err = unmarshalStatus(badPacket.id(), data)
+	statusErr, ok := err.(*StatusError)
+	if !ok {
+		t.Fatal("failed to convert error from unmarshalStatus to *StatusError")
+	}
+	if statusErr.Code != ssh_FX_OP_UNSUPPORTED {
+		t.Errorf("statusErr.Code => %d, wanted %d", statusErr.Code, ssh_FX_OP_UNSUPPORTED)
+	}
 }
 
 // test that server handles concurrent requests correctly