andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Ensure server preference order in ALPN

Browse Source 
Motivation:
With the current implementation the client protocol preference list
takes precedence over the one of the server, since the select method
will return the first item, in the client list, that matches any of the
protocols supported by the server. This violates the recommendation of
http://tools.ietf.org/html/rfc7301#section-3.2.

It will also fail with the current implementation of Chrome, which
sends back Extension application_layer_protocol_negotiation, protocols:
[http/1.1, spdy/3.1, h2-14]

Modifications:
Changed the protocol negotiator to prefer server’s list. Added a test
case that demonstrates the issue and that is fixed with the
modifications of this commit.

Result:
Server’s preference list is used.
This commit is contained in:
Leonardo Freitas Gomes 2015-03-14 20:03:40 +01:00 committed by Norman Maurer
parent 01c3fd3ace
commit 034e0bd8d2
4 changed files with 46 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
handler/src/main/java/io/netty/handler/ssl/JdkAlpnSslEngine.java
View File

@ -20,7 +20,7 @@ import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectionLi
import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector;
import io.netty.util.internal.PlatformDependent;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import javax.net.ssl.SSLEngine;
@ -64,7 +64,8 @@ final class JdkAlpnSslEngine extends JdkSslEngine {
if (server) {
final ProtocolSelector protocolSelector = checkNotNull(applicationNegotiator.protocolSelectorFactory()
.newSelector(this, new HashSet<String>(applicationNegotiator.protocols())), "protocolSelector");
.newSelector(this, new LinkedHashSet<String>(applicationNegotiator.protocols())),
"protocolSelector");
ALPN.put(engine, new ServerProvider() {
@Override
public String select(List<String> protocols) {

5
handler/src/main/java/io/netty/handler/ssl/JdkBaseApplicationProtocolNegotiator.java
View File

@ -142,9 +142,8 @@ class JdkBaseApplicationProtocolNegotiator implements JdkApplicationProtocolNego
@Override
public String select(List<String> protocols) throws Exception {
for (int i = 0; i < protocols.size(); ++i) {
String p = protocols.get(i);
if (supportedProtocols.contains(p)) {
for (String p : supportedProtocols) {
if (protocols.contains(p)) {
jettyWrapper.getSession().setApplicationProtocol(p);
return p;
}

5
handler/src/main/java/io/netty/handler/ssl/JdkNpnSslEngine.java
View File

@ -21,7 +21,7 @@ import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectionLi
import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector;
import io.netty.util.internal.PlatformDependent;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import javax.net.ssl.SSLEngine;
@ -88,7 +88,8 @@ final class JdkNpnSslEngine extends JdkSslEngine {
});
} else {
final ProtocolSelector protocolSelector = checkNotNull(applicationNegotiator.protocolSelectorFactory()
.newSelector(this, new HashSet<String>(applicationNegotiator.protocols())), "protocolSelector");
.newSelector(this, new LinkedHashSet<String>(applicationNegotiator.protocols())),
"protocolSelector");
NextProtoNego.put(engine, new ClientProvider() {
@Override
public boolean supports() {

48
handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java
View File

@ -17,6 +17,7 @@ package io.netty.handler.ssl;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeNoException;
import static org.mockito.Mockito.verify;
@ -61,7 +62,8 @@ import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
public class JdkSslEngineTest {
private static final String APPLICATION_LEVEL_PROTOCOL = "my-protocol";
private static final String PREFERRED_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http2";
private static final String FALLBACK_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http1_1";
private static final String APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE = "my-protocol-FOO";
@Mock
@ -123,6 +125,7 @@ public class JdkSslEngineTest {
clientChannel = null;
serverChannel = null;
serverConnectedChannel = null;
serverException = null;
}
@Test
@ -135,7 +138,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("NPN not on classpath");
}
JdkApplicationProtocolNegotiator apn = new JdkNpnApplicationProtocolNegotiator(true, true,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
mySetup(apn);
runTest();
} catch (RuntimeException e) {
@ -155,7 +158,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("NPN not on classpath");
}
JdkApplicationProtocolNegotiator clientApn = new JdkNpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkNpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE);
mySetup(serverApn, clientApn);
@ -177,7 +180,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("NPN not on classpath");
}
JdkApplicationProtocolNegotiator clientApn = new JdkNpnApplicationProtocolNegotiator(true, true,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkNpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE);
mySetup(serverApn, clientApn);
@ -200,7 +203,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("NPN not on classpath");
}
JdkApplicationProtocolNegotiator clientApn = new JdkNpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkNpnApplicationProtocolNegotiator(true, true,
APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE);
mySetup(serverApn, clientApn);
@ -223,7 +226,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("ALPN not on classpath");
}
JdkApplicationProtocolNegotiator apn = new JdkAlpnApplicationProtocolNegotiator(true, true,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
mySetup(apn);
runTest();
} catch (Exception e) {
@ -243,7 +246,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("ALPN not on classpath");
}
JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE);
mySetup(serverApn, clientApn);
@ -265,7 +268,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("ALPN not on classpath");
}
JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(false, false,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(true, true,
APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE);
mySetup(serverApn, clientApn);
@ -278,6 +281,31 @@ public class JdkSslEngineTest {
}
}
@Test
public void testAlpnCompatibleProtocolsDifferentClientOrder() throws Exception {
try {
// Typical code will not have to check this, but will get a initialization error on class load.
// Check in this test just in case we have multiple tests that just the class and we already ignored the
// initialization error.
if (!JdkAlpnSslEngine.isAvailable()) {
throw new RuntimeException("ALPN not on classpath");
}
// Even the preferred application protocol appears second in the client's list, it will be picked
// because it's the first one on server's list.
JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(false, false,
FALLBACK_APPLICATION_LEVEL_PROTOCOL, PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(true, true,
PREFERRED_APPLICATION_LEVEL_PROTOCOL, FALLBACK_APPLICATION_LEVEL_PROTOCOL);
mySetup(serverApn, clientApn);
assertNull(serverException);
runTest(PREFERRED_APPLICATION_LEVEL_PROTOCOL);
} catch (Exception e) {
// ALPN availability is dependent on the java version. If ALPN is not available because of
// java version incompatibility don't fail the test, but instead just skip the test
assumeNoException(e);
}
}
@Test
public void testAlpnNoCompatibleProtocolsClientHandshakeFailure() throws Exception {
try {
@ -288,7 +316,7 @@ public class JdkSslEngineTest {
throw new RuntimeException("ALPN not on classpath");
}
JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(true, true,
APPLICATION_LEVEL_PROTOCOL);
PREFERRED_APPLICATION_LEVEL_PROTOCOL);
JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(
new ProtocolSelectorFactory() {
@Override
@ -514,7 +542,7 @@ public class JdkSslEngineTest {
}
private void runTest() throws Exception {
runTest(APPLICATION_LEVEL_PROTOCOL);
runTest(PREFERRED_APPLICATION_LEVEL_PROTOCOL);
}
private void runTest(String expectedApplicationProtocol) throws Exception {