Implement validation for SESSION ADD options: ensure SIGNATURE_TYPE is removed per SAMv3.3 spec, and add tests to verify behavior and warnings.

common/session.go

@@ -310,7 +310,13 @@ func (sam *SAM) RemoveSubSession(id string) error {
 func (sam *SAM) buildSessionAddMessage(style, id string, options []string) ([]byte, error) {
 	baseMsg := "SESSION ADD STYLE=" + style + " ID=" + id
 
-	extraStr := strings.Join(options, " ")
+	// Validate and clean options - SESSION ADD should not contain SIGNATURE_TYPE
+	// Per SAMv3.3 spec: "Do not set the DESTINATION option on a SESSION ADD.
+	// The subsession will use the destination specified in the primary session."
+	// This also applies to SIGNATURE_TYPE since it's part of the destination.
+	cleanedOptions := validateSubSessionOptions(options)
+
+	extraStr := strings.Join(cleanedOptions, " ")
 	if extraStr != "" {
 		baseMsg += " " + extraStr
 	}

common/signature_validation.go

@@ -92,3 +92,31 @@ func ExtractSignatureType(options []string) (string, []string) {
 func EnsureSignatureType(sigType string, options []string) []string {
 	return validateAndCleanOptions(sigType, options)
 }
+
+// validateSubSessionOptions validates and cleans options for SESSION ADD commands.
+// Per SAMv3.3 spec: "Do not set the DESTINATION option on a SESSION ADD.
+// The subsession will use the destination specified in the primary session."
+// This also applies to SIGNATURE_TYPE since it's part of the destination.
+// Removes any SIGNATURE_TYPE entries and logs warnings about spec violations.
+func validateSubSessionOptions(options []string) []string {
+	var cleanedOptions []string
+	var removedEntries []string
+
+	for _, opt := range options {
+		if strings.HasPrefix(opt, "SIGNATURE_TYPE=") {
+			removedEntries = append(removedEntries, opt)
+			logrus.WithField("removedOption", opt).Warn("Removing SIGNATURE_TYPE from SESSION ADD - subsessions inherit signature type from primary session per SAMv3.3 spec")
+			continue
+		}
+		cleanedOptions = append(cleanedOptions, opt)
+	}
+
+	if len(removedEntries) > 0 {
+		logrus.WithFields(logrus.Fields{
+			"removedOptions":   removedEntries,
+			"remainingOptions": cleanedOptions,
+		}).Warn("SESSION ADD signature type entries removed - subsessions inherit from primary session")
+	}
+
+	return cleanedOptions
+}

common/signature_validation_test.go

@@ -260,3 +260,73 @@ func TestSignatureTypeConflictResolution(t *testing.T) {
 		}
 	})
 }
+
+// TestValidateSubSessionOptions tests that SESSION ADD properly removes
+// SIGNATURE_TYPE entries according to SAMv3.3 specification.
+func TestValidateSubSessionOptions(t *testing.T) {
+	tests := []struct {
+		name            string
+		options         []string
+		expectedOptions []string
+		expectWarning   bool
+	}{
+		{
+			name:            "NoSignatureType",
+			options:         []string{"PORT=7000", "inbound.length=2"},
+			expectedOptions: []string{"PORT=7000", "inbound.length=2"},
+			expectWarning:   false,
+		},
+		{
+			name:            "WithSignatureType",
+			options:         []string{"SIGNATURE_TYPE=EdDSA_SHA512_Ed25519", "PORT=7000"},
+			expectedOptions: []string{"PORT=7000"},
+			expectWarning:   true,
+		},
+		{
+			name:            "MultipleSignatureTypes",
+			options:         []string{"SIGNATURE_TYPE=ECDSA_SHA256_P256", "PORT=7000", "SIGNATURE_TYPE=EdDSA_SHA512_Ed25519"},
+			expectedOptions: []string{"PORT=7000"},
+			expectWarning:   true,
+		},
+		{
+			name:            "OnlySignatureTypes",
+			options:         []string{"SIGNATURE_TYPE=DSA_SHA1", "SIGNATURE_TYPE=EdDSA_SHA512_Ed25519"},
+			expectedOptions: []string{},
+			expectWarning:   true,
+		},
+		{
+			name:            "EmptyOptions",
+			options:         []string{},
+			expectedOptions: []string{},
+			expectWarning:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := validateSubSessionOptions(tt.options)
+
+			// Check result length
+			if len(result) != len(tt.expectedOptions) {
+				t.Errorf("Expected %d options, got %d", len(tt.expectedOptions), len(result))
+				t.Errorf("Expected: %v", tt.expectedOptions)
+				t.Errorf("Got: %v", result)
+				return
+			}
+
+			// Check each option
+			for i, expected := range tt.expectedOptions {
+				if result[i] != expected {
+					t.Errorf("Option %d: expected %q, got %q", i, expected, result[i])
+				}
+			}
+
+			// Ensure no SIGNATURE_TYPE remains
+			for _, opt := range result {
+				if strings.HasPrefix(opt, "SIGNATURE_TYPE=") {
+					t.Errorf("Found unexpected SIGNATURE_TYPE in result: %q", opt)
+				}
+			}
+		})
+	}
+}

common/subsession_signature_test.go

