[v9] Backport #11616, #11714, and #12499

lib/srv/desktop/rdp/rdpclient/client.go

@@ -66,7 +66,6 @@ import "C"
 import (
 	"context"
 	"errors"
-	"fmt"
 	"image"
 	"io"
 	"os"
@@ -242,8 +241,8 @@ func (c *Client) connect(ctx context.Context) error {
 		C.uint16_t(c.clientHeight),
 		C.bool(c.cfg.AllowClipboard),
 	)
-	if err := cgoError(res.err); err != nil {
-		return trace.Wrap(err)
+	if res.err != C.ErrCodeSuccess {
+		return trace.ConnectionProblem(nil, "RDP connection failed")
 	}
 	c.rustClient = res.client
 	return nil
@@ -260,7 +259,7 @@ func (c *Client) start() {
 
 		// C.read_rdp_output blocks for the duration of the RDP connection and
 		// calls handle_bitmap repeatedly with the incoming bitmaps.
-		if err := cgoError(C.read_rdp_output(c.rustClient)); err != nil {
+		if err := C.read_rdp_output(c.rustClient); err != C.ErrCodeSuccess {
 			c.cfg.Log.Warningf("Failed reading RDP output frame: %v", err)
 
 			// close the TDP connection to the browser
@@ -297,16 +296,15 @@ func (c *Client) start() {
 			switch m := msg.(type) {
 			case tdp.MouseMove:
 				mouseX, mouseY = m.X, m.Y
-				if err := cgoError(C.write_rdp_pointer(
+				if err := C.write_rdp_pointer(
 					c.rustClient,
 					C.CGOMousePointerEvent{
 						x:      C.uint16_t(m.X),
 						y:      C.uint16_t(m.Y),
 						button: C.PointerButtonNone,
 						wheel:  C.PointerWheelNone,
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP mouse pointer: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.MouseButton:
@@ -322,7 +320,7 @@ func (c *Client) start() {
 				default:
 					button = C.PointerButtonNone
 				}
-				if err := cgoError(C.write_rdp_pointer(
+				if err := C.write_rdp_pointer(
 					c.rustClient,
 					C.CGOMousePointerEvent{
 						x:      C.uint16_t(mouseX),
@@ -331,8 +329,7 @@ func (c *Client) start() {
 						down:   m.State == tdp.ButtonPressed,
 						wheel:  C.PointerWheelNone,
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP mouse button: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.MouseWheel:
@@ -351,7 +348,7 @@ func (c *Client) start() {
 				default:
 					wheel = C.PointerWheelNone
 				}
-				if err := cgoError(C.write_rdp_pointer(
+				if err := C.write_rdp_pointer(
 					c.rustClient,
 					C.CGOMousePointerEvent{
 						x:           C.uint16_t(mouseX),
@@ -360,29 +357,26 @@ func (c *Client) start() {
 						wheel:       uint32(wheel),
 						wheel_delta: C.int16_t(m.Delta),
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP mouse wheel: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.KeyboardButton:
-				if err := cgoError(C.write_rdp_keyboard(
+				if err := C.write_rdp_keyboard(
 					c.rustClient,
 					C.CGOKeyboardEvent{
 						code: C.uint16_t(m.KeyCode),
 						down: m.State == tdp.ButtonPressed,
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP key press: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.ClipboardData:
 				if len(m) > 0 {
-					if err := cgoError(C.update_clipboard(
+					if err := C.update_clipboard(
 						c.rustClient,
 						(*C.uint8_t)(unsafe.Pointer(&m[0])),
 						C.uint32_t(len(m)),
-					)); err != nil {
-						c.cfg.Log.Warningf("Failed forwarding RDP clipboard data: %v", err)
+					); err != C.ErrCodeSuccess {
 						return
 					}
 				} else {
@@ -396,11 +390,11 @@ func (c *Client) start() {
 }
 
 //export handle_bitmap
-func handle_bitmap(handle C.uintptr_t, cb *C.CGOBitmap) C.CGOError {
+func handle_bitmap(handle C.uintptr_t, cb *C.CGOBitmap) C.CGOErrCode {
 	return cgo.Handle(handle).Value().(*Client).handleBitmap(cb)
 }
 
-func (c *Client) handleBitmap(cb *C.CGOBitmap) C.CGOError {
+func (c *Client) handleBitmap(cb *C.CGOBitmap) C.CGOErrCode {
 	// Notify the input forwarding goroutine that we're ready for input.
 	// Input can only be sent after connection was established, which we infer
 	// from the fact that a bitmap was sent.
@@ -429,35 +423,38 @@ func (c *Client) handleBitmap(cb *C.CGOBitmap) C.CGOError {
 	copy(img.Pix, data)
 
 	if err := c.cfg.Conn.OutputMessage(tdp.NewPNG(img, c.cfg.Encoder)); err != nil {
-		return C.CString(fmt.Sprintf("failed to send PNG frame %v: %v", img.Rect, err))
+		c.cfg.Log.Errorf("failed to send PNG frame %v: %v", img.Rect, err)
+		return C.ErrCodeFailure
 	}
-	return nil
+	return C.ErrCodeSuccess
 }
 
 //export handle_remote_copy
-func handle_remote_copy(handle C.uintptr_t, data *C.uint8_t, length C.uint32_t) C.CGOError {
+func handle_remote_copy(handle C.uintptr_t, data *C.uint8_t, length C.uint32_t) C.CGOErrCode {
 	goData := C.GoBytes(unsafe.Pointer(data), C.int(length))
 	return cgo.Handle(handle).Value().(*Client).handleRemoteCopy(goData)
 }
 
 // handleRemoteCopy is called from Rust when data is copied
 // on the remote desktop
-func (c *Client) handleRemoteCopy(data []byte) C.CGOError {
+func (c *Client) handleRemoteCopy(data []byte) C.CGOErrCode {
 	c.cfg.Log.Debugf("Received %d bytes of clipboard data from Windows desktop", len(data))
 
 	if err := c.cfg.Conn.OutputMessage(tdp.ClipboardData(data)); err != nil {
-		return C.CString(fmt.Sprintf("failed to send clipboard data: %v", err))
+		c.cfg.Log.Errorf("failed handling remote copy: %v", err)
+		return C.ErrCodeFailure
 	}
-	return nil
+	return C.ErrCodeSuccess
 }
 
 // close frees the memory of the cgo.Handle,
 // closes the RDP client connection,
 // and frees the Rust client.
 func (c *Client) close() {
 	c.closeOnce.Do(func() {
-		// Close the RDP client
-		if err := cgoError(C.close_rdp(c.rustClient)); err != nil {
+		c.handle.Delete()
+
+		if err := C.close_rdp(c.rustClient); err != C.ErrCodeSuccess {
 			c.cfg.Log.Warningf("failed to close the RDP client")
 		}
 
@@ -484,19 +481,3 @@ func (c *Client) UpdateClientActivity() {
 	c.clientLastActive = time.Now().UTC()
 	c.clientActivityMu.Unlock()
 }
-
-// cgoError converts from a CGO-originated error to a Go error, copying the
-// error string and releasing the CGO data.
-func cgoError(s C.CGOError) error {
-	if s == nil {
-		return nil
-	}
-	gs := C.GoString(s)
-	C.free_rust_string(s)
-	return errors.New(gs)
-}
-
-//export free_go_string
-func free_go_string(s *C.char) {
-	C.free(unsafe.Pointer(s))
-}

lib/srv/desktop/rdp/rdpclient/librdprs.h

@@ -20,6 +20,11 @@
  */
 #define CHANNEL_CHUNK_LEGNTH 1600
 
+typedef enum CGOErrCode {
+  ErrCodeSuccess = 0,
+  ErrCodeFailure = 1,
+} CGOErrCode;
+
 typedef enum CGOPointerButton {
   PointerButtonNone,
   PointerButtonLeft,
@@ -46,14 +51,9 @@ typedef enum CGOPointerWheel {
  */
 typedef struct Client Client;
 
-/**
- * CGOError is an alias for a C string pointer, for C API clarity.
- */
-typedef char *CGOError;
-
 typedef struct ClientOrError {
   struct Client *client;
-  CGOError err;
+  enum CGOErrCode err;
 } ClientOrError;
 
 /**
@@ -126,7 +126,7 @@ struct ClientOrError connect_rdp(uintptr_t go_ref,
  *
  * `client_ptr` must be a valid pointer to a Client.
  */
-CGOError update_clipboard(struct Client *client_ptr, uint8_t *data, uint32_t len);
+enum CGOErrCode update_clipboard(struct Client *client_ptr, uint8_t *data, uint32_t len);
 
 /**
  * `read_rdp_output` reads incoming RDP bitmap frames from client at client_ref and forwards them to
@@ -137,28 +137,28 @@ CGOError update_clipboard(struct Client *client_ptr, uint8_t *data, uint32_t len
  * `client_ptr` must be a valid pointer to a Client.
  * `handle_bitmap` *must not* free the memory of CGOBitmap.
  */
-CGOError read_rdp_output(struct Client *client_ptr);
+enum CGOErrCode read_rdp_output(struct Client *client_ptr);
 
 /**
  * # Safety
  *
  * client_ptr must be a valid pointer to a Client.
  */
-CGOError write_rdp_pointer(struct Client *client_ptr, struct CGOMousePointerEvent pointer);
+enum CGOErrCode write_rdp_pointer(struct Client *client_ptr, struct CGOMousePointerEvent pointer);
 
 /**
  * # Safety
  *
  * client_ptr must be a valid pointer to a Client.
  */
-CGOError write_rdp_keyboard(struct Client *client_ptr, struct CGOKeyboardEvent key);
+enum CGOErrCode write_rdp_keyboard(struct Client *client_ptr, struct CGOKeyboardEvent key);
 
 /**
  * # Safety
  *
  * client_ptr must be a valid pointer to a Client.
  */
-CGOError close_rdp(struct Client *client_ptr);
+enum CGOErrCode close_rdp(struct Client *client_ptr);
 
 /**
  * free_rdp lets the Go side inform us when it's done with Client and it can be dropped.
@@ -176,8 +176,6 @@ void free_rdp(struct Client *client_ptr);
  */
 void free_rust_string(char *s);
 
-extern void free_go_string(char *s);
-
-extern CGOError handle_bitmap(uintptr_t client_ref, struct CGOBitmap *b);
+extern enum CGOErrCode handle_bitmap(uintptr_t client_ref, struct CGOBitmap *b);
 
-extern CGOError handle_remote_copy(uintptr_t client_ref, uint8_t *data, uint32_t len);
+extern enum CGOErrCode handle_remote_copy(uintptr_t client_ref, uint8_t *data, uint32_t len);