Skip to content

[FEAT] 128 byte oprf support (breaks compatibility)#14

Open
Scratch-net wants to merge 2 commits into
mainfrom
128-byte-oprf
Open

[FEAT] 128 byte oprf support (breaks compatibility)#14
Scratch-net wants to merge 2 commits into
mainfrom
128-byte-oprf

Conversation

@Scratch-net
Copy link
Copy Markdown
Contributor

@Scratch-net Scratch-net commented Mar 25, 2026

Summary by CodeRabbit

Release Notes

  • Chores
    • Increased maximum OPRF range segment capacity to support larger data blocks
    • Optimized oblivious transfer pool sizing for improved performance and resource allocation
    • Updated attestor connection endpoint configuration
    • Enhanced sample provider pattern matching for broader data coverage

…putation

  Reduces OPRF latency from in high-latency environments
  by precomputing Oblivious Transfer before sessions.

  Protocol changes:
  - Precompute 100,000 OTs at TEE-pair connection time (after attestation)
  - Online phase: 2 rounds instead of 4 using derandomized OT
  - OT pool with 50,000 watermark triggers async extension of 50,000 more
  - TEEs reject client connections until OT pool ready and TEE-TEE connected

  Implementation:
  - New oprfmpc/ot_pool.go: OTPool (garbler) and OTReceiverPool (evaluator)
  - New tee_k/ot_precompute.go: blocking precomputation, async extension
  - New tee_t/ot_precompute.go: receiver-side OT handling
  - Rewritten oprfmpc/circuit.go: CMACGarblerOnline, CMACEvaluatorOnline
  - Delete all 4-round code paths (no legacy, no backwards compatibility)

  Proto changes:
  - Add OTPrecomputeRequest/Response/Complete, OPRFOnlineFull messages
  - Delete OPRFMPCRound1/2/3 messages
  - Add mandatory output_labels to OPRFMPCResult for garbler verification
  - Compact TEE-to-TEE field numbers (62-67 for OPRF/OT section)
  - Increase WebSocket message limit to 30 MB for 100K OT serialization

  Security:
  - Output label verification mandatory (detect malicious evaluator)
  - OT entries single-use, cleared on disconnect
  - Zero tolerance: any protocol error terminates session
  Reduces OPRF latency from in high-latency environments
  by precomputing Oblivious Transfer before sessions.

  Protocol changes:
  - Precompute 100,000 OTs at TEE-pair connection time (after attestation)
  - Online phase: 2 rounds instead of 4 using derandomized OT
  - OT pool with 50,000 watermark triggers async extension of 50,000 more
  - TEEs reject client connections until OT pool ready and TEE-TEE connected

  Implementation:
  - New oprfmpc/ot_pool.go: OTPool (garbler) and OTReceiverPool (evaluator)
  - New tee_k/ot_precompute.go: blocking precomputation, async extension
  - New tee_t/ot_precompute.go: receiver-side OT handling
  - Rewritten oprfmpc/circuit.go: CMACGarblerOnline, CMACEvaluatorOnline
  - Delete all 4-round code paths (no legacy, no backwards compatibility)

  Proto changes:
  - Add OTPrecomputeRequest/Response/Complete, OPRFOnlineFull messages
  - Delete OPRFMPCRound1/2/3 messages
  - Add mandatory output_labels to OPRFMPCResult for garbler verification
  - Compact TEE-to-TEE field numbers (62-67 for OPRF/OT section)
  - Increase WebSocket message limit to 30 MB for 100K OT serialization

  Security:
  - Output label verification mandatory (detect malicious evaluator)
  - OT entries single-use, cleared on disconnect
  - Zero tolerance: any protocol error terminates session
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

The changes scale OPRF MPC cryptographic operations from 64-byte to 128-byte ranges across the stack. This involves updating buffer sizes ([80]byte[144]byte for combined inputs), CMAC input bit counts (640 → 1152), OT pool consumption metrics (640 → 1152 OTs per OPRF), validation thresholds (64 → 128 bytes), and padding helper functions accordingly.

