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

9 files changed, 13 insertions, 46 deletions

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<Port> ports)
     {
@@ -227,15 +223,15 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem
             {
                 continue;
             }
-            final Collection<Protocol> protocols = port.getProtocols();
+
             Connector connector = null;
 
-            //TODO: what to do if protocol HTTP and transport SSL?
-            if (protocols.contains(Protocol.HTTP))
+            Collection<Transport> 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<Port> httpPorts = new HashSet<Port>();
         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 @@
             </div>
             <div id="formAddPort:fieldsHTTP">
                 <select id="formAddPort.protocolsHTTP" name="protocols" data-dojo-type="dijit.form.FilteringSelect"
-                    data-dojo-props="name: 'protocols', value: '', label: 'HTTP protocol*:'" missingMessage="HTTP protocol must be supplied">
+                    data-dojo-props="name: 'protocols', value: 'HTTP', label: 'HTTP protocol*:'" missingMessage="HTTP protocol must be supplied">
                     <option value="HTTP">HTTP</option>
-                    <option value="HTTPS">HTTPS</option>
                 </select>
             </div>
             <div id="formAddPort:transport" >
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<String, Object> newAttributes = new HashMap<String, Object>();
-        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);