From e3dcb3115a12506277311dc383c76b04068c6f6b Mon Sep 17 00:00:00 2001 From: "Remi GASCOU (Podalirius)" <79218792+p0dalirius@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:37:58 +0200 Subject: [PATCH] Fix ToBytes corrupting entries lacking the 32-bit kvno extension (#2) ToBytes always appended a 32-bit key version number to every entry, even those parsed without the optional 32-bit kvno extension. This added 4 bytes per entry and inflated each entry's size field, so loading a standard keytab and writing it back produced a larger, corrupt file (example.kt: 204 -> 216 bytes). Track whether the extension was present when parsing (HasVno) and emit the 32-bit vno from ToBytes only when it is, so entries without the extension round-trip to identical bytes. Fixes #2 --- src/keytab/KeytabEntry.go | 18 +++++-- src/keytab/KeytabEntry_kvno_test.go | 82 +++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 src/keytab/KeytabEntry_kvno_test.go diff --git a/src/keytab/KeytabEntry.go b/src/keytab/KeytabEntry.go index b3eb5df..d141613 100644 --- a/src/keytab/KeytabEntry.go +++ b/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))) diff --git a/src/keytab/KeytabEntry_kvno_test.go b/src/keytab/KeytabEntry_kvno_test.go new file mode 100644 index 0000000..ce9f1f9 --- /dev/null +++ b/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) + } +}