@@ -0,0 +1,133 @@
+package common
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestAddSubSessionSignatureTypeHandling tests that SESSION ADD correctly handles
+// signature type parameters according to SAMv3.3 spec.
+// Per spec: "Do not set the DESTINATION option on a SESSION ADD. The subsession
+// will use the destination specified in the primary session."
+// This also applies to SIGNATURE_TYPE since it's part of the destination.
+func TestAddSubSessionSignatureTypeHandling(t *testing.T) {
+	tests := []struct {
+		name            string
+		style           string
+		id              string
+		options         []string
+		expectWarning   bool
+		expectedOptions []string
+	}{
+		{
+			name:            "NoSignatureType_Valid",
+			style:           "STREAM",
+			id:              "test-stream",
+			options:         []string{"FROM_PORT=8080", "inbound.length=2"},
+			expectWarning:   false,
+			expectedOptions: []string{"FROM_PORT=8080", "inbound.length=2"},
+		},
+		{
+			name:            "WithSignatureType_ShouldWarn",
+			style:           "DATAGRAM",
+			id:              "test-datagram",
+			options:         []string{"SIGNATURE_TYPE=EdDSA_SHA512_Ed25519", "PORT=7000", "inbound.length=2"},
+			expectWarning:   true,
+			expectedOptions: []string{"PORT=7000", "inbound.length=2"},
+		},
+		{
+			name:            "MultipleSignatureTypes_ShouldWarn",
+			style:           "RAW",
+			id:              "test-raw",
+			options:         []string{"SIGNATURE_TYPE=ECDSA_SHA256_P256", "PORT=7001", "SIGNATURE_TYPE=EdDSA_SHA512_Ed25519"},
+			expectWarning:   true,
+			expectedOptions: []string{"PORT=7001"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test the buildSessionAddMessage function behavior
+			sam := &SAM{}
+
+			// For now, let's just test the current behavior
+			message, err := sam.buildSessionAddMessage(tt.style, tt.id, tt.options)
+			if err != nil {
+				t.Errorf("buildSessionAddMessage() error = %v", err)
+				return
+			}
+
+			messageStr := string(message)
+			t.Logf("Generated message: %s", strings.TrimSpace(messageStr))
+
+			// Check if SIGNATURE_TYPE appears in the message
+			hasSignatureType := strings.Contains(messageStr, "SIGNATURE_TYPE=")
+
+			if hasSignatureType {
+				t.Logf("⚠️  POTENTIAL ISSUE: SIGNATURE_TYPE found in SESSION ADD message")
+				t.Logf("According to SAMv3.3 spec, subsessions should inherit signature type from primary session")
+
+				// Count how many SIGNATURE_TYPE entries
+				signatureCount := strings.Count(messageStr, "SIGNATURE_TYPE=")
+				if signatureCount > 0 {
+					t.Logf("Found %d SIGNATURE_TYPE entries in SESSION ADD message", signatureCount)
+					t.Error("SESSION ADD should not contain SIGNATURE_TYPE according to SAMv3.3 spec")
+				}
+			}
+
+			// Verify expected format
+			expectedStart := "SESSION ADD STYLE=" + tt.style + " ID=" + tt.id
+			if !strings.HasPrefix(messageStr, expectedStart) {
+				t.Errorf("Message should start with %q, got %q", expectedStart, messageStr[:len(expectedStart)])
+			}
+		})
+	}
+}
+
+// TestSessionAddVsSessionCreate demonstrates the difference between
+// SESSION CREATE (which should allow SIGNATURE_TYPE) and SESSION ADD (which should not).
+func TestSessionAddVsSessionCreate(t *testing.T) {
+	sam := &SAM{}
+
+	t.Run("SessionCreateWithSignatureType", func(t *testing.T) {
+		// SESSION CREATE should allow and handle SIGNATURE_TYPE
+		options := []string{"SIGNATURE_TYPE=EdDSA_SHA512_Ed25519", "inbound.length=2"}
+
+		// Mock the SAM config to have a signature type
+		sam.SAMEmit.I2PConfig.SigType = "EdDSA_SHA512_Ed25519"
+
+		message, err := sam.buildSessionCreateMessage(options)
+		if err != nil {
+			t.Errorf("buildSessionCreateMessage() error = %v", err)
+			return
+		}
+
+		messageStr := string(message)
+		t.Logf("SESSION CREATE message: %s", strings.TrimSpace(messageStr))
+
+		// Should handle signature type conflicts properly (this tests our fix)
+		if strings.Count(messageStr, "SIGNATURE_TYPE=") > 1 {
+			t.Error("SESSION CREATE should not have duplicate SIGNATURE_TYPE entries")
+		}
+	})
+
+	t.Run("SessionAddWithSignatureType", func(t *testing.T) {
+		// SESSION ADD should not include SIGNATURE_TYPE according to spec
+		options := []string{"SIGNATURE_TYPE=ECDSA_SHA256_P256", "PORT=7000"}
+
+		message, err := sam.buildSessionAddMessage("DATAGRAM", "test-sub", options)
+		if err != nil {
+			t.Errorf("buildSessionAddMessage() error = %v", err)
+			return
+		}
+
+		messageStr := string(message)
+		t.Logf("SESSION ADD message: %s", strings.TrimSpace(messageStr))
+
+		// This demonstrates the potential issue
+		if strings.Contains(messageStr, "SIGNATURE_TYPE=") {
+			t.Error("SESSION ADD should not contain SIGNATURE_TYPE according to SAMv3.3 spec")
+			t.Log("ISSUE DETECTED: SESSION ADD allows SIGNATURE_TYPE in options, but spec says subsessions inherit from primary")
+		}
+	})
+}