Skip to content

Commit

Permalink
fix: Improve isStunPacket check.
Browse files Browse the repository at this point in the history
Return early when the first two bits don't match. This is for:
1. Correctness: A packet that doesn't start with 00 is not STUN even if
   the magic cookie happens to match
2. Performance: Most packets are RTP and will fail the first byte check,
   so no need to check the magic cookie.
  • Loading branch information
bgrozev committed Jul 18, 2024
1 parent b9d5a03 commit 452da85
Showing 1 changed file with 30 additions and 36 deletions.
66 changes: 30 additions & 36 deletions src/main/java/org/ice4j/socket/StunDatagramPacketFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -177,47 +177,41 @@ public int hashCode()
*/
public static boolean isStunPacket(DatagramPacket p)
{
boolean isStunPacket = false;
byte[] data = p.getData();
int offset = p.getOffset();
int length = p.getLength();

// All STUN messages MUST start with a 20-byte header followed by zero
// or more Attributes.
if (length >= 20)
// All STUN messages MUST start with a 20-byte header followed by zero or more Attributes.
if (length < 20)
{
// If the MAGIC COOKIE is present this is a STUN packet (RFC5389
// compliant).
if (data[offset + 4] == Message.MAGIC_COOKIE[0]
&& data[offset + 5] == Message.MAGIC_COOKIE[1]
&& data[offset + 6] == Message.MAGIC_COOKIE[2]
&& data[offset + 7] == Message.MAGIC_COOKIE[3])
{
isStunPacket = true;
}
// Else, this packet may be a STUN packet (RFC3489 compliant). To
// determine this, we must continue the checks.
else
{
// The most significant 2 bits of every STUN message MUST be
// zeroes. This can be used to differentiate STUN packets from
// other protocols when STUN is multiplexed with other protocols
// on the same port.
byte b0 = data[offset];
boolean areFirstTwoBitsValid = ((b0 & 0xC0) == 0);

// Checks if the length of the data correspond to the length
// field of the STUN header. The message length field of the
// STUN header does not include the 20-byte of the STUN header.
int total_header_length
= ((((int)data[2]) & 0xff) << 8)
+ (((int) data[3]) & 0xff)
+ 20;
boolean isHeaderLengthValid = (length == total_header_length);

isStunPacket = areFirstTwoBitsValid && isHeaderLengthValid;
}
return false;
}

// The most significant 2 bits of every STUN message MUST be zeroes. This can be used to differentiate STUN
// packets from other protocols when STUN is multiplexed with other protocols on the same port.
if ((data[offset] & 0xC0) != 0)
{
return false;
}

// If the MAGIC COOKIE is present this is a STUN packet (RFC5389 compliant).
if (data[offset + 4] == Message.MAGIC_COOKIE[0]
&& data[offset + 5] == Message.MAGIC_COOKIE[1]
&& data[offset + 6] == Message.MAGIC_COOKIE[2]
&& data[offset + 7] == Message.MAGIC_COOKIE[3])
{
return true;
}
// Else, this packet may be a STUN packet (RFC3489 compliant). To determine this, we must continue the checks.
else
{
// Checks if the length of the data correspond to the length field of the STUN header. The message length
// field of the STUN header does not include the 20-byte of the STUN header.
int total_header_length
= ((((int)data[2]) & 0xff) << 8)
+ (((int) data[3]) & 0xff)
+ 20;
return (length == total_header_length);
}
return isStunPacket;
}
}

0 comments on commit 452da85

Please sign in to comment.