Merge pull request #195 from b-open-io/pr-190

fix: address remaining code review issues from PR #190

Author: rohenaz

CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. The format
 
 ## Table of Contents
 
+- [1.2.3 - 2025-06-30](#123---2025-06-30)
 - [1.2.2 - 2025-06-27](#122---2025-06-27)
 - [1.2.1 - 2025-06-12](#121---2025-06-12)
 - [1.2.0 - 2025-06-10](#120---2025-06-10)
@@ -37,6 +38,28 @@ All notable changes to this project will be documented in this file. The format
 - [1.1.0 - 2024-08-19](#110---2024-08-19)
 - [1.0.0 - 2024-06-06](#100---2024-06-06)
 
+## [1.2.3] - 2025-06-30
+
+### Added
+- Enhanced peer authentication with proper message handling at receiver side
+- Centralized certificate exchange logic with new `sendCertificates` method
+- Added `SignCertificateWithWalletForTest` for wallet-based certificate signing in tests
+- Implemented proper nonce creation/verification using `utils.CreateNonce` and `utils.VerifyNonce`
+
+### Changed
+- Updated `AUTH_PROTOCOL_ID` from "authrite message signature" to "auth message signature"
+- Changed `AuthMessage` JSON marshaling to use `wallet.BytesList` for Payload and Signature fields
+- Enhanced signature creation/verification to use wallet methods with explicit protocol details
+- Improved mock wallet with `VerifyHMAC` implementation
+
+### Fixed
+- Fixed base64 concatenation bug in peer authentication nonce handling
+- Fixed certificate signing bug in `certificate_debug.go`
+- Fixed `TestPeerCertificateExchange` test to properly encrypt certificate fields
+- Improved error handling consistency using `NewAuthError`
+- Fixed variable naming consistency
+- Enhanced session state management and error messages throughout auth flow
+
 ## [1.2.2] - 2025-06-27
 ### Added
 - New `CurrentHeight` function on chaintracker interface

auth/peer.go

@@ -494,15 +494,21 @@ func (p *Peer) handleInitialRequest(ctx context.Context, message *AuthMessage, s
 		Certificates: certs,
 	}
 
-	data := message.InitialNonce + session.SessionNonce
-	sigData, err := base64.StdEncoding.DecodeString(data)
+	// Decode the nonces first before concatenating
+	initialNonceBytes, err := base64.StdEncoding.DecodeString(message.InitialNonce)
 	if err != nil {
-		return NewAuthError("failed to prepare data to sign", err)
+		return NewAuthError("failed to decode initial nonce", err)
 	}
+	sessionNonceBytes, err := base64.StdEncoding.DecodeString(session.SessionNonce)
+	if err != nil {
+		return NewAuthError("failed to decode session nonce", err)
+	}
+	// Concatenate the decoded bytes
+	sigData := append(initialNonceBytes, sessionNonceBytes...)
 
 	keyID := fmt.Sprintf("%s %s", message.InitialNonce, session.SessionNonce)
 
-	arg := wallet.CreateSignatureArgs{
+	args := wallet.CreateSignatureArgs{
 		EncryptionArgs: wallet.EncryptionArgs{
 			ProtocolID: wallet.Protocol{
 				// SecurityLevel set to 2 (SecurityLevelEveryAppAndCounterparty) as specified in BRC-31 (Authrite)
@@ -515,13 +521,12 @@ func (p *Peer) handleInitialRequest(ctx context.Context, message *AuthMessage, s
 				Counterparty: message.IdentityKey,
 			},
 		},
-		// Sign the certificate request data, as in TypeScript
 		Data: sigData,
 	}
 
-	sigResult, err := p.wallet.CreateSignature(ctx, arg, "")
+	sigResult, err := p.wallet.CreateSignature(ctx, args, "")
 	if err != nil {
-		return fmt.Errorf("failed to sign initial response: %w", err)
+		return NewAuthError("failed to sign initial response", err)
 	}
 
 	response.Signature = sigResult.Signature.Serialize()
@@ -545,16 +550,21 @@ func (p *Peer) handleInitialResponse(ctx context.Context, message *AuthMessage,
 		return ErrSessionNotFound
 	}
 
-	data := message.InitialNonce + session.SessionNonce
-
-	sigData, err := base64.StdEncoding.DecodeString(data)
+	// Decode the nonces first before concatenating
+	initialNonceBytes, err := base64.StdEncoding.DecodeString(message.InitialNonce)
+	if err != nil {
+		return NewAuthError("failed to decode initial nonce", err)
+	}
+	sessionNonceBytes, err := base64.StdEncoding.DecodeString(session.SessionNonce)
 	if err != nil {
-		return NewAuthError("failed to prepare data to sign", err)
+		return NewAuthError("failed to decode session nonce", err)
 	}
+	// Concatenate the decoded bytes
+	sigData := append(initialNonceBytes, sessionNonceBytes...)
 
 	signature, err := ec.ParseSignature(message.Signature)
 	if err != nil {
-		return fmt.Errorf("failed to parse signature: %w", err)
+		return NewAuthError("failed to parse signature", err)
 	}
 
 	verifyResult, err := p.wallet.VerifySignature(ctx, wallet.VerifySignatureArgs{
@@ -612,13 +622,13 @@ func (p *Peer) handleInitialResponse(ctx context.Context, message *AuthMessage,
 			utilsRequestedCerts,
 		)
 		if err != nil {
-			return fmt.Errorf("invalid certificates: %w", err)
+			return NewAuthError("invalid certificates", err)
 		}
 
 		for _, callback := range p.onCertificateReceivedCallbacks {
 			err := callback(senderPublicKey, message.Certificates)
 			if err != nil {
-				return fmt.Errorf("certificate received callback error: %w", err)
+				return NewAuthError("certificate received callback error", err)
 			}
 		}
 	}
@@ -638,7 +648,7 @@ func (p *Peer) handleInitialResponse(ctx context.Context, message *AuthMessage,
 	if len(message.RequestedCertificates.Certifiers) > 0 || len(message.RequestedCertificates.CertificateTypes) > 0 {
 		err = p.sendCertificates(ctx, message)
 		if err != nil {
-			return fmt.Errorf("failed to send requested certificates: %w", err)
+			return NewAuthError("failed to send requested certificates", err)
 		}
 	}
 
@@ -712,7 +722,7 @@ func (p *Peer) handleCertificateRequest(ctx context.Context, message *AuthMessag
 	// Try to parse the signature
 	signature, err := ec.ParseSignature(message.Signature)
 	if err != nil {
-		return fmt.Errorf("failed to parse signature: %w", err)
+		return NewAuthError("failed to parse signature", err)
 	}
 
 	// Verify signature
@@ -740,7 +750,7 @@ func (p *Peer) handleCertificateRequest(ctx context.Context, message *AuthMessag
 	if len(message.RequestedCertificates.Certifiers) > 0 || len(message.RequestedCertificates.CertificateTypes) > 0 {
 		err = p.sendCertificates(ctx, message)
 		if err != nil {
-			return fmt.Errorf("failed to send requested certificates: %w", err)
+			return NewAuthError("failed to send requested certificates", err)
 		}
 	}
 
@@ -776,7 +786,7 @@ func (p *Peer) handleCertificateResponse(ctx context.Context, message *AuthMessa
 	// Try to parse the signature
 	signature, err := ec.ParseSignature(message.Signature)
 	if err != nil {
-		return fmt.Errorf("failed to parse signature: %w", err)
+		return NewAuthError("failed to parse signature", err)
 	}
 
 	// Verify signature
@@ -865,7 +875,7 @@ func (p *Peer) handleGeneralMessage(ctx context.Context, message *AuthMessage, s
 	// Try to parse the signature
 	signature, err := ec.ParseSignature(message.Signature)
 	if err != nil {
-		return fmt.Errorf("failed to parse signature: %w", err)
+		return NewAuthError("failed to parse signature", err)
 	}
 
 	// Verify signature

auth/peer_test.go

@@ -1,6 +1,7 @@
 package auth
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"fmt"
@@ -405,7 +406,6 @@ func (t *LoggingMockTransport) OnData(callback func(context.Context, *AuthMessag
 
 // TestPeerCertificateExchange tests certificate request and exchange
 func TestPeerCertificateExchange(t *testing.T) {
-	t.Skip("Needs a fix for DecryptFields - probably something in test setup need to be adjusted")
 
 	var certType = tu.GetByte32FromString("testCertType")
 	requiredField := "testField"
@@ -451,13 +451,23 @@ func TestPeerCertificateExchange(t *testing.T) {
 		return &wallet.VerifySignatureResult{Valid: true}, nil
 	}
 
-	// Create raw certificates with proper base64 encoding using our helper
+	// Generate a symmetric key for field encryption
+	fieldSymmetricKeyBytes := bytes.Repeat([]byte{1}, 32)
+	fieldSymmetricKey := ec.NewSymmetricKey(fieldSymmetricKeyBytes)
+	
+	// Encrypt the field value
+	plainFieldValue := []byte("decrypted field value")
+	encryptedFieldBytes, err := fieldSymmetricKey.Encrypt(plainFieldValue)
+	require.NoError(t, err)
+	encryptedFieldValue := base64.StdEncoding.EncodeToString(encryptedFieldBytes)
+
+	// Create raw certificates with encrypted fields
 	aliceCertRaw := wallet.Certificate{
 		Type:               certType,
 		SerialNumber:       tu.GetByte32FromString("serial1"),
 		Subject:            aliceSubject,
 		Certifier:          bobSubject,
-		Fields:             map[string]string{requiredField: "fieldValue"},
+		Fields:             map[string]string{requiredField: encryptedFieldValue},
 		RevocationOutpoint: tu.OutpointFromString(t, "a755810c21e17183ff6db6685f0de239fd3a0a3c0d4ba7773b0b0d1748541e2b.0"),
 	}
 
@@ -466,7 +476,7 @@ func TestPeerCertificateExchange(t *testing.T) {
 		SerialNumber:       tu.GetByte32FromString("serial2"),
 		Subject:            bobSubject,
 		Certifier:          aliceSubject,
-		Fields:             map[string]string{requiredField: "fieldValue"},
+		Fields:             map[string]string{requiredField: encryptedFieldValue},
 		RevocationOutpoint: tu.OutpointFromString(t, "a755810c21e17183ff6db6685f0de239fd3a0a3c0d4ba7773b0b0d1748541e2b.1"),
 	}
 
@@ -510,28 +520,29 @@ func TestPeerCertificateExchange(t *testing.T) {
 	logger.Printf("DEBUG: Alice cert signature: %x", aliceCert.Signature)
 	logger.Printf("DEBUG: Bob cert signature: %x", bobCert.Signature)
 
-	// Create mock keyring results - also properly encoded
-	fieldValueBase64 := base64.StdEncoding.EncodeToString([]byte("key-for-field"))
+	// Mock keyring - in real usage this would be the symmetric key encrypted for the verifier
+	// For testing, we just need valid encrypted data that MockDecrypt can "decrypt" to fieldSymmetricKeyBytes
+	encryptedSymmetricKey := base64.StdEncoding.EncodeToString(fieldSymmetricKeyBytes)
 	aliceWallet.MockProveCertificate = func(ctx context.Context, args wallet.ProveCertificateArgs, originator string) (*wallet.ProveCertificateResult, error) {
 		return &wallet.ProveCertificateResult{
-			KeyringForVerifier: map[string]string{requiredField: fieldValueBase64},
+			KeyringForVerifier: map[string]string{requiredField: encryptedSymmetricKey},
 		}, nil
 	}
 	bobWallet.MockProveCertificate = func(ctx context.Context, args wallet.ProveCertificateArgs, originator string) (*wallet.ProveCertificateResult, error) {
 		return &wallet.ProveCertificateResult{
-			KeyringForVerifier: map[string]string{requiredField: fieldValueBase64},
+			KeyringForVerifier: map[string]string{requiredField: encryptedSymmetricKey},
 		}, nil
 	}
 
-	// Configure wallet mocks for Decrypt to make DecryptFields work
+	// MockDecrypt returns the symmetric key for field decryption
 	aliceWallet.MockDecrypt = func(ctx context.Context, args wallet.DecryptArgs, originator string) (*wallet.DecryptResult, error) {
 		return &wallet.DecryptResult{
-			Plaintext: []byte("decrypted field value"),
+			Plaintext: fieldSymmetricKeyBytes,
 		}, nil
 	}
 	bobWallet.MockDecrypt = func(ctx context.Context, args wallet.DecryptArgs, originator string) (*wallet.DecryptResult, error) {
 		return &wallet.DecryptResult{
-			Plaintext: []byte("decrypted field value"),
+			Plaintext: fieldSymmetricKeyBytes,
 		}, nil
 	}
 

auth/utils/certificate_debug.go

@@ -82,7 +82,7 @@ func SignCertificateWithWalletForTest(ctx context.Context, cert wallet.Certifica
 		return encodedCert, fmt.Errorf("failed to get identity key of signer: %w", err)
 	}
 
-	cert.Certifier = publicKeyResult.PublicKey
+	encodedCert.Certifier = publicKeyResult.PublicKey
 
 	certObj, err := certificates.FromWalletCertificate(&encodedCert)
 	if err != nil {