PROTON-1962: update some defaults and related handling

Author: gemmellr

proton-j/src/main/java/org/apache/qpid/proton/engine/SslDomain.java

@@ -49,15 +49,24 @@ public enum Mode
     /**
      * Determines the level of peer validation.
      *
-     * {@link #ANONYMOUS_PEER} is configured by default.
+     * {@link #VERIFY_PEER_NAME} is used by default in {@link Mode#CLIENT client}
+     * mode if not configured otherwise, with {@link #ANONYMOUS_PEER} used for
+     * {@link Mode#SERVER server} mode if not configured otherwise.
      */
     public enum VerifyMode
     {
         /**
-         * will only connect to those peers that provide a valid identifying certificate signed
-         * by a trusted CA and are using an authenticated cipher
+         * Requires peers provide a valid identifying certificate signed by
+         * a trusted certificate. Does not verify hostname details of the
+         * peer certificate, use {@link #VERIFY_PEER_NAME} for this instead.
          */
         VERIFY_PEER,
+        /**
+         * Requires peers provide a valid identifying certificate signed
+         * by a trusted certificate, including verifying hostname details
+         * of the certificate using peer details provided when configuring
+         * TLS via {@link Transport#ssl(SslDomain, SslPeerDetails)}.
+         */
         VERIFY_PEER_NAME,
         /**
          * does not require a valid certificate, and permits use of ciphers that

proton-j/src/main/java/org/apache/qpid/proton/engine/Transport.java

@@ -194,15 +194,21 @@ public static Transport create() {
      * {@link Ssl} object, regardless of the parameters supplied.
      *
      * @param sslDomain the SSL settings to use
-     * @param sslPeerDetails may be null, in which case SSL session resume will not be attempted
+     * @param sslPeerDetails peer details, used for SNI, hostname verification, etc when connecting. May be null.
      * @return an {@link Ssl} object representing the SSL session.
+     * @throws IllegalArgumentException if the sslDomain requests hostname verification but sslPeerDetails are null.
+     * @throws IllegalStateException if the sslDomain has not been initialised.
      */
-    Ssl ssl(SslDomain sslDomain, SslPeerDetails sslPeerDetails);
+    Ssl ssl(SslDomain sslDomain, SslPeerDetails sslPeerDetails) throws IllegalArgumentException;
 
     /**
-     * As per {@link #ssl(SslDomain, SslPeerDetails)} but no attempt is made to resume a previous SSL session.
+     * Equivalent to {@link #ssl(SslDomain, SslPeerDetails)} but passing null for SslPeerDetails, meaning no SNI detail
+     * is sent, hostname verification isn't supported etc when connecting.
+     *
+     * @throws IllegalArgumentException if the sslDomain requests hostname verification.
+     * @throws IllegalStateException if the sslDomain has not been initialised.
      */
-    Ssl ssl(SslDomain sslDomain);
+    Ssl ssl(SslDomain sslDomain) throws IllegalArgumentException;
 
 
     /**

proton-j/src/main/java/org/apache/qpid/proton/engine/impl/ssl/SimpleSslTransportWrapper.java

@@ -360,9 +360,15 @@ public void process() throws TransportException
         try {
             unwrapInput();
         } catch (SSLException e) {
-            _logger.log(Level.WARNING, e.getMessage());
+            if(_logger.isLoggable(Level.FINEST)){
+                _logger.log(Level.FINEST, e.getMessage(), e);
+            } else {
+                _logger.log(Level.WARNING, e.getMessage());
+            }
             _inputBuffer.position(_inputBuffer.limit());
             _tail_closed = true;
+
+            throw new TransportException(e);
         } finally {
             _inputBuffer.compact();
         }

proton-j/src/main/java/org/apache/qpid/proton/engine/impl/ssl/SslDomainImpl.java

@@ -19,15 +19,14 @@
 package org.apache.qpid.proton.engine.impl.ssl;
 
 import javax.net.ssl.SSLContext;
-import org.apache.qpid.proton.ProtonUnsupportedOperationException;
 import org.apache.qpid.proton.engine.ProtonJSslDomain;
 import org.apache.qpid.proton.engine.SslDomain;
 import org.apache.qpid.proton.engine.SslPeerDetails;
 
 public class SslDomainImpl implements SslDomain, ProtonSslEngineProvider, ProtonJSslDomain
 {
     private Mode _mode;
-    private VerifyMode _verifyMode = VerifyMode.ANONYMOUS_PEER;
+    private VerifyMode _verifyMode;
     private String _certificateFile;
     private String _privateKeyFile;
     private String _privateKeyPassword;
@@ -94,17 +93,18 @@ public SSLContext getSslContext()
     @Override
     public void setPeerAuthentication(VerifyMode verifyMode)
     {
-        if(verifyMode == VerifyMode.VERIFY_PEER_NAME)
-        {
-            throw new ProtonUnsupportedOperationException();
-        }
         _verifyMode = verifyMode;
         _sslEngineFacadeFactory.resetCache();
     }
 
     @Override
     public VerifyMode getPeerAuthentication()
     {
+        if(_verifyMode == null)
+        {
+           return _mode == Mode.SERVER ? VerifyMode.ANONYMOUS_PEER : VerifyMode.VERIFY_PEER_NAME;
+        }
+
         return _verifyMode;
     }
 

proton-j/src/main/java/org/apache/qpid/proton/engine/impl/ssl/SslEngineFacadeFactory.java

@@ -53,6 +53,7 @@
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLParameters;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509TrustManager;
@@ -232,6 +233,13 @@ private SSLEngine createAndInitialiseSslEngine(SslDomain domain, SslPeerDetails
             {
                 sslEngine.setNeedClientAuth(true);
             }
+
+            if(domain.getPeerAuthentication() == SslDomain.VerifyMode.VERIFY_PEER_NAME)
+            {
+                SSLParameters sslParameters = sslEngine.getSSLParameters();
+                sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
+                sslEngine.setSSLParameters(sslParameters);
+            }
         }
 
         if(_logger.isLoggable(Level.FINE))

proton-j/src/main/java/org/apache/qpid/proton/engine/impl/ssl/SslImpl.java

@@ -25,6 +25,7 @@
 import org.apache.qpid.proton.ProtonUnsupportedOperationException;
 import org.apache.qpid.proton.engine.Ssl;
 import org.apache.qpid.proton.engine.SslDomain;
+import org.apache.qpid.proton.engine.SslDomain.VerifyMode;
 import org.apache.qpid.proton.engine.SslPeerDetails;
 import org.apache.qpid.proton.engine.Transport;
 import org.apache.qpid.proton.engine.TransportException;
@@ -54,6 +55,14 @@ public SslImpl(SslDomain domain, SslPeerDetails peerDetails)
         _domain = domain;
         _protonSslEngineProvider = (ProtonSslEngineProvider)domain;
         _peerDetails = peerDetails;
+
+        if(_domain.getMode() == null) {
+            throw new IllegalStateException("Client/server mode must be configured, SslDomain must have init called.");
+        }
+
+        if(_peerDetails == null && _domain.getPeerAuthentication() == VerifyMode.VERIFY_PEER_NAME) {
+            throw new IllegalArgumentException("Peer hostname verification is enabled, but no peer details were provided");
+        }
     }
 
     public TransportWrapper wrap(TransportInput inputProcessor, TransportOutput outputProcessor)

proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java

@@ -37,6 +37,7 @@
 import org.apache.qpid.proton.engine.Event;
 import org.apache.qpid.proton.engine.Sasl;
 import org.apache.qpid.proton.engine.Transport;
+import org.apache.qpid.proton.engine.TransportException;
 import org.apache.qpid.proton.engine.impl.TransportImpl;
 import org.apache.qpid.proton.engine.Record;
 import org.apache.qpid.proton.reactor.Reactor;
@@ -232,7 +233,7 @@ public void run(Selectable selectable) {
                     } else {
                         transport.process();
                     }
-                } catch (IOException e) {
+                } catch (IOException | TransportException e) {
                     ErrorCondition condition = new ErrorCondition();
                     condition.setCondition(Symbol.getSymbol("proton:io"));
                     condition.setDescription(e.getMessage());

proton-j/src/test/java/org/apache/qpid/proton/engine/impl/ssl/SimpleSslTransportWrapperTest.java

@@ -347,12 +347,19 @@ public void testUnderlyingInputHasZeroCapacityMidProcessing()
     @Test
     public void testSslUnwrapThrowsException_returnsErrorResultAndRefusesFurtherInput() throws Exception
     {
-        SSLException sslException = new SSLException("unwrap exception");
+        String unwrapExceptionMessage = "unwrap exception message";
+        SSLException sslException = new SSLException(unwrapExceptionMessage);
         _dummySslEngine.rejectNextEncodedPacket(sslException);
 
         _sslWrapper.tail().put("<-A->".getBytes(StandardCharsets.UTF_8));
-        _sslWrapper.process();
-        assertEquals(_sslWrapper.capacity(), Transport.END_OF_STREAM);
+        try {
+            _sslWrapper.process();
+            fail("no exception");
+        } catch(TransportException e) {
+            assertEquals("javax.net.ssl.SSLException: " + unwrapExceptionMessage, e.getMessage());
+        }
+
+        assertEquals(Transport.END_OF_STREAM, _sslWrapper.capacity());
     }
 
     @Test

proton-j/src/test/java/org/apache/qpid/proton/engine/impl/ssl/SslDomainImplTest.java

@@ -0,0 +1,73 @@
+/*
+ * 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.qpid.proton.engine.impl.ssl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+
+import org.apache.qpid.proton.engine.SslDomain;
+import org.apache.qpid.proton.engine.SslDomain.Mode;
+import org.apache.qpid.proton.engine.SslDomain.VerifyMode;
+import org.junit.Test;
+
+public class SslDomainImplTest {
+
+    @Test
+    public void testInitGetMode() throws Exception
+    {
+        SslDomain sslDomain = SslDomain.Factory.create();
+        assertNull("Unexpected mode, none was set", sslDomain.getMode());
+
+        sslDomain.init(Mode.CLIENT);
+        assertEquals("Unexpected mode", Mode.CLIENT, sslDomain.getMode());
+
+        sslDomain.init(Mode.SERVER);
+        assertEquals("Unexpected mode", Mode.SERVER, sslDomain.getMode());
+    }
+
+    @Test
+    public void testVerifyModeDefault() throws Exception
+    {
+        SslDomain clientSslDomain = SslDomain.Factory.create();
+        assertEquals("Unexpected default verification mode", VerifyMode.VERIFY_PEER_NAME, clientSslDomain.getPeerAuthentication());
+        clientSslDomain.init(Mode.CLIENT);
+        assertEquals("Unexpected default verification mode", VerifyMode.VERIFY_PEER_NAME, clientSslDomain.getPeerAuthentication());
+
+        SslDomain serverSslDomain = SslDomain.Factory.create();
+        serverSslDomain.init(Mode.SERVER);
+        assertEquals("Unexpected default verification mode", VerifyMode.ANONYMOUS_PEER, serverSslDomain.getPeerAuthentication());
+    }
+
+    @Test
+    public void testVerifyModeSetGet() throws Exception
+    {
+        SslDomain clientSslDomain = SslDomain.Factory.create();
+        clientSslDomain.init(Mode.CLIENT);
+        assertNotEquals("Unexpected verification mode", VerifyMode.VERIFY_PEER, clientSslDomain.getPeerAuthentication());
+        clientSslDomain.setPeerAuthentication(VerifyMode.VERIFY_PEER);
+        assertEquals("Unexpected verification mode", VerifyMode.VERIFY_PEER, clientSslDomain.getPeerAuthentication());
+
+        SslDomain serverSslDomain = SslDomain.Factory.create();
+        serverSslDomain.init(Mode.SERVER);
+        assertNotEquals("Unexpected verification mode", VerifyMode.VERIFY_PEER, serverSslDomain.getPeerAuthentication());
+        serverSslDomain.setPeerAuthentication(VerifyMode.VERIFY_PEER);
+        assertEquals("Unexpected verification mode", VerifyMode.VERIFY_PEER, serverSslDomain.getPeerAuthentication());
+    }
+}