Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify message integrity before accepting a new session. #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Verify message integrity before accepting a new session. #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 4 additions & 19 deletions src/main/java/org/ice4j/ice/harvest/AbstractTcpListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,7 @@ public boolean accept(DatagramPacket p)
* @throws IllegalStateException
* @throws IOException
*/
protected abstract void acceptSession(
Socket socket, String ufrag, DatagramPacket pushback)
protected abstract void acceptSession(Socket socket, DatagramPacket pushback)
throws IOException, IllegalStateException;

/**
Expand Down Expand Up @@ -966,8 +965,7 @@ else if (read > 0)
// if we had read enough data
if(channel.length <= bytesRead.length - 2)
{
processFirstDatagram(
preBuffered, channel, key);
processFirstDatagram(preBuffered, channel, key);
}
else
{
Expand Down Expand Up @@ -1065,18 +1063,6 @@ private void processFirstDatagram(
ChannelDesc channel, SelectionKey key)
throws IOException, IllegalStateException
{
// Does this look like a STUN binding request?
// What's the username?
String ufrag
= AbstractUdpListener.getUfrag(bytesRead,
(char) 0,
(char) bytesRead.length);

if (ufrag == null)
{
throw new IOException("Cannot extract ufrag");
}

// The rest of the stack will read from the socket's
// InputStream. We cannot change the blocking mode
// before the channel is removed from the selector (by
Expand All @@ -1086,14 +1072,13 @@ private void processFirstDatagram(

// Construct a DatagramPacket from the just-read packet
// which is to be pushed back
DatagramPacket p
= new DatagramPacket(bytesRead, bytesRead.length);
DatagramPacket p = new DatagramPacket(bytesRead, bytesRead.length);
Socket socket = channel.channel.socket();

p.setAddress(socket.getInetAddress());
p.setPort(socket.getPort());

acceptSession(socket, ufrag, p);
acceptSession(socket, p);
}

/**
Expand Down
90 changes: 2 additions & 88 deletions src/main/java/org/ice4j/ice/harvest/AbstractUdpListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,80 +91,6 @@ public static List<TransportAddress> getAllowedAddresses(int port)
return addresses;
}

/**
* Tries to parse the bytes in <tt>buf</tt> at offset <tt>off</tt> (and
* length <tt>len</tt>) as a STUN Binding Request message. If successful,
* looks for a USERNAME attribute and returns the local username fragment
* part (see RFC5245 Section 7.1.2.3).
* In case of any failure returns <tt>null</tt>.
*
* @param buf the bytes.
* @param off the offset.
* @param len the length.
* @return the local ufrag from the USERNAME attribute of the STUN message
* contained in <tt>buf</tt>, or <tt>null</tt>.
*/
static String getUfrag(byte[] buf, int off, int len)
{
// RFC5389, Section 6:
// All STUN messages MUST start with a 20-byte header followed by zero
// or more Attributes.
if (buf == null || buf.length < off + len || len < 20)
{
return null;
}

// RFC5389, Section 6:
// The magic cookie field MUST contain the fixed value 0x2112A442 in
// network byte order.
if ( !( (buf[off + 4] & 0xFF) == 0x21 &&
(buf[off + 5] & 0xFF) == 0x12 &&
(buf[off + 6] & 0xFF) == 0xA4 &&
(buf[off + 7] & 0xFF) == 0x42))
{
if (logger.isLoggable(Level.FINE))
{
logger.fine("Not a STUN packet, magic cookie not found.");
}
return null;
}

try
{
Message stunMessage
= Message.decode(buf,
(char) off,
(char) len);

if (stunMessage.getMessageType()
!= Message.BINDING_REQUEST)
{
return null;
}

UsernameAttribute usernameAttribute
= (UsernameAttribute)
stunMessage.getAttribute(Attribute.USERNAME);
if (usernameAttribute == null)
return null;

String usernameString
= new String(usernameAttribute.getUsername());
return usernameString.split(":")[0];
}
catch (Exception e)
{
// Catch everything. We are going to log, and then drop the packet
// anyway.
if (logger.isLoggable(Level.FINE))
{
logger.fine("Failed to extract local ufrag: " + e);
}
}

return null;
}

/**
* The map which keeps the known remote addresses and their associated
* candidateSockets.
Expand Down Expand Up @@ -322,15 +248,8 @@ private void runInHarvesterThread()
else
{
// Packet from an unknown source. Is it a STUN Binding Request?
String ufrag = getUfrag(buf.buffer, 0, buf.len);
if (ufrag == null)
{
// Not a STUN Binding Request or doesn't have a valid
// USERNAME attribute. Drop it.
continue;
}

maybeAcceptNewSession(buf, remoteAddress, ufrag);
maybeAcceptNewSession(buf, remoteAddress);
// Maybe add to #sockets here in the base class?
}
}
Expand Down Expand Up @@ -359,13 +278,8 @@ private void runInHarvesterThread()
* accepted socket.
* @param remoteAddress the remote address from which the datagram was
* received.
* @param ufrag the local ICE username fragment of the received STUN Binding
* Request.
*/
protected abstract void maybeAcceptNewSession(
Buffer buf,
InetSocketAddress remoteAddress,
String ufrag);
protected abstract void maybeAcceptNewSession(Buffer buf, InetSocketAddress remoteAddress);

