fix: exclude opcode from P_DATA_V1 AEAD additional data for OpenVPN (#3237)

OpenVPN authenticates the opcode/peer-id header as AEAD additional data only for P_DATA_V2. For P_DATA_V1 the additional data is the packet ID alone. We included the opcode for both, so every data packet exchanged with a server that does not push peer-id (e.g. SoftEther) failed authentication and was silently dropped after a successful handshake.
2026-10-10 04:03:11 +08:00 · 2026-09-25 00:24:38 +08:00
parent 41b8a05915
commit 8d57a8c57c
2 changed files with 204 additions and 6 deletions
@@ -220,9 +220,7 @@ func (d *DataChannel) encryptAEAD(packet []byte, packetID uint32) ([]byte, error
 	var packetIDBytes [4]byte
 	binary.BigEndian.PutUint32(packetIDBytes[:], packetID)
 	nonce := d.nonce(packetID, d.sendImplicitIV)
-	ad := make([]byte, 0, len(header)+len(packetIDBytes))
-	ad = append(ad, header...)
-	ad = append(ad, packetIDBytes[:]...)
+	ad := aeadAdditionalData(header, packetIDBytes[:])
 	sealed := d.sendAEAD.Seal(nil, nonce[:], packet, ad)

 	out := make([]byte, 0, len(header)+4+DataChannelTagSize+len(packet))
@@ -288,9 +286,7 @@ func (d *DataChannel) decryptAEAD(packet []byte, headerSize int) ([]byte, error)
 	combined := make([]byte, 0, len(ciphertext)+DataChannelTagSize)
 	combined = append(combined, ciphertext...)
 	combined = append(combined, tag...)
-	ad := make([]byte, 0, len(header)+len(packetIDBytes))
-	ad = append(ad, header...)
-	ad = append(ad, packetIDBytes...)
+	ad := aeadAdditionalData(header, packetIDBytes)

 	nonce := d.nonce(packetID, d.recvImplicitIV)
 	plain, err := d.recvAEAD.Open(nil, nonce[:], combined, ad)
@@ -426,6 +422,23 @@ func (d *DataChannel) acceptPacketID(packetID uint32) error {
 	return nil
 }

+// aeadAdditionalData returns the AEAD additional data of a data channel packet.
+//
+// OpenVPN only authenticates the opcode/peer-id header for P_DATA_V2. For
+// P_DATA_V1 the additional data is the packet ID alone: in OpenVPN's
+// handle_data_channel_packet (ssl.c) ad_start is set after the opcode byte has
+// been skipped for P_DATA_V1, but before it for P_DATA_V2. Servers that only
+// speak P_DATA_V1 (e.g. SoftEther) reject packets that include the opcode.
+func aeadAdditionalData(header, packetID []byte) []byte {
+	ad := make([]byte, 0, len(header)+len(packetID))
+	if len(header) > 0 {
+		if opcode, _ := parseOpcodeKeyID(header[0]); opcode == PDataV2 {
+			ad = append(ad, header...)
+		}
+	}
+	return append(ad, packetID...)
+}
+
 func dataHeader(peerID uint32, keyID uint8) []byte {
 	if peerID != PeerIDUnset {
 		return []byte{
@@ -0,0 +1,185 @@
+package openvpn
+
+import (
+	"bytes"
+	"crypto/aes"
+	"crypto/cipher"
+	"encoding/binary"
+	"testing"
+)
+
+// These tests pin the P_DATA_V1 / P_DATA_V2 AEAD wire format against the
+// OpenVPN protocol definition rather than against mihomo itself, so that a
+// consistently-wrong implementation on both ends cannot pass.
+//
+// OpenVPN 2.x (src/openvpn/ssl.c, handle_data_channel_packet): for P_DATA_V2
+// ad_start is taken *before* skipping the opcode byte, so the additional data
+// is opcode|peer-id|packet-id; for P_DATA_V1 it is taken *after* skipping the
+// opcode, so the additional data is the 4-byte packet-id only. SoftEther's
+// OpenVPN server (Interop_OpenVPN.c) does the same for V1.
+
+func v1TestKeys() (client, server *KeyMaterial) {
+	client = &KeyMaterial{
+		SendCipherKey: bytes.Repeat([]byte{0x11}, 32),
+		SendHMACKey:   bytes.Repeat([]byte{0x22}, maxHMACKeyLength),
+		RecvCipherKey: bytes.Repeat([]byte{0x33}, 32),
+		RecvHMACKey:   bytes.Repeat([]byte{0x44}, maxHMACKeyLength),
+	}
+	server = &KeyMaterial{
+		SendCipherKey: client.RecvCipherKey,
+		SendHMACKey:   client.RecvHMACKey,
+		RecvCipherKey: client.SendCipherKey,
+		RecvHMACKey:   client.SendHMACKey,
+	}
+	return
+}
+
+// refAEAD is an independent reference implementation of the OpenVPN AEAD
+// data channel: nonce = packet-id || implicit IV (first 8 bytes of the HMAC key),
+// wire = header || packet-id || tag || ciphertext.
+func refAEAD(t *testing.T, cipherKey, hmacKey []byte) (cipher.AEAD, func(uint32) []byte) {
+	t.Helper()
+	block, err := aes.NewCipher(cipherKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	aead, err := cipher.NewGCM(block)
+	if err != nil {
+		t.Fatal(err)
+	}
+	nonce := func(pid uint32) []byte {
+		n := make([]byte, 12)
+		binary.BigEndian.PutUint32(n[:4], pid)
+		copy(n[4:], hmacKey[:8])
+		return n
+	}
+	return aead, nonce
+}
+
+func refSeal(t *testing.T, cipherKey, hmacKey, header []byte, pid uint32, ad, plain []byte) []byte {
+	aead, nonce := refAEAD(t, cipherKey, hmacKey)
+	sealed := aead.Seal(nil, nonce(pid), plain, ad)
+	tagStart := len(sealed) - DataChannelTagSize
+	var pidb [4]byte
+	binary.BigEndian.PutUint32(pidb[:], pid)
+	out := append([]byte{}, header...)
+	out = append(out, pidb[:]...)
+	out = append(out, sealed[tagStart:]...)
+	return append(out, sealed[:tagStart]...)
+}
+
+func refOpen(t *testing.T, cipherKey, hmacKey []byte, packet []byte, headerSize int, ad []byte) ([]byte, error) {
+	aead, nonce := refAEAD(t, cipherKey, hmacKey)
+	pid := binary.BigEndian.Uint32(packet[headerSize : headerSize+4])
+	tag := packet[headerSize+4 : headerSize+4+DataChannelTagSize]
+	ct := packet[headerSize+4+DataChannelTagSize:]
+	return aead.Open(nil, nonce(pid), append(append([]byte{}, ct...), tag...), ad)
+}
+
+var testIPPacket = []byte{0x45, 0, 0, 20, 1, 2, 3, 4, 64, 6, 0, 0, 10, 8, 0, 2, 1, 1, 1, 1}
+
+func TestDataChannelAESGCMV1RoundTrip(t *testing.T) {
+	ck, sk := v1TestKeys()
+	client, err := NewDataChannel(ck, CipherAES256GCM, AuthSHA1, PeerIDUnset, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	server, err := NewDataChannel(sk, CipherAES256GCM, AuthSHA1, PeerIDUnset, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	enc, err := client.Encrypt(testIPPacket)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if opcode, _ := parseOpcodeKeyID(enc[0]); opcode != PDataV1 {
+		t.Fatalf("unexpected data opcode: %s", opcode)
+	}
+	plain, err := server.Decrypt(enc)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(plain, testIPPacket) {
+		t.Fatalf("unexpected decrypted packet: %x", plain)
+	}
+}
+
+// mihomo -> standard OpenVPN/SoftEther peer: our V1 packet must authenticate
+// with AD = packet-id only, and must NOT authenticate with opcode|packet-id.
+func TestDataChannelAEADV1EncryptMatchesOpenVPN(t *testing.T) {
+	ck, _ := v1TestKeys()
+	client, err := NewDataChannel(ck, CipherAES256GCM, AuthSHA1, PeerIDUnset, 4)
+	if err != nil {
+		t.Fatal(err)
+	}
+	enc, err := client.Encrypt(testIPPacket)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if enc[0] != 0x34 { // P_DATA_V1, key-id 4, as seen on the wire from OpenVPN 2.6
+		t.Fatalf("unexpected first byte %#02x", enc[0])
+	}
+	pidOnly := enc[1:5]
+	plain, err := refOpen(t, ck.SendCipherKey, ck.SendHMACKey, enc, 1, pidOnly)
+	if err != nil {
+		t.Fatalf("standard peer rejects our P_DATA_V1 packet (AD must be packet-id only): %v", err)
+	}
+	if !bytes.Equal(plain, testIPPacket) {
+		t.Fatalf("unexpected plaintext: %x", plain)
+	}
+	if _, err := refOpen(t, ck.SendCipherKey, ck.SendHMACKey, enc, 1, enc[:5]); err == nil {
+		t.Fatal("P_DATA_V1 must not authenticate the opcode byte")
+	}
+}
+
+// standard OpenVPN/SoftEther peer -> mihomo: a V1 packet sealed with
+// AD = packet-id only must decrypt.
+func TestDataChannelAEADV1DecryptMatchesOpenVPN(t *testing.T) {
+	ck, sk := v1TestKeys()
+	client, err := NewDataChannel(ck, CipherAES256GCM, AuthSHA1, PeerIDUnset, 4)
+	if err != nil {
+		t.Fatal(err)
+	}
+	header := []byte{0x34}
+	var pidb [4]byte
+	binary.BigEndian.PutUint32(pidb[:], 1)
+	pkt := refSeal(t, sk.SendCipherKey, sk.SendHMACKey, header, 1, pidb[:], testIPPacket)
+	plain, err := client.Decrypt(pkt)
+	if err != nil {
+		t.Fatalf("failed to decrypt standard P_DATA_V1 packet: %v", err)
+	}
+	if !bytes.Equal(plain, testIPPacket) {
+		t.Fatalf("unexpected plaintext: %x", plain)
+	}
+	// tampering with the (unauthenticated-by-AD) opcode key-id must not be
+	// what makes or breaks decryption, but tampering with the packet id must.
+	binary.BigEndian.PutUint32(pidb[:], 2)
+	pkt = refSeal(t, sk.SendCipherKey, sk.SendHMACKey, header, 2, pidb[:], testIPPacket)
+	pkt[4] ^= 0xff
+	if _, err := client.Decrypt(pkt); err == nil {
+		t.Fatal("expected authentication failure after packet id tamper")
+	}
+}
+
+// Regression guard: P_DATA_V2 must keep authenticating opcode|peer-id|packet-id.
+func TestDataChannelAEADV2StillAuthenticatesHeader(t *testing.T) {
+	ck, sk := v1TestKeys()
+	client, err := NewDataChannel(ck, CipherAES256GCM, AuthSHA1, 7, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	enc, err := client.Encrypt(testIPPacket)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := refOpen(t, ck.SendCipherKey, ck.SendHMACKey, enc, 4, enc[:8]); err != nil {
+		t.Fatalf("standard peer rejects our P_DATA_V2 packet: %v", err)
+	}
+	header := []byte{opcodeKeyID(PDataV2, 0), 0, 0, 7}
+	var pidb [4]byte
+	binary.BigEndian.PutUint32(pidb[:], 1)
+	pkt := refSeal(t, sk.SendCipherKey, sk.SendHMACKey, header, 1, append(append([]byte{}, header...), pidb[:]...), testIPPacket)
+	if _, err := client.Decrypt(pkt); err != nil {
+		t.Fatalf("failed to decrypt standard P_DATA_V2 packet: %v", err)
+	}
+}