From 80b526be7df3471876eceaa2c610747ca7fb89da Mon Sep 17 00:00:00 2001
From: Timothy Bish <tabish121@gmail.com>
Date: Wed, 14 Oct 2015 11:38:50 -0400
Subject: [PATCH] https://issues.apache.org/jira/browse/AMQ-6010

Fix for failed SSL connections not releasing the connection count in the
transport which leads to connections being rejected as having eceeded
the maximum configured connections.
---
 .../amqp/AmqpProtocolDiscriminator.java       |   1 +
 ...MSMaxConnectionsSSLHandshakeFailsTest.java | 161 ++++++++++++++++++
 .../src/test/resources/alternative.keystore   | Bin 0 -> 2201 bytes
 3 files changed, 162 insertions(+)
 create mode 100644 activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/JMSMaxConnectionsSSLHandshakeFailsTest.java
 create mode 100644 activemq-amqp/src/test/resources/alternative.keystore

diff --git a/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/AmqpProtocolDiscriminator.java b/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/AmqpProtocolDiscriminator.java
index 9ae478705b..6c0326a1ce 100644
--- a/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/AmqpProtocolDiscriminator.java
+++ b/activemq-amqp/src/main/java/org/apache/activemq/transport/amqp/AmqpProtocolDiscriminator.java
@@ -101,6 +101,7 @@ public class AmqpProtocolDiscriminator implements AmqpProtocolConverter {
 
     @Override
     public void onAMQPException(IOException error) {
+        transport.sendToActiveMQ(error);
     }
 
     @Override
diff --git a/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/JMSMaxConnectionsSSLHandshakeFailsTest.java b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/JMSMaxConnectionsSSLHandshakeFailsTest.java
new file mode 100644
index 0000000000..55bf26566b
--- /dev/null
+++ b/activemq-amqp/src/test/java/org/apache/activemq/transport/amqp/JMSMaxConnectionsSSLHandshakeFailsTest.java
@@ -0,0 +1,161 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.transport.amqp;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.io.File;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Collection;
+
+import javax.jms.Connection;
+
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * Test that failed SSL Handshakes don't leave the transport in a bad sate.
+ */
+@RunWith(Parameterized.class)
+public class JMSMaxConnectionsSSLHandshakeFailsTest extends JMSClientTestSupport {
+
+    private static final int MAX_CONNECTIONS = 10;
+
+    private final String connectorScheme;
+
+    @Parameters(name="{0}")
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][] {
+                {"amqp+ssl"},
+                {"amqp+nio+ssl"},
+            });
+    }
+
+    public JMSMaxConnectionsSSLHandshakeFailsTest(String connectorScheme) {
+        this.connectorScheme = connectorScheme;
+    }
+
+    @Test(timeout = 60000)
+    public void testFailedSSLConnectionAttemptsDoesNotBreakTransport() throws Exception {
+
+        for (int i = 0; i < MAX_CONNECTIONS; ++i) {
+            try {
+                createFailingConnection();
+                fail("Should not be able to connect.");
+            } catch (Exception ex) {
+                LOG.debug("Connection failed as expected");
+            }
+        }
+
+        for (int i = 0; i < MAX_CONNECTIONS; ++i) {
+            try {
+                createNonSslConnection().start();;
+                fail("Should not be able to connect.");
+            } catch (Exception ex) {
+                LOG.debug("Connection failed as expected");
+            }
+        }
+
+        for (int i = 0; i < MAX_CONNECTIONS; ++i) {
+            try {
+                createGoodConnection();
+                LOG.debug("Connection created as expected");
+            } catch (Exception ex) {
+                fail("Should be able to connect: " + ex.getMessage());
+            }
+        }
+
+        assertEquals(0, getProxyToBroker().getCurrentConnectionsCount());
+    }
+
+    protected Connection createNonSslConnection() throws Exception {
+        return new JmsConnectionFactory(getGoodClientConnectURI(false)).createConnection();
+    }
+
+    protected Connection createFailingConnection() throws Exception {
+        return new JmsConnectionFactory(getBadClientConnectURI()).createConnection();
+    }
+
+    protected Connection createGoodConnection() throws Exception {
+        return new JmsConnectionFactory(getGoodClientConnectURI(true)).createConnection();
+    }
+
+    protected URI getGoodClientConnectURI(boolean useSsl) throws Exception {
+        URI brokerURI = getBrokerURI();
+
+        String amqpURI = (useSsl ? "amqps://" : "amqp://") + brokerURI.getHost() + ":" + brokerURI.getPort();
+
+        if (useSsl) {
+            amqpURI = amqpURI + "?transport.verifyHost=false";
+        }
+
+        return new URI(amqpURI);
+    }
+
+    protected URI getBadClientConnectURI() throws Exception {
+        URI brokerURI = getBrokerURI();
+
+        String amqpURI = "amqps://" + brokerURI.getHost() + ":" + brokerURI.getPort() +
+                         "?transport.verifyHost=false" +
+                         "&transport.keyStoreLocation=" + getUntrustedKeyStoreLocation();
+
+        return new URI(amqpURI);
+    }
+
+    protected String getUntrustedKeyStoreLocation() {
+        File brokerKeyStore = new File(System.getProperty("javax.net.ssl.keyStore"));
+        File untrustedStore = new File(brokerKeyStore.getParent(), "alternative.keystore");
+
+        return untrustedStore.toString();
+    }
+
+    //----- Configure the test support plumbing for this test ----------------//
+
+    @Override
+    protected String getAdditionalConfig() {
+        return "&transport.needClientAuth=true&maximumConnections=" + MAX_CONNECTIONS;
+    }
+
+    @Override
+    protected boolean isUseTcpConnector() {
+        return false;
+    }
+
+    @Override
+    protected boolean isUseSslConnector() {
+        return connectorScheme.equals("amqp+ssl");
+    }
+
+    @Override
+    protected boolean isUseNioPlusSslConnector() {
+        return connectorScheme.equals("amqp+nio+ssl");
+    }
+
+    @Override
+    protected URI getBrokerURI() {
+        if (connectorScheme.equals("amqp+ssl")) {
+            return amqpSslURI;
+        } else {
+            return amqpNioPlusSslURI;
+        }
+    }
+}
diff --git a/activemq-amqp/src/test/resources/alternative.keystore b/activemq-amqp/src/test/resources/alternative.keystore
new file mode 100644
index 0000000000000000000000000000000000000000..6ab1286b15983301f75ffdaa630be3ed4c96807c
GIT binary patch
literal 2201
zcmchY`8U)J8^*ui8OAoU49SE-Q54@X_DV5mB57m^*|TLg#ug8T7MW;L#+vNLma=8d
z+G7i$DHM`kS<Ai+UeEiU&gr~=!284X!{<8J5BIsweeQdAes>-K0NB33|A^Hi$jvcm
zpVYk+?H&UFD+rNUcAx_YPFV!V0?NSnK#&z+A+t;y?Cizr{9OYv(WR&L@UH4ZznF_2
zvjXVurMrnzwbAaSk!`s*nc?Ae8iP*hB}Y(UdKMe_QmKLHLUb>~$+zl-r@Eyu8_hgJ
zvj>h?O$VBnh1X~Xb-|RkKA9{Yh=I;-3bNAP!#oM0^CUcPeAmTNeg2*gyU)_HgjCKX
z$f!Wfh8awB4UM@^afL4htdd$tV!`C0mdzB#*(g#?*e+>^Z~aSC(aS4Q%`q}BvvFsl
zu3~lbFTUv#{cu&bFMZ@IeMvh9P=G<@0iw%UqAUnC+7S2ioG?}CZjmWZAZ5<6OM~*f
zM3Y0%CbW$BGjXsWr5<rX-68%x4#S2%Ig!@G>}v`|5zNk3su`bA%@J0_4mfg2zs?qT
zv%Rxro67a{W1W8-d*zAb%FOZeVU4=RZ00**0gUk@3yz#Inzk)tRU(0F*(jKTp=#Pw
zRrcxR4POdkFh|ESfptZA#l+vzKO}x>QQGQja=(O!*9?!@Je|7gaj<f*WT7v@pZVaJ
z!$Vwy2gm6mReMEx7TGbr(99|Rd>o2s%H$ytXR*5nfEi{kzq?V1+tRt<dGyDq%QpqO
z70Of%KkJ%30{BZitj{iHhb=p*CURwM*Wt}hw22sFPPw)JV2BgT);f7lLYsEFa|KTX
zB8#m;g=S5CV*-DAb70X~;_Tg&VTbt&SYu=8T$-N<O8zS0aC_u&&2I`G(Zjdw5t4ky
z)!1BFQ}(8J1&KyUI&_=2C*7eit)$hWs!x$Y5h{CpR0{NCokDzTb!W0CRsOD?{+-m=
z5&4m<dD2mv>vbe;<m<~y%@-S~8Q%|FXwo?)QuO(NnQ%~9@WqWvGsi{O>_<jgkzl{`
zeM+2k&~11`=<q<l;L1QlhK>-w%gic1ZE;+g!%AF;r9f1_II*+P=1^+PfIc<W%Oo=m
zn--@aHz*S{W4tWQhh-^#A#SsCHcxEkOToMTCCU437GaD&K8uncJawh79&=>I51i|3
z#%YX|Pg*nNt@F3Ex!tCyUBt-yQ=0`RR~{|;IT4&y=+)jLRf0k-pK4X?dp>A>GJLTE
zd6iKe&e7_nyqVt0FdAY@to(Rw0E09p+uPc{Gv^Y~vl@3NQp5n3pqo)La-5YZ82q5M
z7wp%0uS@Nim~)twaa$UFsrM#sOy%Q)d99X!-1hwWw98rxYD1b2iKvOR9YxY4&=79E
z+T-;gIpZ5UDh2PPrr0{_jIY96laBn-s+7!HaC1}W?DYmklDEsUD1C5<jCo@_-7Up`
zrQZm;QK9o_Hks~1_sRGxbjr==*X!x@ug>EzVd8Qd{sy&N>`pWfesM=$-`?Ihq#}<^
zURT#wn#Lci6!qPjX|!ljA2qMN#zC&nJ3J?oTD$w!%Pu8dy-Y6g$ZU3uT(pf`#^cIW
zUFjjt3nG%+!3mKaR34-5uxlOXhw-uY88QY*@AEI!P^^t^MOCw{%|qC4-&dM^jckFy
zZ@h#&u@A|&z)}Aj`UzV_-1pY23x(3YOK;UXlIpw}FqN2Rj|=&~d;XfurIjV<%f~JD
z%)F9c3~FvCQL#>52HgEwl=bS{H{P|0UQn~z!(CG@B)&N$$V-s=6kp5Z$kthm_VvJs
zeyc?MDgwIy<ur&QY3@CFp7kL{Tkqi}K@jL&w#8;XG;HR0MTq_HUA!)*1gQZ4+z2AW
zuRvs&@^d&0guz&@{?&F2;zn@D+{NEr=LSI-D*(A-xF8M$+=K;zf|*?V4eUr5PV+ag
zAz_y_F@g~4pBN_+Y3JbQ>hI{`Bj@Go{M+XGADbWI{U;4aa=3dr*txrS5&a-BA#RKU
z2EwRd6ctq!F&6u%@(2DuM<Ig}e-^R-Cg5a{2Y|>RCyWe&K+RY9gv5q!h<5!6s|&^&
zgALCG1-cpue*X0o$w*o7V5{mW{rZp|6N&~6DYvV+y?M_6NUhfO(Y<H+T0Z0VP8N3n
zE2=%{-WG;z$pYO0hYOFl8eQ||EPdvAn$uKSWFRj=lUNv1gwL^z4LNq;gkEZo7HM5~
zp_iPaWQrn$Y~1&z3Ipmgfy~5f2Cl0=w=$ZhDK*YEhibtF<<9sP&NIeY+L+rxy-K^d
zaR}6Y0!w>mX!I+Spe!*7>pEg!nlEw6T*>@#mTYNzceRj7X5i>y>|(^oE8A+{hGIXB
zn7<FoEQ`^(j%Js=C!BL?T{-4X8pF*cA*V*HXx#5tVQ>%t|7>V~RrVVK0T};8RA0J7
zW3$e^EvPv*R^*v#hG&V~Ha?=jPOG_XEk!=K#S&w#AOs(tS$b=eNH1Sc5=9SMdxO!@
z8U_-52bmL9YSy2x>jYX{1S3Fm-|Eu#)vIKITJhU~3rGVh(m35oFzghb%;`{6aVTY2
z+2nLNt$vViMQ~0<<tUk*af3ZWAVu8f$p{PWb5Wc`$r(aTjk;^yUE6_RaW)i^5WW{t
zDSphj2ZNnd=JI`EG&jrJCFwY0!mF{~rGU0>=-fWMfqDAL?T6FDn3fH${Aa^gKCUEi
z%ehp0rnCx)JfhJ%&}@SN8V5(1b&?hKD!BU=%b#7}xpv_UyVz3^4MGWd0=Ad7b7hpL
NFTHHfLTlaA|2O?e%9#KF

literal 0
HcmV?d00001