Skip to content

Conversation

@justin-layerv
Copy link
Contributor

Summary

  • Fix GCM nonce to use all 12 bytes instead of only 8
  • Add random 4-byte prefix to increase nonce entropy
  • Maintain backward compatibility with existing implementations

Problem

The GCM nonce only used 8 of 12 bytes. Bytes 0-4 were always zero:

// Before (only 8 bytes used):
func (h *HeaderCurve) NonceBytes() []byte {
    var nonce [GCMNonceSize]byte  // 12 bytes, initialized to zero
    copy(nonce[4:12], h.HeaderCommon[16:24])  // Only 8 bytes!
    return nonce[:]  // nonce[0:4] always zero
}

This reduced effective nonce entropy and made patterns more predictable.

Solution

Use HeaderCommon bytes 12-16 (previously unused) as a random nonce prefix:

// After (all 12 bytes used):
func (h *HeaderCurve) NonceBytes() []byte {
    var nonce [GCMNonceSize]byte
    copy(nonce[0:4], h.HeaderCommon[12:16])  // Nonce prefix (4 bytes)
    copy(nonce[4:12], h.HeaderCommon[16:24]) // Counter (8 bytes)
    return nonce[:]
}

HeaderCommon Layout (24 bytes)

Bytes Field Usage
0-4 Preamble Random
4-8 Type/Size XOR'd with preamble
8-10 Version Major/Minor
10-12 Flags Protocol flags
12-16 Nonce Prefix NEW: Random prefix
16-24 Counter Transaction ID

Files Modified

File Change
nhp/core/scheme/curve/header.go Add SetNoncePrefix(), fix NonceBytes()
nhp/core/scheme/gmsm/header.go Add SetNoncePrefix(), fix NonceBytes()
nhp/core/packet.go Add SetNoncePrefix() to Header interface
nhp/core/initiator.go Call SetNoncePrefix() with random value
nhp/core/scheme/curve/header_test.go Add nonce verification tests

Backward Compatibility

  • Old senders: bytes 12-15 are zero → still works, just less entropy
  • New senders: bytes 12-15 are random → full entropy
  • No protocol version bump needed - responders just read bytes as-is

Test Plan

  • TestNonceBytesLength - Verify 12 bytes returned
  • TestNonceBytesContent - Verify prefix and counter placement
  • TestNonceNoZeroGap - Verify no zero gaps when values set
  • TestNonceUniqueness - Verify different inputs produce unique nonces

@justin-layerv justin-layerv self-assigned this Jan 4, 2026
@justin-layerv justin-layerv force-pushed the security/fix-gcm-nonce-init branch from f4a6f91 to d981241 Compare January 4, 2026 17:07
Previously, the GCM nonce only used 8 of 12 bytes, with bytes 0-4
always set to zero. This reduced entropy and made nonce patterns
more predictable.

Problem:
- NonceBytes() only copied 8-byte counter to positions 4-12
- Positions 0-4 were always zero, reducing effective nonce entropy

Solution:
- Use HeaderCommon bytes 12-16 (previously unused) as nonce prefix
- NonceBytes() now uses all 12 bytes: prefix (4) + counter (8)
- Initiator sets random prefix via SetNoncePrefix() on packet creation

Changes:
- nhp/core/scheme/curve/header.go: Add SetNoncePrefix(), fix NonceBytes()
- nhp/core/scheme/gmsm/header.go: Add SetNoncePrefix(), fix NonceBytes()
- nhp/core/packet.go: Add SetNoncePrefix() to Header interface
- nhp/core/initiator.go: Call SetNoncePrefix() with random value
- nhp/core/scheme/curve/header_test.go: Add nonce verification tests

Backward Compatibility:
- Old senders: bytes 12-15 are zero (still works, just less entropy)
- New senders: bytes 12-15 are random (full entropy)
- No protocol version bump needed
@justin-layerv justin-layerv force-pushed the security/fix-gcm-nonce-init branch from d981241 to 821d57a Compare January 4, 2026 17:09
Copy link
Contributor

@craftleon craftleon left a comment

Choose a reason for hiding this comment

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

Header bytes [12:16] are reserved in purpose. For opennhp project, prefer not use them as nonce prefix.

@craftleon
Copy link
Contributor

Change declined

@craftleon craftleon closed this Jan 7, 2026
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.

3 participants