Changes

Cohort / File(s) Summary
Client-Side Range Building
client/redaction.go, demo_lib/main.go
Updated MPC range validation from 64-byte to 128-byte limit in buildOPRFMPCRanges(). Demo configuration also updated with new attestor endpoint and broader regex pattern for response redaction.
OPRF MPC Circuit Core
oprfmpc/circuit.go
Extended garbler/evaluator input buffers from [80]byte to [144]byte in CMACGarblerOnline() and CMACEvaluatorOnline(). Updated MPCL circuit to process 128-byte data (8 AES blocks instead of 4), expanded CMAC input bit count from 640 to 1152, and renamed padding function from PadZeros64 to PadZeros128 with corresponding array size increase.
OPRF MPC Constants & Pooling
oprfmpc/ot_pool.go
Increased OT pool sizing parameters: OTPoolInitialSize (100K → 180K), OTPoolExtendSize (50K → 90K), OTPoolWatermark (50K → 90K), and OT consumption per OPRF from 640 to 1152 to accommodate larger range processing.
OPRF MPC Tests
oprfmpc/circuit_crypto_test.go, oprfmpc/serialization_fuzz_test.go
Updated all CMAC test cases to use 128-byte input arrays ([144]byte garbler/evaluator buffers). Tests now recompute CMAC over zero-padded 128-byte messages and adjust key offset from 64 to 128. Extended AES block processing loop from 4 to 8 blocks.
Protocol Definition
proto/transport.proto
Updated OPRFRangeSpec.tls_length documentation constraint from max 64 to max 128.
TEE Range Processing
tee_k/oprf_handler.go, tee_t/oprf_evaluator.go
Extended TLS range validation and MPC input construction to support 128-byte limits. Updated padding calls from PadZeros64 to PadZeros128 and enlarged garbler/evaluator input buffers from [80]byte to [144]byte with corresponding offset adjustments for keystream/ciphertext placement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Oprf mpc #13: Directly modified the same buildOPRFMPCRanges() function to increase TLS length validation threshold from 64 to 128 bytes.

Poem

🐰 From sixty-four our bounds now soar,
To one-twenty-eight—much room for more!
AES blocks doubled, circuits wide,
With buffered cryptography our guide. ✨🔐

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: expanding OPRF support from 64 bytes to 128 bytes with compatibility implications. It is specific, concise, and directly reflects the primary modification across all affected files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 128-byte-oprf

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@promptless
Copy link
Copy Markdown

promptless Bot commented Mar 25, 2026

📝 Documentation updates detected!

New suggestion: Add changelog entry for MPC OPRF 128-byte capacity upgrade


Tip: Adjust how proactive or focused Promptless is in Agent Settings ⚙️

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
demo_lib/main.go (1)

225-231: ⚠️ Potential issue | 🟠 Major

addr no longer resolves to just the hostname.

(?<addr>.*)/\" is greedy and either captures the scheme/markup or fails entirely when the extractor returns the raw href value. paramValues["addr"] is still only www.trafficsafetymarketing.gov, so this sample will stop matching the expected response value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@demo_lib/main.go` around lines 225 - 231, The regex "(?<addr>.*)/\"" in the
extractor is greedy and can capture scheme/markup or fail when the extractor
returns a raw href; update the extraction logic used by the regex (referencing
the "regex" field and the named group "addr") to use a non-greedy or stricter
pattern that extracts only the hostname (e.g., match protocol optional then
capture host) so that the captured value matches the expected
paramValues["addr"] ("www.trafficsafetymarketing.gov"); ensure the change is
applied where the extractor uses the "xPath" and "regex" values so subsequent
matching against paramValues["addr"] succeeds.
tee_k/oprf_handler.go (1)

81-87: ⚠️ Potential issue | 🔴 Critical

Use overflow-safe range math before slicing the keystream.

