provide a human-readable error message with non-(200|401|404) server …

…response codes (#104)

The server now attaches a response body with its error responses that are different from 404 and 401.

This allows providing better information to the client and help them debug their setup.

cmd/ssh3/main.go

@@ -612,7 +612,7 @@ func mainWithStatusCode() int {
 
 	c, err := client.Dial(ctx, options, qconn, roundTripper, agentClient)
 	if err != nil {
-		log.Error().Msgf("could not establish SSH3 conversation: %s", err)
+		log.Error().Msgf("could not dial %s: %s", options.CanonicalHostFormat(), err)
 		return -1
 	}
 	if localTCPAddr != nil && remoteTCPAddr != nil {

conversation.go

@@ -6,6 +6,7 @@ import (
 	"crypto/tls"
 	"encoding/base64"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 
@@ -162,7 +163,17 @@ func (c *Conversation) EstablishClientConversation(req *http.Request, roundTripp
 	} else if rsp.StatusCode == http.StatusUnauthorized {
 		return util.Unauthorized{}
 	} else {
-		return fmt.Errorf("returned non-200 and non-401 status code: %d", rsp.StatusCode)
+		bodyContent, err := io.ReadAll(rsp.Body)
+		rsp.Body.Close()
+		if err != nil {
+			log.Error().Msgf("could not read response body from server: %s", err)
+		}
+
+		return util.OtherHTTPError{
+			HasBody:    rsp.ContentLength > 0,
+			Body:       string(bodyContent),
+			StatusCode: rsp.StatusCode,
+		}
 	}
 }
 

unix_server/auth.go

@@ -21,20 +21,21 @@ func HandleAuths(ctx context.Context, enablePasswordLogin bool, defaultMaxPacket
 		return nil, fmt.Errorf("password login not supported on %s/%s systems", runtime.GOOS, runtime.GOARCH)
 	}
 	return func(w http.ResponseWriter, r *http.Request) {
-		defer w.(http.Flusher).Flush()
 		w.Header().Set("Server", ssh3.GetCurrentVersionString())
 		major, minor, patch, err := ssh3.ParseVersionString(r.UserAgent())
 		log.Debug().Msgf("received request from User-Agent %s (major %d, minor %d, patch %d)", r.UserAgent(), major, minor, patch)
 		// currently apply strict version rules
 		if err != nil || major != ssh3.MAJOR || minor != ssh3.MINOR {
-			w.WriteHeader(http.StatusForbidden)
 			if err == nil {
-				w.Write([]byte(fmt.Sprintf("Unsupported version: %d.%d.%d not supported by server in version %s", major, minor, patch, ssh3.GetCurrentVersionString())))
+				http.Error(w, fmt.Sprintf("Unsupported version: %d.%d.%d not supported by server with version %s", major, minor, patch, ssh3.GetCurrentVersionString()), http.StatusForbidden)
 			} else {
-				w.Write([]byte("Unsupported user-agent"))
+				http.Error(w, "Unsupported user-agent", http.StatusForbidden)
 			}
 			return
 		}
+		// Only call Flush() here, as calling flush prevents from adding the Content-Length header to the response
+		// The Content-Length can be useful upon receiving an error response
+		defer w.(http.Flusher).Flush()
 		hijacker, ok := w.(http3.Hijacker)
 		if !ok { // should never happen, unless quic-go change their API
 			log.Error().Msgf("failed to hijack")

util/types.go

@@ -55,6 +55,20 @@ func (e Unauthorized) Error() string {
 	return "Unauthorized"
 }
 
+type OtherHTTPError struct {
+	StatusCode int
+	HasBody    bool
+	Body       string
+}
+
+func (e OtherHTTPError) Error() string {
+	str := fmt.Sprintf("HTTP response with %d status code", e.StatusCode)
+	if e.HasBody {
+		str = fmt.Sprintf("%s: %s", str, e.Body)
+	}
+	return str
+}
+
 type BytesReadCloser struct {
 	*bytes.Reader
 }