From 15b97238f3a5a0e5a76838e3326fff6da86b56d2 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Fri, 17 May 2013 15:31:44 +0000 Subject: QPID-4858: Prevent silent use of insecure HTTP connector when HTTP protocol with SSL transport was requested. Remove separate HTTPS protocol and use HTTP protocol with SSL transport for consistency with all other protocol types. git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1483866 13f79535-47bb-0310-9956-ffa450edef68 --- .../server/management/plugin/HttpManagement.java | 18 +++++++----------- .../src/main/java/resources/addPort.html | 3 +-- .../java/resources/js/qpid/management/addPort.js | 20 -------------------- .../store/ManagementModeStoreHandler.java | 1 - .../java/org/apache/qpid/server/model/Protocol.java | 1 - .../qpid/server/model/adapter/PortAdapter.java | 6 ------ .../qpid/server/model/adapter/PortFactory.java | 4 ++-- .../apache/qpid/systest/rest/BasicAuthRestTest.java | 4 ++-- .../qpid/systest/rest/BrokerRestHttpsTest.java | 2 +- 9 files changed, 13 insertions(+), 46 deletions(-) (limited to 'java') diff --git a/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java b/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java index 8e99b8d65a..d87a1755da 100644 --- a/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java +++ b/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java @@ -64,6 +64,7 @@ import org.apache.qpid.server.model.Protocol; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.model.Session; import org.apache.qpid.server.model.State; +import org.apache.qpid.server.model.Transport; import org.apache.qpid.server.model.TrustStore; import org.apache.qpid.server.model.User; import org.apache.qpid.server.model.VirtualHost; @@ -206,11 +207,6 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem return (Integer)getAttribute(TIME_OUT); } - private boolean isManagementHttp(Port port) - { - return port.getProtocols().contains(Protocol.HTTP) || port.getProtocols().contains(Protocol.HTTPS); - } - @SuppressWarnings("unchecked") private Server createServer(Collection ports) { @@ -227,15 +223,15 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem { continue; } - final Collection protocols = port.getProtocols(); + Connector connector = null; - //TODO: what to do if protocol HTTP and transport SSL? - if (protocols.contains(Protocol.HTTP)) + Collection transports = port.getTransports(); + if (!transports.contains(Transport.SSL)) { connector = new SelectChannelConnector(); } - else if (protocols.contains(Protocol.HTTPS)) + else if (transports.contains(Transport.SSL)) { KeyStore keyStore = port.getKeyStore(); if (keyStore == null) @@ -253,7 +249,7 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem } else { - throw new IllegalArgumentException("Unexpected protocol " + protocols); + throw new IllegalArgumentException("Unexpected transport on port " + port.getName() + ":" + transports); } lastPort = port.getPort(); connector.setPort(port.getPort()); @@ -365,7 +361,7 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem Collection httpPorts = new HashSet(); for (Port port : ports) { - if (isManagementHttp(port)) + if (port.getProtocols().contains(Protocol.HTTP)) { httpPorts.add(port); } diff --git a/java/broker-plugins/management-http/src/main/java/resources/addPort.html b/java/broker-plugins/management-http/src/main/java/resources/addPort.html index 8fcc4e890b..59abbadf32 100644 --- a/java/broker-plugins/management-http/src/main/java/resources/addPort.html +++ b/java/broker-plugins/management-http/src/main/java/resources/addPort.html @@ -66,9 +66,8 @@
diff --git a/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/addPort.js b/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/addPort.js index 705cffe7ff..0343d3393a 100644 --- a/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/addPort.js +++ b/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/addPort.js @@ -239,17 +239,6 @@ define(["dojo/_base/xhr", } disableTransportWidget = true; } - else if (newValue == "HTTP" && registry.byId("formAddPort.protocolsHTTP").value == "HTTPS") - { - if (transportWidget.value != "SSL") - { - transportWidget.set("value", "SSL"); - - // changing of transport widget value will cause the call to toggleSslWidgets - toggleSsl = false; - } - disableTransportWidget = true; - } if (toggleSsl) { toggleSslWidgets(newValue, transportWidget.value); @@ -290,15 +279,6 @@ define(["dojo/_base/xhr", registry.byId("formAddPort.authenticationProvider").set("disabled", isRMI); }); - registry.byId("formAddPort.protocolsHTTP").on("change", function(newValue){ - var isHTTPS = newValue == "HTTPS"; - var transportWidget = registry.byId("formAddPort.transports"); - if (isHTTPS && transportWidget.value != "SSL") { - transportWidget.set("value", "SSL"); - } - transportWidget.set("disabled", isHTTPS); - }); - theForm.on("submit", function(e) { event.stop(e); diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java index 574fe1ae2c..639f3cd5c4 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java +++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java @@ -298,7 +298,6 @@ public class ManagementModeStoreHandler implements ConfigurationEntryStore quiesce = managementModeRmiPortOverride > 0; break; case HTTP: - case HTTPS: quiesce = managementModeHttpPortOverride > 0; break; default: diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/Protocol.java b/java/broker/src/main/java/org/apache/qpid/server/model/Protocol.java index 6cd5eb23a4..e9d50fbc59 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/model/Protocol.java +++ b/java/broker/src/main/java/org/apache/qpid/server/model/Protocol.java @@ -34,7 +34,6 @@ public enum Protocol AMQP_1_0(ProtocolType.AMQP), JMX_RMI(ProtocolType.JMX), HTTP(ProtocolType.HTTP), - HTTPS(ProtocolType.HTTP), RMI(ProtocolType.RMI); private final ProtocolType _protocolType; diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java index de6ae06b94..0547f961d0 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java +++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java @@ -411,7 +411,6 @@ public class PortAdapter extends AbstractAdapter implements Port || (wantClientCertificate != null && wantClientCertificate.booleanValue()); String keyStoreName = (String) merged.get(KEY_STORE); - boolean hasKeyStore = keyStoreName != null; if(keyStoreName != null) { if (_broker.findKeyStoreByName(keyStoreName) == null) @@ -454,11 +453,6 @@ public class PortAdapter extends AbstractAdapter implements Port } } - if (protocols != null && protocols.contains(Protocol.HTTPS) && !hasKeyStore) - { - throw new IllegalConfigurationException("Can't create port which requires SSL but has no key store configured."); - } - if (protocols != null && protocols.contains(Protocol.RMI) && usesSsl) { throw new IllegalConfigurationException("Can't create RMI Registry port which requires SSL."); diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java index 2efe189d73..8dc446e5b2 100644 --- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java +++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortFactory.java @@ -124,7 +124,7 @@ public class PortFactory } Protocol protocol = protocols.iterator().next(); - if(!broker.isManagementMode()) + if(!broker.isManagementMode() && protocol.getProtocolType() != ProtocolType.HTTP) { //ManagementMode needs this relaxed to allow its overriding management ports to be inserted. @@ -150,7 +150,7 @@ public class PortFactory } } - if(port.getTransports().contains(Transport.SSL) || port.getProtocols().contains(Protocol.HTTPS)) + if(port.getTransports().contains(Transport.SSL)) { if(port.getKeyStore() == null) { diff --git a/java/systests/src/main/java/org/apache/qpid/systest/rest/BasicAuthRestTest.java b/java/systests/src/main/java/org/apache/qpid/systest/rest/BasicAuthRestTest.java index 1df1796e73..c8320a6d82 100644 --- a/java/systests/src/main/java/org/apache/qpid/systest/rest/BasicAuthRestTest.java +++ b/java/systests/src/main/java/org/apache/qpid/systest/rest/BasicAuthRestTest.java @@ -33,6 +33,7 @@ import org.apache.commons.configuration.ConfigurationException; import org.apache.qpid.server.management.plugin.HttpManagement; import org.apache.qpid.server.model.Port; import org.apache.qpid.server.model.Protocol; +import org.apache.qpid.server.model.Transport; import org.apache.qpid.test.utils.TestBrokerConfiguration; public class BasicAuthRestTest extends QpidRestTestCase @@ -58,9 +59,8 @@ public class BasicAuthRestTest extends QpidRestTestCase getRestTestHelper().setUseSsl(useSsl); if (useSsl) { - getBrokerConfiguration().setObjectAttribute(TestBrokerConfiguration.ENTRY_NAME_HTTP_PORT, Port.PROTOCOLS, Collections.singleton(Protocol.HTTPS)); + getBrokerConfiguration().setObjectAttribute(TestBrokerConfiguration.ENTRY_NAME_HTTP_PORT, Port.TRANSPORTS, Collections.singleton(Transport.SSL)); getBrokerConfiguration().setObjectAttribute(TestBrokerConfiguration.ENTRY_NAME_HTTP_PORT, Port.KEY_STORE, TestBrokerConfiguration.ENTRY_NAME_SSL_KEYSTORE); - } super.customizeConfiguration(); } diff --git a/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestHttpsTest.java b/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestHttpsTest.java index da38b9be33..18370185e5 100644 --- a/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestHttpsTest.java +++ b/java/systests/src/main/java/org/apache/qpid/systest/rest/BrokerRestHttpsTest.java @@ -52,7 +52,7 @@ public class BrokerRestHttpsTest extends QpidRestTestCase super.customizeConfiguration(); getRestTestHelper().setUseSsl(true); Map newAttributes = new HashMap(); - newAttributes.put(Port.PROTOCOLS, Collections.singleton(Protocol.HTTPS)); + newAttributes.put(Port.PROTOCOLS, Collections.singleton(Protocol.HTTP)); newAttributes.put(Port.TRANSPORTS, Collections.singleton(Transport.SSL)); newAttributes.put(Port.KEY_STORE, TestBrokerConfiguration.ENTRY_NAME_SSL_KEYSTORE); getBrokerConfiguration().setObjectAttributes(TestBrokerConfiguration.ENTRY_NAME_HTTP_PORT,newAttributes); -- cgit v1.2.1