/**
* Gets an unused <tt>Buffer</tt> instance, creating it if necessary.
Expand Down
52 changes: 46 additions & 6 deletions src/main/java/org/ice4j/ice/harvest/SinglePortUdpHarvester.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
package org.ice4j.ice.harvest;

import org.ice4j.*;
import org.ice4j.attribute.*;
import org.ice4j.ice.*;
import org.ice4j.message.*;
import org.ice4j.security.*;
import org.ice4j.socket.*;
import org.ice4j.stack.*;

Expand Down Expand Up @@ -127,25 +130,62 @@ public HarvestStatistics getHarvestStatistics()
* local ufrag of {@code ufrag}, and if one is found it accepts the new
* socket and adds it to the candidate.
*/
protected void maybeAcceptNewSession(Buffer buf,
InetSocketAddress remoteAddress,
String ufrag)
protected void maybeAcceptNewSession(Buffer buf, InetSocketAddress remoteAddress)
{
MyCandidate candidate = candidates.get(ufrag);
Message stunMessage;
try
{
stunMessage = Message.decode(buf.buffer, (char) 0, (char) buf.len);
}
catch (StunException e)
{
// Not a valid STUN Message
return;
}

if (stunMessage.getMessageType() != Message.BINDING_REQUEST)
{
return;
}

UsernameAttribute usernameAttribute = (UsernameAttribute) stunMessage.getAttribute(Attribute.USERNAME);
if (usernameAttribute == null)
{
return;
}

String localUfrag = new String(usernameAttribute.getUsername()).split(":")[0];
MyCandidate candidate = candidates.get(localUfrag);
if (candidate == null)
{
// A STUN Binding Request with an unknown USERNAME. Drop it.
return;
}

// This is a STUN Binding Request destined for this
MessageIntegrityAttribute messageIntegrityAttribute
= (MessageIntegrityAttribute) stunMessage.getAttribute(Attribute.MESSAGE_INTEGRITY);
if (!candidate.getStunStack().validateMessageIntegrity(
messageIntegrityAttribute,
LongTermCredential.toString(usernameAttribute.getUsername()),
!stunMessage.containsAttribute(Attribute.REALM) && !stunMessage.containsAttribute(Attribute.NONCE),
RawMessage.build(
buf.buffer,
buf.len,
new TransportAddress(remoteAddress, Transport.UDP),
localAddress)))
{
// Invalid message integrity
return;
}

// This is a verified STUN Binding Request destined for this
// specific Candidate/Component/Agent.
try
{
// 1. Create a socket for this remote address
// 2. Set-up de-multiplexing for future datagrams
// with this address to this socket.
MySocket newSocket = addSocket(remoteAddress, ufrag);
MySocket newSocket = addSocket(remoteAddress, localUfrag);

// 3. Let the candidate and its STUN stack no about the
// new socket.
Expand Down
52 changes: 47 additions & 5 deletions src/main/java/org/ice4j/ice/harvest/TcpHarvester.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
import java.util.logging.*;

import org.ice4j.*;
import org.ice4j.attribute.*;
import org.ice4j.ice.*;
import org.ice4j.message.*;
import org.ice4j.security.*;
import org.ice4j.socket.*;
import org.ice4j.stack.*;

/**
* An implementation of {@link AbstractTcpListener} which acts as a
Expand Down Expand Up @@ -455,15 +459,53 @@ private void purgeComponents()
* {@inheritDoc}
*/
@Override
protected void acceptSession(Socket socket, String ufrag,
DatagramPacket pushback)
protected void acceptSession(Socket socket, DatagramPacket pushback)
throws IOException, IllegalStateException
{
Component component = getComponent(ufrag);
Message stunMessage;
try
{
stunMessage = Message.decode(pushback.getData(), (char) 0, (char) pushback.getLength());
}
catch (StunException e)
{
// Not a valid STUN Message
throw new IllegalArgumentException("Invalid STUN message");
}

if (stunMessage.getMessageType() != Message.BINDING_REQUEST)
{
throw new IllegalArgumentException("Not a binding request");
}

UsernameAttribute usernameAttribute = (UsernameAttribute) stunMessage.getAttribute(Attribute.USERNAME);
if (usernameAttribute == null)
{
throw new IllegalArgumentException("No USERNAME present");
}

String localUfrag = new String(usernameAttribute.getUsername()).split(":")[0];
Component component = getComponent(localUfrag);
if (component == null)
{
throw new IllegalStateException(
"No component found for ufrag " + ufrag);
// A STUN Binding Request with an unknown USERNAME. Drop it.
throw new IllegalArgumentException("No associated component found");
}

MessageIntegrityAttribute messageIntegrityAttribute
= (MessageIntegrityAttribute) stunMessage.getAttribute(Attribute.MESSAGE_INTEGRITY);
if (!component.getParentStream().getParentAgent().getStunStack().validateMessageIntegrity(
messageIntegrityAttribute,
LongTermCredential.toString(usernameAttribute.getUsername()),
!stunMessage.containsAttribute(Attribute.REALM) && !stunMessage.containsAttribute(Attribute.NONCE),
RawMessage.build(
pushback.getData(),
pushback.getLength(),
new TransportAddress(pushback.getAddress(), pushback.getPort(), Transport.TCP),
new TransportAddress(socket.getLocalAddress(), socket.getLocalPort(), Transport.TCP))))
{
// A STUN Binding Request with an unknown USERNAME. Drop it.
throw new IllegalArgumentException("Message integrity verification failed.");
}

addSocketToComponent(socket, component, pushback);
Expand Down