diff --git a/src/main/java/org/ice4j/ice/harvest/AbstractTcpListener.java b/src/main/java/org/ice4j/ice/harvest/AbstractTcpListener.java index 17f3e4d3..42293652 100644 --- a/src/main/java/org/ice4j/ice/harvest/AbstractTcpListener.java +++ b/src/main/java/org/ice4j/ice/harvest/AbstractTcpListener.java @@ -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; /** @@ -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 { @@ -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 @@ -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); } /** diff --git a/src/main/java/org/ice4j/ice/harvest/AbstractUdpListener.java b/src/main/java/org/ice4j/ice/harvest/AbstractUdpListener.java index 0f041e92..8a4612cb 100644 --- a/src/main/java/org/ice4j/ice/harvest/AbstractUdpListener.java +++ b/src/main/java/org/ice4j/ice/harvest/AbstractUdpListener.java @@ -91,80 +91,6 @@ public static List getAllowedAddresses(int port) return addresses; } - /** - * Tries to parse the bytes in buf at offset off (and - * length len) 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 null. - * - * @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 buf, or null. - */ - 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. @@ -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? } } @@ -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 Buffer instance, creating it if necessary. diff --git a/src/main/java/org/ice4j/ice/harvest/SinglePortUdpHarvester.java b/src/main/java/org/ice4j/ice/harvest/SinglePortUdpHarvester.java index 90622c07..e80837e0 100644 --- a/src/main/java/org/ice4j/ice/harvest/SinglePortUdpHarvester.java +++ b/src/main/java/org/ice4j/ice/harvest/SinglePortUdpHarvester.java @@ -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.*; @@ -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. diff --git a/src/main/java/org/ice4j/ice/harvest/TcpHarvester.java b/src/main/java/org/ice4j/ice/harvest/TcpHarvester.java index 78768d17..847b4ac2 100644 --- a/src/main/java/org/ice4j/ice/harvest/TcpHarvester.java +++ b/src/main/java/org/ice4j/ice/harvest/TcpHarvester.java @@ -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 @@ -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);