Mercurial > hg > release > thermostat-0.9
changeset 1093:c2982ec31c62
Make SSL misconfiguration fatal
After having mistyped the password for my keystore used for Thermostat,
I noticed that these kinds of misconfigurations cause the agent to hang.
This is because after failing to create a TrustManager using information
from the ssl.properties file, we still attempt to connect. This commit
propagates the errors to the connection class, so that we can show the
user what is wrong with their configuration (wrong password, keystore
doesn't exist), and then exit.
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-May/006580.html
line wrap: on
line diff
--- a/common/core/src/main/java/com/redhat/thermostat/common/internal/CustomX509TrustManager.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/internal/CustomX509TrustManager.java Fri May 17 14:06:04 2013 -0400 @@ -54,6 +54,7 @@ import javax.net.ssl.X509TrustManager; import com.redhat.thermostat.common.ssl.SSLConfiguration; +import com.redhat.thermostat.common.ssl.SslInitException; import com.redhat.thermostat.common.utils.LoggingUtils; /** @@ -84,13 +85,13 @@ // For testing CustomX509TrustManager(X509TrustManager defaultTrustManager, - File keyStoreFile, String keyStorePassword) { + File keyStoreFile, String keyStorePassword) throws SslInitException { this.defaultX509TrustManager = defaultTrustManager; this.ourX509TrustManager = getOurTrustManager(keyStoreFile, keyStorePassword); } // For testing - CustomX509TrustManager(File keyStoreFile, String keyStorePassword) { + CustomX509TrustManager(File keyStoreFile, String keyStorePassword) throws SslInitException { this.defaultX509TrustManager = getDefaultTrustManager(); this.ourX509TrustManager = getOurTrustManager(keyStoreFile, keyStorePassword); } @@ -98,11 +99,11 @@ /* * Main constructor, which uses ssl.properties as config if present. */ - CustomX509TrustManager() { + CustomX509TrustManager() throws SslInitException { this(SSLConfiguration.getKeystoreFile(), SSLConfiguration.getKeyStorePassword()); } - private X509TrustManager getDefaultTrustManager() { + private X509TrustManager getDefaultTrustManager() throws SslInitException { try { TrustManagerFactory tmf = TrustManagerFactory.getInstance( "SunX509", "SunJSSE"); @@ -125,7 +126,7 @@ } } } catch (NoSuchAlgorithmException | NoSuchProviderException | KeyStoreException e) { - logger.log(Level.WARNING, "Could not retrieve system trust manager", e); + throw new SslInitException("Could not retrieve system trust manager", e); } return null; } @@ -137,7 +138,7 @@ * exist or no X509TrustManager was found in the backing trust store. */ private X509TrustManager getOurTrustManager(File trustStoreFile, - String keyStorePassword) { + String keyStorePassword) throws SslInitException { KeyStore trustStore = KeyStoreProvider.getKeyStore(trustStoreFile, keyStorePassword); if (trustStore != null) { // backing keystore file existed and initialization was successful. @@ -155,9 +156,7 @@ } } catch (NoSuchAlgorithmException | NoSuchProviderException | KeyStoreException e) { - logger.log(Level.WARNING, - "Could not load Thermostat trust manager"); - return null; + throw new SslInitException("Could not load Thermostat trust manager", e); } } logger.log(Level.FINE, "No Thermostat trust manager found");
--- a/common/core/src/main/java/com/redhat/thermostat/common/internal/KeyStoreProvider.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/internal/KeyStoreProvider.java Fri May 17 14:06:04 2013 -0400 @@ -44,18 +44,14 @@ import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; -import java.util.logging.Level; -import java.util.logging.Logger; -import com.redhat.thermostat.common.utils.LoggingUtils; +import com.redhat.thermostat.common.ssl.SslInitException; public class KeyStoreProvider { - private static final Logger logger = LoggingUtils - .getLogger(KeyStoreProvider.class); - - public static KeyStore getKeyStore(File trustStoreFile, String keyStorePassword) { - if (trustStoreFile != null && trustStoreFile.exists()) { + public static KeyStore getKeyStore(File trustStoreFile, String keyStorePassword) + throws SslInitException { + if (trustStoreFile != null) { try (InputStream is = new FileInputStream(trustStoreFile)) { KeyStore trustStore = KeyStore.getInstance(KeyStore .getDefaultType()); @@ -63,9 +59,7 @@ return trustStore; } catch (IOException | CertificateException | NoSuchAlgorithmException | KeyStoreException e) { - logger.log(Level.WARNING, - "Could not load Thermostat trust manager", e); - return null; + throw new SslInitException("Could not load Thermostat trust manager", e); } } return null;
--- a/common/core/src/main/java/com/redhat/thermostat/common/internal/TrustManagerFactory.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/internal/TrustManagerFactory.java Fri May 17 14:06:04 2013 -0400 @@ -38,6 +38,8 @@ import javax.net.ssl.X509TrustManager; +import com.redhat.thermostat.common.ssl.SslInitException; + /** * Factory class in order to be able to get at the thermostat X509TrustManager. @@ -47,8 +49,9 @@ /** * @return the thermostat X509TrustManager. + * @throws SslInitException if an error occured retrieving the trust manager. */ - public static X509TrustManager getTrustManager() { + public static X509TrustManager getTrustManager() throws SslInitException { return new CustomX509TrustManager(); } }
--- a/common/core/src/main/java/com/redhat/thermostat/common/ssl/SSLContextFactory.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/ssl/SSLContextFactory.java Fri May 17 14:06:04 2013 -0400 @@ -169,7 +169,7 @@ serverContext = serverCtxt; } - private static TrustManager[] getTrustManagers() { + private static TrustManager[] getTrustManagers() throws SslInitException { TrustManager tm = TrustManagerFactory.getTrustManager(); return new TrustManager[] { tm }; }
--- a/common/core/src/main/java/com/redhat/thermostat/common/ssl/SslInitException.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/ssl/SslInitException.java Fri May 17 14:06:04 2013 -0400 @@ -45,5 +45,9 @@ public SslInitException(String message) { super(message); } + + public SslInitException(String message, Throwable cause) { + super(message, cause); + } }
--- a/common/core/src/test/java/com/redhat/thermostat/common/internal/CustomX509TrustManagerTest.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/internal/CustomX509TrustManagerTest.java Fri May 17 14:06:04 2013 -0400 @@ -50,6 +50,7 @@ import org.junit.Test; import com.redhat.thermostat.common.internal.CustomX509TrustManager; +import com.redhat.thermostat.common.ssl.SslInitException; /** * This trust manager test uses files in src/test/resources. Files are as @@ -88,7 +89,7 @@ } @Test - public void testLoadEmptyTrustStoreForOur() { + public void testLoadEmptyTrustStoreForOur() throws SslInitException { File emptyKeyStore = new File(this.getClass() .getResource("/empty.keystore").getFile()); X509TrustManager tm = new CustomX509TrustManager(null, emptyKeyStore, @@ -123,7 +124,7 @@ } @Test - public void canGetCustomCaCertFromOurTrustManager() { + public void canGetCustomCaCertFromOurTrustManager() throws SslInitException { File ourKeyStore = new File(this.getClass() .getResource("/test_ca.keystore").getFile()); X509TrustManager tm = new CustomX509TrustManager((X509TrustManager)null, ourKeyStore, "testpassword");
--- a/common/core/src/test/java/com/redhat/thermostat/common/internal/KeyStoreProviderTest.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/internal/KeyStoreProviderTest.java Fri May 17 14:06:04 2013 -0400 @@ -43,28 +43,30 @@ import org.junit.Test; +import com.redhat.thermostat.common.ssl.SslInitException; + public class KeyStoreProviderTest { - @Test - public void nonExistentFileReturnsNull() { + @Test(expected=SslInitException.class) + public void nonExistentFileThrowsException() throws SslInitException { File notThere = new File("/some/path/that/doesnt/exists"); assertNull(KeyStoreProvider.getKeyStore(notThere, "ignored")); } @Test - public void nullFileReturnsNull() { + public void nullFileReturnsNull() throws SslInitException { assertNull(KeyStoreProvider.getKeyStore(null, "ignored")); } + @Test(expected=SslInitException.class) + public void existingFileWithWrongPwdThrowsException() throws SslInitException { + File keystore = new File(this.getClass() + .getResource("/test_ca.keystore").getFile()); + KeyStoreProvider.getKeyStore(keystore, "wrong password"); + } + @Test - public void existingFileWithWrongPwdReturnsNull() { - File keystore = new File(this.getClass() - .getResource("/test_ca.keystore").getFile()); - assertNull(KeyStoreProvider.getKeyStore(keystore, "wrong password")); - } - - @Test - public void existingFileWithCorrectPasswordWorks() { + public void existingFileWithCorrectPasswordWorks() throws SslInitException { File keystore = new File(this.getClass() .getResource("/test_ca.keystore").getFile()); assertNotNull("Should have been able to retrieve and load keystore", KeyStoreProvider.getKeyStore(keystore, "testpassword"));
--- a/common/core/src/test/java/com/redhat/thermostat/common/ssl/SSLContextFactoryTest.java Thu May 16 16:39:00 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/ssl/SSLContextFactoryTest.java Fri May 17 14:06:04 2013 -0400 @@ -63,9 +63,8 @@ import com.redhat.thermostat.common.internal.TrustManagerFactory; - @RunWith(PowerMockRunner.class) -@PrepareForTest({ SSLConfiguration.class, SSLContext.class, KeyManagerFactory.class }) +@PrepareForTest({ SSLConfiguration.class, SSLContext.class, KeyManagerFactory.class, javax.net.ssl.TrustManagerFactory.class }) public class SSLContextFactoryTest { /* @@ -102,6 +101,11 @@ when(KeyManagerFactory.getInstance("SunX509", "SunJSSE")).thenReturn(mockFactory); KeyManager[] mockKms = new KeyManager[] { mock(X509KeyManager.class) }; when(mockFactory.getKeyManagers()).thenReturn(mockKms); + PowerMockito.mockStatic(javax.net.ssl.TrustManagerFactory.class); + javax.net.ssl.TrustManagerFactory mockTrustFactory = PowerMockito.mock(javax.net.ssl.TrustManagerFactory.class); + when(mockTrustFactory.getTrustManagers()).thenReturn(new TrustManager[0]); + when(javax.net.ssl.TrustManagerFactory.getInstance("SunX509", "SunJSSE")).thenReturn(mockTrustFactory); + SSLContextFactory.getServerContext(); verify(context).init(keymanagersCaptor.capture(), tmsCaptor.capture(), any(SecureRandom.class)); @@ -133,6 +137,10 @@ PowerMockito.mockStatic(SSLContext.class); SSLContext context = PowerMockito.mock(SSLContext.class); when(SSLContext.getInstance("TLSv1.2", "SunJSSE")).thenReturn(context); + PowerMockito.mockStatic(javax.net.ssl.TrustManagerFactory.class); + javax.net.ssl.TrustManagerFactory mockTrustFactory = PowerMockito.mock(javax.net.ssl.TrustManagerFactory.class); + when(mockTrustFactory.getTrustManagers()).thenReturn(new TrustManager[0]); + when(javax.net.ssl.TrustManagerFactory.getInstance("SunX509", "SunJSSE")).thenReturn(mockTrustFactory); ArgumentCaptor<TrustManager[]> tmsCaptor = ArgumentCaptor .forClass(TrustManager[].class);