From ddb75700a08ebca65e8a6dfac825f4ec781af43c Mon Sep 17 00:00:00 2001
From: Neil <git@neilalexander.dev>
Date: Sat, 4 Nov 2023 17:57:04 +0000
Subject: [PATCH] Report errors during handshake stage (#1091)

Co-authored-by: Neil Alexander <neilalexander@users.noreply.github.com>
---
 src/core/link.go         |  5 +++--
 src/core/version.go      | 27 +++++++++++++++------------
 src/core/version_test.go |  6 +++---
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/core/link.go b/src/core/link.go
index c3808698..f3cb4318 100644
--- a/src/core/link.go
+++ b/src/core/link.go
@@ -546,8 +546,9 @@ func (l *links) handler(linkType linkType, options linkOptions, conn net.Conn) e
 	}
 	meta = version_metadata{}
 	base := version_getBaseMetadata()
-	if !meta.decode(conn, options.password) {
-		return conn.Close()
+	if err := meta.decode(conn, options.password); err != nil {
+		_ = conn.Close()
+		return err
 	}
 	if !meta.check() {
 		return fmt.Errorf("remote node incompatible version (local %s, remote %s)",
diff --git a/src/core/version.go b/src/core/version.go
index 332e18c8..28b16430 100644
--- a/src/core/version.go
+++ b/src/core/version.go
@@ -87,22 +87,22 @@ func (m *version_metadata) encode(privateKey ed25519.PrivateKey, password []byte
 }
 
 // Decodes version metadata from its wire format into the struct.
-func (m *version_metadata) decode(r io.Reader, password []byte) bool {
+func (m *version_metadata) decode(r io.Reader, password []byte) error {
 	bh := [6]byte{}
 	if _, err := io.ReadFull(r, bh[:]); err != nil {
-		return false
+		return err
 	}
 	meta := [4]byte{'m', 'e', 't', 'a'}
 	if !bytes.Equal(bh[:4], meta[:]) {
-		return false
+		return fmt.Errorf("invalid handshake preamble")
 	}
-	bs := make([]byte, binary.BigEndian.Uint16(bh[4:6]))
+	hl := binary.BigEndian.Uint16(bh[4:6])
+	if hl < ed25519.SignatureSize {
+		return fmt.Errorf("invalid handshake length")
+	}
+	bs := make([]byte, hl)
 	if _, err := io.ReadFull(r, bs); err != nil {
-		return false
-	}
-
-	if len(bs) < ed25519.SignatureSize {
-		return false
+		return err
 	}
 	sig := bs[len(bs)-ed25519.SignatureSize:]
 	bs = bs[:len(bs)-ed25519.SignatureSize]
@@ -132,14 +132,17 @@ func (m *version_metadata) decode(r io.Reader, password []byte) bool {
 
 	hasher, err := blake2b.New512(password)
 	if err != nil {
-		return false
+		return fmt.Errorf("invalid password supplied")
 	}
 	n, err := hasher.Write(m.publicKey)
 	if err != nil || n != ed25519.PublicKeySize {
-		return false
+		return fmt.Errorf("failed to generate hash")
 	}
 	hash := hasher.Sum(nil)
-	return ed25519.Verify(m.publicKey, hash, sig)
+	if !ed25519.Verify(m.publicKey, hash, sig) {
+		return fmt.Errorf("password is incorrect")
+	}
+	return nil
 }
 
 // Checks that the "meta" bytes and the version numbers are the expected values.
diff --git a/src/core/version_test.go b/src/core/version_test.go
index 7efe2f35..512c6e59 100644
--- a/src/core/version_test.go
+++ b/src/core/version_test.go
@@ -34,7 +34,7 @@ func TestVersionPasswordAuth(t *testing.T) {
 		}
 
 		var decoded version_metadata
-		if allowed := decoded.decode(bytes.NewBuffer(encoded), tt.password2); allowed != tt.allowed {
+		if allowed := decoded.decode(bytes.NewBuffer(encoded), tt.password2) == nil; allowed != tt.allowed {
 			t.Fatalf("Permutation %q -> %q should have been %v but was %v", tt.password1, tt.password2, tt.allowed, allowed)
 		}
 	}
@@ -67,8 +67,8 @@ func TestVersionRoundtrip(t *testing.T) {
 			}
 			encoded := bytes.NewBuffer(meta)
 			decoded := &version_metadata{}
-			if !decoded.decode(encoded, password) {
-				t.Fatalf("failed to decode")
+			if err := decoded.decode(encoded, password); err != nil {
+				t.Fatalf("failed to decode: %s", err)
 			}
 			if !reflect.DeepEqual(test, decoded) {
 				t.Fatalf("round-trip failed\nwant: %+v\n got: %+v", test, decoded)