Fix ToBytes corrupting entries lacking the 32-bit kvno extension (#2)

src/keytab/KeytabEntry.go

@@ -18,6 +18,10 @@ type KeytabEntry struct {
 	Vno8          uint8
 	Key           KeyBlock
 	Vno           uint32
+	// HasVno indicates whether the optional 32-bit key version number (Vno)
+	// extension is present in this entry. It controls whether Vno is serialized
+	// by ToBytes, so that entries read without the extension round-trip unchanged.
+	HasVno bool
 	// Internal
 	RawBytes     []byte
 	RawBytesSize uint32
@@ -81,12 +85,17 @@ func (k *KeytabEntry) FromBytes(data []byte) error {
 	k.RawBytesSize += k.Key.RawBytesSize
 
 	// Vno
+	// The 32-bit key version number is an optional extension: it is present only
+	// when at least 4 bytes remain in the entry record. When present it supersedes
+	// the 8-bit Vno8 value.
 	if len(data) >= 4 {
 		k.Vno = binary.BigEndian.Uint32(data[0:4])
+		k.HasVno = true
 		k.RawBytesSize += 4
 		// data = data[4:]
 	} else {
 		k.Vno = 0
+		k.HasVno = false
 	}
 
 	k.RawBytes = k.RawBytes[:k.RawBytesSize]
@@ -143,9 +152,12 @@ func (k *KeytabEntry) ToBytes() ([]byte, error) {
 	}
 	data = append(data, keyBytes...)
 
-	// Add the vno
-	binary.BigEndian.PutUint32(buffer4, k.Vno)
-	data = append(data, buffer4...)
+	// Add the vno only when the optional 32-bit key version number extension is
+	// present, so that entries read without it round-trip to the same bytes.
+	if k.HasVno {
+		binary.BigEndian.PutUint32(buffer4, k.Vno)
+		data = append(data, buffer4...)
+	}
 
 	// At the start of the data, add the size of the entry
 	binary.BigEndian.PutUint32(buffer4, uint32(len(data)))

src/keytab/KeytabEntry_kvno_test.go

@@ -0,0 +1,82 @@
+package keytab
+
+import (
+	"bytes"
+	"encoding/binary"
+	"testing"
+)
+
+// rawEntry builds the raw bytes of a keytab entry record, optionally appending
+// the 32-bit kvno extension.
+func rawEntry(withKvno bool, kvno uint32) []byte {
+	body := []byte{}
+
+	put16 := func(v uint16) { b := make([]byte, 2); binary.BigEndian.PutUint16(b, v); body = append(body, b...) }
+	put32 := func(v uint32) { b := make([]byte, 4); binary.BigEndian.PutUint32(b, v); body = append(body, b...) }
+
+	put16(1)                          // NumComponents
+	put16(17)                         // Realm length
+	body = append(body, []byte("TESTSEGMENT.local")...)
+	put16(6)                          // Component length
+	body = append(body, []byte("krbtgt")...)
+	put32(1)                          // NameType
+	put32(0)                          // Timestamp
+	body = append(body, 0x02)         // Vno8
+	put16(uint16(EncryptionType_RC4_HMAC))
+	put16(16)                         // Key length
+	body = append(body, bytes.Repeat([]byte{0x23}, 16)...)
+	if withKvno {
+		put32(kvno)
+	}
+
+	out := make([]byte, 4)
+	binary.BigEndian.PutUint32(out, uint32(len(body)))
+	return append(out, body...)
+}
+
+// Test_KeytabEntry_NoKvnoRoundTrip ensures an entry parsed without the 32-bit
+// kvno extension serializes back to the identical bytes (no spurious 4 bytes).
+func Test_KeytabEntry_NoKvnoRoundTrip(t *testing.T) {
+	in := rawEntry(false, 0)
+
+	entry := KeytabEntry{}
+	if err := entry.FromBytes(in); err != nil {
+		t.Fatalf("FromBytes failed: %v", err)
+	}
+	if entry.HasVno {
+		t.Fatalf("HasVno should be false for an entry without the kvno extension")
+	}
+
+	out, err := entry.ToBytes()
+	if err != nil {
+		t.Fatalf("ToBytes failed: %v", err)
+	}
+	if !bytes.Equal(in, out) {
+		t.Fatalf("round-trip changed the bytes:\n in (%d): % x\nout (%d): % x", len(in), in, len(out), out)
+	}
+}
+
+// Test_KeytabEntry_KvnoRoundTrip ensures the 32-bit kvno extension is preserved
+// across a parse/serialize round-trip.
+func Test_KeytabEntry_KvnoRoundTrip(t *testing.T) {
+	in := rawEntry(true, 0x01020304)
+
+	entry := KeytabEntry{}
+	if err := entry.FromBytes(in); err != nil {
+		t.Fatalf("FromBytes failed: %v", err)
+	}
+	if !entry.HasVno {
+		t.Fatalf("HasVno should be true for an entry with the kvno extension")
+	}
+	if entry.Vno != 0x01020304 {
+		t.Fatalf("expected Vno 0x01020304, got 0x%08x", entry.Vno)
+	}
+
+	out, err := entry.ToBytes()
+	if err != nil {
+		t.Fatalf("ToBytes failed: %v", err)
+	}
+	if !bytes.Equal(in, out) {
+		t.Fatalf("round-trip changed the bytes:\n in (%d): % x\nout (%d): % x", len(in), in, len(out), out)
+	}
+}