Use RandomBytes for fake record envelope; reject trailing bytes in DeserializeConfiguration

[pquerna]

GetFakeRecord uses make([]byte, conf.Sizes.Envelope) for the envelope, producing all zeros. Real envelopes contain a random nonce + MAC tag, so fake records are distinguishable at rest. The fix is the same RandomBytes call already used for MaskingKey on the line above.

Separately, DeserializeConfiguration discards the offset from DecodeVector (_), silently accepting trailing bytes. DecodeServerKeyMaterial in skm.go already checks this; this brings the config parser in line.

Neither change affects on-wire behavior (the masking key is random, so the XOR output is indistinguishable either way).

Tests

First commit adds tests, second commit adds fixes.

• TestGetFakeRecord extended -- envelope is not all-zero
• TestDeserializeConfiguration_TrailingBytes -- rejects trailing bytes with empty and non-empty context
• TestGenerateKE2_FakeRecord -- fake records still produce valid KE2
• Full existing suite passes

[Copilot]

Pull request overview

This PR hardens OPAQUE’s “fake record” generation and configuration deserialization to better match real-world encodings and reduce distinguishability/leniency.

Changes:

• Make GetFakeRecord generate a random envelope (instead of zero-filled) to avoid trivially recognizable fake records at rest.
• Make DeserializeConfiguration reject any trailing bytes after the context vector for strict parsing.
• Add/extend tests covering fake record KE2 generation, non-zero fake envelopes, and trailing-byte rejection in configuration parsing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
opaque.go	Randomizes fake record envelope; enforces no trailing bytes in configuration deserialization.
tests/opaque_test.go	Adds trailing-bytes test for DeserializeConfiguration; strengthens TestGetFakeRecord to ensure envelope isn’t all-zero.
tests/server_test.go	Adds TestGenerateKE2_FakeRecord to ensure KE2 generation works with fake records across suites.

Comments suppressed due to low confidence (1)

opaque.go:205

• DeserializeConfiguration assigns Context directly from the DecodeVector slice, which aliases the caller-provided encoded buffer. This makes the returned Configuration mutable via external modifications to encoded and can lead to surprising behavior. Consider cloning the decoded context bytes (e.g., via slices.Clone/append([]byte(nil), ctx...)) before storing them in the Configuration.

	c := &Configuration{
		OPRF:    Group(encoded[0]),
		AKE:     Group(encoded[1]),
		KSF:     ksf.Identifier(encoded[2]),
		KDF:     crypto.Hash(encoded[3]),
		MAC:     crypto.Hash(encoded[4]),
		Hash:    crypto.Hash(encoded[5]),
		Context: ctx,
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

ke2.Serialize() is called twice in the KE2 size mismatch check, which can duplicate work and allocations if Serialize builds a new slice each call. Store the serialized bytes in a local variable and reuse it for both len(...) and the error message.

Suggested change

	if len(ke2.Serialize()) != conf.internal.Sizes.KE2 {
	t.Fatalf("KE2 size mismatch: got %d, want %d",
	len(ke2.Serialize()), conf.internal.Sizes.KE2)
	serializedKE2 := ke2.Serialize()
	if len(serializedKE2) != conf.internal.Sizes.KE2 {
	t.Fatalf("KE2 size mismatch: got %d, want %d",
	len(serializedKE2), conf.internal.Sizes.KE2)

[bytemare]

Hi @pquerna,

thank you for your suggestions!

The RFC states that the fake envelope should be an all 0 byte string, c.f. 6.3.2.2 towards the end. So I'd keep it like that. Have you encountered an incompatibility with other implementations?

Regarding the configuration deserialization, I see your point, and support the strict defensive approach. Have you seen a scenario where this has led to some risk?

If that's ok with you, I'd keep the deserialization patch and its test. Also, please rebase and cryptographically sign your commits, and sign them off following DCO.