r.TlsStart+r.TlsLength is evaluated as int32; a large untrusted TlsStart can wrap negative, bypass this bounds check, and then panic in ConsolidatedKeystream[r.TlsStart : r.TlsStart+r.TlsLength]. Validate start, length, and end in a wider type before calling initiateOPRFForRange.

🛡️ Proposed fix
 	for i, r := range teekState.OPRFRanges {
-		if r.TlsStart < 0 || r.TlsLength <= 0 || r.TlsLength > 128 {
+		start := int64(r.TlsStart)
+		length := int64(r.TlsLength)
+		end := start + length
+		if start < 0 || length <= 0 || length > 128 {
 			return fmt.Errorf("invalid range %d: start=%d length=%d", i, r.TlsStart, r.TlsLength)
 		}
-		if int(r.TlsStart+r.TlsLength) > len(teekState.ConsolidatedKeystream) {
+		if end > int64(len(teekState.ConsolidatedKeystream)) {
 			return fmt.Errorf("range %d exceeds keystream (end=%d, keystream_len=%d)",
-				i, r.TlsStart+r.TlsLength, len(teekState.ConsolidatedKeystream))
+				i, end, len(teekState.ConsolidatedKeystream))
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tee_k/oprf_handler.go` around lines 81 - 87, The bounds check uses int32
arithmetic and can overflow; compute start, length, and end in a wider signed
integer type (e.g., int64) before any addition or slice. For each
teekState.OPRFRanges entry (r.TlsStart, r.TlsLength) convert to int64, verify
start >= 0, length > 0 and <= 128, compute end := start + length, ensure end
does not overflow and end <= int64(len(teekState.ConsolidatedKeystream)), then
convert back to int for the slice and for calling initiateOPRFForRange to avoid
wrapping/panic when slicing ConsolidatedKeystream.
tee_t/oprf_evaluator.go (1)

38-44: ⚠️ Potential issue | 🔴 Critical

Fix integer overflow in range validation before casting to int.

r.TlsStart+r.TlsLength adds two int32 values, which can overflow before the cast to int. A sum like int32(1073741824) + int32(1073741824) wraps to a negative number, passes the validation at line 42, but then causes a panic or incorrect behavior when used directly in the slice at line 118.

Cast both operands to int64 before addition to prevent overflow:

Suggested fix
 	// Validate ranges against consolidated ciphertext
 	for i, r := range msg.GetRanges() {
 		if r.TlsStart < 0 || r.TlsLength <= 0 || r.TlsLength > 128 {
 			return fmt.Errorf("invalid range %d: start=%d length=%d", i, r.TlsStart, r.TlsLength)
 		}
-		if int(r.TlsStart+r.TlsLength) > len(teetState.ConsolidatedResponseCiphertext) {
+		end := int64(r.TlsStart) + int64(r.TlsLength)
+		if end > int64(len(teetState.ConsolidatedResponseCiphertext)) {
 			return fmt.Errorf("range %d exceeds ciphertext (end=%d, ciphertext_len=%d)",
-				i, r.TlsStart+r.TlsLength, len(teetState.ConsolidatedResponseCiphertext))
+				i, end, len(teetState.ConsolidatedResponseCiphertext))
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tee_t/oprf_evaluator.go` around lines 38 - 44, The range end check in the
loop over msg.GetRanges() uses r.TlsStart + r.TlsLength as int32 and can
overflow before casting to int; change the validation in the loop (where
r.TlsStart and r.TlsLength are inspected) to compute the sum as int64 (e.g.,
cast r.TlsStart and r.TlsLength to int64, add, compare against
int64(len(teetState.ConsolidatedResponseCiphertext))) and then safely convert
the validated end to int when slicing the ConsolidatedResponseCiphertext in any
subsequent use (refer to the loop over msg.GetRanges(), r.TlsStart, r.TlsLength,
and teetState.ConsolidatedResponseCiphertext to locate the code).
🧹 Nitpick comments (1)
oprfmpc/circuit_crypto_test.go (1)

418-436: Keep this as a true known-answer test.

Recomputing expectedCMAC with computeAESCMAC() makes this check self-referential, so the circuit and helper can drift together and still pass. Please pin the padded 128-byte tag from a trusted CMAC implementation, or rename this away from NISTVectors.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@oprfmpc/circuit_crypto_test.go` around lines 418 - 436, The test currently
computes expectedCMAC at runtime using computeAESCMAC(paddedMessage[:]), making
the known-answer test self-referential; replace that dynamic computation by
hardcoding the 128-byte zero-padded CMAC tag value (computed from a trusted CMAC
implementation) into expectedCMAC so the test asserts against a fixed reference
vector, and if this vector is not a NIST vector rename the test away from
NISTVectors; update references to expectedCMAC, paddedMessage, computeAESCMAC,
and the garblerInput/evaluatorInput setup accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/redaction.go`:
- Around line 499-505: The loop that checks OPRF range length currently logs and
continues when tlsLength > 128, which can silently drop all provider-declared
OPRF ranges; change this to fail fast by returning an error instead of
continuing. In the function that contains the loop (the code path that
eventually calls sendRedactionSpec), replace the "continue" after logging with a
returned error (e.g., fmt.Errorf or errors.Wrap) that includes context such as
tlsStart, tlsEnd, and tlsLength so callers can handle or surface the failure;
alternatively, if preferred, implement explicit splitting of the range (using
tlsStart/tlsEnd) into <=128-byte chunks before proceeding, but do not leave the
current "continue" behavior. Ensure the returned error propagates up to prevent
sendRedactionSpec from submitting an empty/no-OPRF redaction.

In `@oprfmpc/circuit.go`:
- Around line 405-414: The PadZeros128 helper (function PadZeros128) currently
doesn't validate negative dataLen and will panic on slice operations; add a
guard at the start to reject dataLen < 0 (returning an error like "dataLen
negative: %d") so callers receive a clear error instead of a runtime panic,
keeping existing checks for dataLen > 128 and len(data) < dataLen intact.

In `@tee_t/oprf_evaluator.go`:
- Around line 126-129: The CMAC input construction uses teetState.OPRFKeyShare
without checking its length, which can silently truncate or pad and corrupt the
CMAC; before copying into evaluatorInput (the 144-byte buffer) validate that
teetState.OPRFKeyShare has length exactly 16 bytes and return a clear error (or
panic) if not, then proceed to copy into evaluatorInput[:128] and
evaluatorInput[128:]; ensure the validation occurs in the same scope where
evaluatorInput is constructed so the CMAC/OPRF evaluation only runs with a
correctly-sized key share.

---

Outside diff comments:
In `@demo_lib/main.go`:
- Around line 225-231: The regex "(?<addr>.*)/\"" in the extractor is greedy and
can capture scheme/markup or fail when the extractor returns a raw href; update
the extraction logic used by the regex (referencing the "regex" field and the
named group "addr") to use a non-greedy or stricter pattern that extracts only
the hostname (e.g., match protocol optional then capture host) so that the
captured value matches the expected paramValues["addr"]
("www.trafficsafetymarketing.gov"); ensure the change is applied where the
extractor uses the "xPath" and "regex" values so subsequent matching against
paramValues["addr"] succeeds.

In `@tee_k/oprf_handler.go`:
- Around line 81-87: The bounds check uses int32 arithmetic and can overflow;
compute start, length, and end in a wider signed integer type (e.g., int64)
before any addition or slice. For each teekState.OPRFRanges entry (r.TlsStart,
r.TlsLength) convert to int64, verify start >= 0, length > 0 and <= 128, compute
end := start + length, ensure end does not overflow and end <=
int64(len(teekState.ConsolidatedKeystream)), then convert back to int for the
slice and for calling initiateOPRFForRange to avoid wrapping/panic when slicing
ConsolidatedKeystream.

In `@tee_t/oprf_evaluator.go`:
- Around line 38-44: The range end check in the loop over msg.GetRanges() uses
r.TlsStart + r.TlsLength as int32 and can overflow before casting to int; change
the validation in the loop (where r.TlsStart and r.TlsLength are inspected) to
compute the sum as int64 (e.g., cast r.TlsStart and r.TlsLength to int64, add,
compare against int64(len(teetState.ConsolidatedResponseCiphertext))) and then
safely convert the validated end to int when slicing the
ConsolidatedResponseCiphertext in any subsequent use (refer to the loop over
msg.GetRanges(), r.TlsStart, r.TlsLength, and
teetState.ConsolidatedResponseCiphertext to locate the code).

---

Nitpick comments:
In `@oprfmpc/circuit_crypto_test.go`:
- Around line 418-436: The test currently computes expectedCMAC at runtime using
computeAESCMAC(paddedMessage[:]), making the known-answer test self-referential;
replace that dynamic computation by hardcoding the 128-byte zero-padded CMAC tag
value (computed from a trusted CMAC implementation) into expectedCMAC so the
test asserts against a fixed reference vector, and if this vector is not a NIST
vector rename the test away from NISTVectors; update references to expectedCMAC,
paddedMessage, computeAESCMAC, and the garblerInput/evaluatorInput setup
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7672465-7b71-4601-8b09-f75306af1d18

📥 Commits

Reviewing files that changed from the base of the PR and between 12d5f9b and 3cb46bb.

⛔ Files ignored due to path filters (1)
  • proto/transport.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (9)
  • client/redaction.go
  • demo_lib/main.go
  • oprfmpc/circuit.go
  • oprfmpc/circuit_crypto_test.go
  • oprfmpc/ot_pool.go
  • oprfmpc/serialization_fuzz_test.go
  • proto/transport.proto
  • tee_k/oprf_handler.go
  • tee_t/oprf_evaluator.go

Comment thread client/redaction.go
Comment on lines +499 to 505
// Validate length fits in single OPRF (max 128 bytes)
tlsLength := tlsEnd - tlsStart
if tlsLength > 64 {
if tlsLength > 128 {
c.logger.Error("MPC OPRF range too long",
zap.Int("tls_length", tlsLength),
zap.Int("max", 64))
zap.Int("max", 128))
continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fail fast instead of silently dropping provider-declared OPRF ranges.

If every requested range hits this branch, sendRedactionSpec() later sends an empty submission, so a hash:"oprf-mpc" redaction is downgraded to “no OPRF needed”. Please return an error here, or split the range explicitly, instead of continue.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/redaction.go` around lines 499 - 505, The loop that checks OPRF range
length currently logs and continues when tlsLength > 128, which can silently
drop all provider-declared OPRF ranges; change this to fail fast by returning an
error instead of continuing. In the function that contains the loop (the code
path that eventually calls sendRedactionSpec), replace the "continue" after
logging with a returned error (e.g., fmt.Errorf or errors.Wrap) that includes
context such as tlsStart, tlsEnd, and tlsLength so callers can handle or surface
the failure; alternatively, if preferred, implement explicit splitting of the
range (using tlsStart/tlsEnd) into <=128-byte chunks before proceeding, but do
not leave the current "continue" behavior. Ensure the returned error propagates
up to prevent sendRedactionSpec from submitting an empty/no-OPRF redaction.

Comment thread oprfmpc/circuit.go
Comment on lines +405 to 414
// PadZeros128 pads data with zeros to exactly 128 bytes.
func PadZeros128(data []byte, dataLen int) ([128]byte, error) {
if dataLen > 128 {
return [128]byte{}, fmt.Errorf("dataLen too large: %d > 128", dataLen)
}
if len(data) < dataLen {
return [64]byte{}, fmt.Errorf("data slice too short: %d < %d", len(data), dataLen)
return [128]byte{}, fmt.Errorf("data slice too short: %d < %d", len(data), dataLen)
}
var padded [64]byte
var padded [128]byte
copy(padded[:dataLen], data[:dataLen])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Reject negative dataLen.

dataLen < 0 passes the current guards and then panics on the slice expressions below. Returning an error keeps this helper safe even if a caller misses the upfront range validation.

🐛 Proposed fix
 func PadZeros128(data []byte, dataLen int) ([128]byte, error) {
-	if dataLen > 128 {
-		return [128]byte{}, fmt.Errorf("dataLen too large: %d > 128", dataLen)
+	if dataLen < 0 || dataLen > 128 {
+		return [128]byte{}, fmt.Errorf("invalid dataLen: %d", dataLen)
 	}
 	if len(data) < dataLen {
 		return [128]byte{}, fmt.Errorf("data slice too short: %d < %d", len(data), dataLen)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// PadZeros128 pads data with zeros to exactly 128 bytes.
func PadZeros128(data []byte, dataLen int) ([128]byte, error) {
if dataLen > 128 {
return [128]byte{}, fmt.Errorf("dataLen too large: %d > 128", dataLen)
}
if len(data) < dataLen {
return [64]byte{}, fmt.Errorf("data slice too short: %d < %d", len(data), dataLen)
return [128]byte{}, fmt.Errorf("data slice too short: %d < %d", len(data), dataLen)
}
var padded [64]byte
var padded [128]byte
copy(padded[:dataLen], data[:dataLen])
// PadZeros128 pads data with zeros to exactly 128 bytes.
func PadZeros128(data []byte, dataLen int) ([128]byte, error) {
if dataLen < 0 || dataLen > 128 {
return [128]byte{}, fmt.Errorf("invalid dataLen: %d", dataLen)
}
if len(data) < dataLen {
return [128]byte{}, fmt.Errorf("data slice too short: %d < %d", len(data), dataLen)
}
var padded [128]byte
copy(padded[:dataLen], data[:dataLen])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@oprfmpc/circuit.go` around lines 405 - 414, The PadZeros128 helper (function
PadZeros128) currently doesn't validate negative dataLen and will panic on slice
operations; add a guard at the start to reject dataLen < 0 (returning an error
like "dataLen negative: %d") so callers receive a clear error instead of a
runtime panic, keeping existing checks for dataLen > 128 and len(data) < dataLen
intact.

Comment thread tee_t/oprf_evaluator.go
Comment on lines +126 to +129
// Build evaluator input: [128 bytes data][16 bytes key]
var evaluatorInput [144]byte
copy(evaluatorInput[:128], paddedCiphertext[:])
copy(evaluatorInput[128:], teetState.OPRFKeyShare)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd /tmp && find . -name "oprf_evaluator.go" -type f 2>/dev/null

Repository: reclaimprotocol/reclaim-tee

Length of output: 53


🏁 Script executed:

git ls-files | grep oprf_evaluator

Repository: reclaimprotocol/reclaim-tee

Length of output: 94


🏁 Script executed:

cat -n tee_t/oprf_evaluator.go | sed -n '120,135p'

Repository: reclaimprotocol/reclaim-tee

Length of output: 773


🏁 Script executed:

rg "OPRFKeyShare" -B 2 -A 2 tee_t/oprf_evaluator.go

Repository: reclaimprotocol/reclaim-tee

Length of output: 435


🏁 Script executed:

rg "type.*OPRFKeyShare\|OPRFKeyShare\s*\[" -A 1 -B 1

Repository: reclaimprotocol/reclaim-tee

Length of output: 53


🏁 Script executed:

rg "type.*struct" tee_t/oprf_evaluator.go | head -20

Repository: reclaimprotocol/reclaim-tee

Length of output: 53


🏁 Script executed:

rg "OPRFKeyShare\s*\[|OPRFKeyShare.*byte" -A 1 -B 1

Repository: reclaimprotocol/reclaim-tee

Length of output: 1820


🏁 Script executed:

rg "struct.*{" -A 20 | grep -A 20 "OPRFKeyShare"

Repository: reclaimprotocol/reclaim-tee

Length of output: 152


🏁 Script executed:

fd "\.go$" -x grep -l "OPRFKeyShare" {} \; | head -10

Repository: reclaimprotocol/reclaim-tee

Length of output: 208


🏁 Script executed:

cat -n tee_t/tee_t.go | grep -A 10 "initializeOPRFKeyShare"

Repository: reclaimprotocol/reclaim-tee

Length of output: 1245


🏁 Script executed:

cat -n tee_t/session_manager.go | grep -B 5 -A 5 "OPRFKeyShare ="

Repository: reclaimprotocol/reclaim-tee

Length of output: 53


🏁 Script executed:

rg "OPRFKeyShare\s*=" tee_t/ -B 2 -A 2

Repository: reclaimprotocol/reclaim-tee

Length of output: 366


🏁 Script executed:

cat -n tee_t/tee_t.go | sed -n '111,145p'

Repository: reclaimprotocol/reclaim-tee

Length of output: 1687


🏁 Script executed:

rg "len.*OPRFKeyShare|OPRFKeyShare.*len" -B 2 -A 2

Repository: reclaimprotocol/reclaim-tee

Length of output: 53


🏁 Script executed:

rg "if.*OPRFKeyShare|OPRFKeyShare.*if" -B 2 -A 2

Repository: reclaimprotocol/reclaim-tee

Length of output: 53


🏁 Script executed:

cat -n tee_t/tee_t.go | sed -n '140,160p'

Repository: reclaimprotocol/reclaim-tee

Length of output: 834


🏁 Script executed:

rg "initializeOPRFKeyShare|oprfKeyShare" tee_t/tee_t.go -B 1 -A 1

Repository: reclaimprotocol/reclaim-tee

Length of output: 1309


🏁 Script executed:

cat -n tee_t/oprf_evaluator.go | sed -n '100,130p'

Repository: reclaimprotocol/reclaim-tee

Length of output: 1460


Add validation for OPRFKeyShare length before use in CMAC evaluation.

Line 129's copy operation will silently truncate or zero-fill if teetState.OPRFKeyShare is not exactly 16 bytes, producing an incorrect CMAC input instead of a hard failure. Although initialization code validates the key share length, defensive validation at the point of use is necessary for cryptographic operations.

Suggested fix
 	// Build evaluator input: [128 bytes data][16 bytes key]
+	if len(teetState.OPRFKeyShare) != 16 {
+		return fmt.Errorf("invalid OPRF key share length: got %d, want 16", len(teetState.OPRFKeyShare))
+	}
 	var evaluatorInput [144]byte
 	copy(evaluatorInput[:128], paddedCiphertext[:])
 	copy(evaluatorInput[128:], teetState.OPRFKeyShare)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Build evaluator input: [128 bytes data][16 bytes key]
var evaluatorInput [144]byte
copy(evaluatorInput[:128], paddedCiphertext[:])
copy(evaluatorInput[128:], teetState.OPRFKeyShare)
// Build evaluator input: [128 bytes data][16 bytes key]
if len(teetState.OPRFKeyShare) != 16 {
return fmt.Errorf("invalid OPRF key share length: got %d, want 16", len(teetState.OPRFKeyShare))
}
var evaluatorInput [144]byte
copy(evaluatorInput[:128], paddedCiphertext[:])
copy(evaluatorInput[128:], teetState.OPRFKeyShare)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tee_t/oprf_evaluator.go` around lines 126 - 129, The CMAC input construction
uses teetState.OPRFKeyShare without checking its length, which can silently
truncate or pad and corrupt the CMAC; before copying into evaluatorInput (the
144-byte buffer) validate that teetState.OPRFKeyShare has length exactly 16
bytes and return a clear error (or panic) if not, then proceed to copy into
evaluatorInput[:128] and evaluatorInput[128:]; ensure the validation occurs in
the same scope where evaluatorInput is constructed so the CMAC/OPRF evaluation
only runs with a correctly-sized key share.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant