Move PkiRealm checks for SSL and client authentication to a bootstrap check (elastic/x-pack-elasticsearch#1442)

This commit cleans up the check for SSL with client authentication when a PKI realm is enabled by
moving it from the realm to a actual bootstrap check.

A bug was found during this cleanup in the check for transport profiles and that is also fixed in
this commit.

relates elastic/x-pack-elasticsearch#420

Original commit: elastic/x-pack-elasticsearch@3aa6a3edc0

plugin/src/main/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheck.java

@@ -0,0 +1,77 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.security;
+
+import org.elasticsearch.bootstrap.BootstrapCheck;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.xpack.security.authc.RealmSettings;
+import org.elasticsearch.xpack.security.authc.pki.PkiRealm;
+import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4Transport;
+import org.elasticsearch.xpack.ssl.SSLService;
+
+import java.util.Map;
+
+import static org.elasticsearch.xpack.XPackSettings.HTTP_SSL_ENABLED;
+import static org.elasticsearch.xpack.security.Security.setting;
+
+class PkiRealmBootstrapCheck implements BootstrapCheck {
+
+    private final SSLService sslService;
+    private final Settings settings;
+
+    PkiRealmBootstrapCheck(Settings settings, SSLService sslService) {
+        this.settings = settings;
+        this.sslService = sslService;
+    }
+
+    /**
+     * If a PKI realm is enabled, checks to see if SSL and Client authentication are enabled on at
+     * least one network communication layer.
+     */
+    @Override
+    public boolean check() {
+        final boolean pkiRealmEnabled = settings.getGroups(RealmSettings.PREFIX).values().stream()
+                .filter(s -> PkiRealm.TYPE.equals(s.get("type")))
+                .anyMatch(s -> s.getAsBoolean("enabled", true));
+        if (pkiRealmEnabled) {
+            // HTTP
+            final boolean httpSsl = HTTP_SSL_ENABLED.get(settings);
+            Settings httpSSLSettings = SSLService.getHttpTransportSSLSettings(settings);
+            final boolean httpClientAuth = sslService.isSSLClientAuthEnabled(httpSSLSettings);
+            if (httpSsl && httpClientAuth) {
+                return false;
+            }
+
+            // Default Transport
+            final Settings transportSSLSettings = settings.getByPrefix(setting("transport.ssl."));
+            final boolean clientAuthEnabled = sslService.isSSLClientAuthEnabled(transportSSLSettings);
+            if (clientAuthEnabled) {
+                return false;
+            }
+
+            // Transport Profiles
+            Map<String, Settings> groupedSettings = settings.getGroups("transport.profiles.");
+            for (Map.Entry<String, Settings> entry : groupedSettings.entrySet()) {
+                if (sslService.isSSLClientAuthEnabled(SecurityNetty4Transport.profileSslSettings(entry.getValue()), transportSSLSettings)) {
+                    return false;
+                }
+            }
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public String errorMessage() {
+        return "A PKI realm is enabled but cannot be used as neither HTTP or Transport have SSL and client authentication enabled";
+    }
+
+    @Override
+    public boolean alwaysEnforce() {
+        return true;
+    }
+}

plugin/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -503,7 +503,8 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin {
                 new DefaultPasswordBootstrapCheck(settings),
                 new SSLBootstrapCheck(sslService, settings, env),
                 new TokenPassphraseBootstrapCheck(settings),
-                new TokenSSLBootstrapCheck(settings)
+                new TokenSSLBootstrapCheck(settings),
+                new PkiRealmBootstrapCheck(settings, sslService)
             );
         } else {
             return Collections.emptyList();

plugin/src/main/java/org/elasticsearch/xpack/security/authc/InternalRealms.java

@@ -69,8 +69,7 @@ public class InternalRealms {
                 resourceWatcherService, nativeRoleMappingStore, threadPool));
         map.put(LdapRealm.LDAP_TYPE, config -> new LdapRealm(LdapRealm.LDAP_TYPE, config,
                 sslService, resourceWatcherService, nativeRoleMappingStore, threadPool));
-        map.put(PkiRealm.TYPE, config -> new PkiRealm(config, sslService, resourceWatcherService,
+        map.put(PkiRealm.TYPE, config -> new PkiRealm(config, resourceWatcherService, nativeRoleMappingStore));
-                nativeRoleMappingStore));
         return Collections.unmodifiableMap(map);
     }
 

plugin/src/main/java/org/elasticsearch/xpack/security/authc/pki/PkiRealm.java

@@ -16,14 +16,11 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.security.Security;
 import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
 import org.elasticsearch.xpack.security.authc.support.mapper.CompositeRoleMapper;
 import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore;
-import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4Transport;
 import org.elasticsearch.xpack.ssl.CertUtils;
 import org.elasticsearch.xpack.ssl.SSLConfigurationSettings;
-import org.elasticsearch.xpack.ssl.SSLService;
 import org.elasticsearch.xpack.security.user.User;
 import org.elasticsearch.xpack.security.authc.AuthenticationToken;
 import org.elasticsearch.xpack.security.authc.Realm;
@@ -56,27 +53,23 @@ public class PkiRealm extends Realm {
     private static final SSLConfigurationSettings SSL_SETTINGS = SSLConfigurationSettings.withoutPrefix();
 
     // For client based cert validation, the auth type must be specified but UNKNOWN is an acceptable value
-    public static final String AUTH_TYPE = "UNKNOWN";
+    private static final String AUTH_TYPE = "UNKNOWN";
 
     private final X509TrustManager trustManager;
     private final Pattern principalPattern;
     private final UserRoleMapper roleMapper;
 
 
-    public PkiRealm(RealmConfig config, SSLService sslService,
+    public PkiRealm(RealmConfig config, ResourceWatcherService watcherService, NativeRoleMappingStore nativeRoleMappingStore) {
-                    ResourceWatcherService watcherService,
+        this(config, new CompositeRoleMapper(TYPE, config, watcherService, nativeRoleMappingStore));
-                    NativeRoleMappingStore nativeRoleMappingStore) {
-        this(config, new CompositeRoleMapper(TYPE, config, watcherService, nativeRoleMappingStore),
-                sslService);
     }
 
     // pkg private for testing
-    PkiRealm(RealmConfig config, UserRoleMapper roleMapper, SSLService sslService) {
+    PkiRealm(RealmConfig config, UserRoleMapper roleMapper) {
         super(TYPE, config);
         this.trustManager = trustManagers(config);
         this.principalPattern = USERNAME_PATTERN_SETTING.get(config.settings());
         this.roleMapper = roleMapper;
-        checkSSLEnabled(config, sslService);
     }
 
     @Override
@@ -202,45 +195,6 @@ public class PkiRealm extends Realm {
         }
     }
 
-    /**
-     * Checks to see if both SSL and Client authentication are enabled on at least one network communication layer. If
-     * not an exception will be thrown
-     *
-     * @param config this realm's configuration
-     * @param sslService the SSLService to use for ssl configurations
-     */
-    // TODO move this to a Bootstrap check!
-    static void checkSSLEnabled(RealmConfig config, SSLService sslService) {
-        Settings settings = config.globalSettings();
-
-        // HTTP
-        final boolean httpSsl = HTTP_SSL_ENABLED.get(settings);
-        Settings httpSSLSettings = SSLService.getHttpTransportSSLSettings(settings);
-        final boolean httpClientAuth = sslService.isSSLClientAuthEnabled(httpSSLSettings);
-        if (httpSsl && httpClientAuth) {
-            return;
-        }
-
-        // Default Transport
-        final Settings transportSSLSettings = settings.getByPrefix(setting("transport.ssl."));
-        final boolean clientAuthEnabled = sslService.isSSLClientAuthEnabled(transportSSLSettings);
-        if (clientAuthEnabled) {
-            return;
-        }
-
-        // Transport Profiles
-        Map<String, Settings> groupedSettings = settings.getGroups("transport.profiles.");
-        for (Map.Entry<String, Settings> entry : groupedSettings.entrySet()) {
-            Settings profileSettings = entry.getValue().getByPrefix(Security.settingPrefix());
-            if (sslService.isSSLClientAuthEnabled(SecurityNetty4Transport.profileSslSettings(profileSettings), transportSSLSettings)) {
-                return;
-            }
-        }
-
-        throw new IllegalStateException("PKI realm [" + config.name() + "] is enabled but cannot be used as neither HTTP or Transport " +
-                "has SSL with client authentication enabled");
-    }
-
     /**
      * @return The {@link Setting setting configuration} for this realm type
      */

plugin/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.security;
+
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.security.authc.pki.PkiRealm;
+import org.elasticsearch.xpack.ssl.SSLService;
+
+public class PkiRealmBootstrapCheckTests extends ESTestCase {
+
+    public void testPkiRealmBootstrapDefault() throws Exception {
+        assertFalse(new PkiRealmBootstrapCheck(Settings.EMPTY, new SSLService(Settings.EMPTY,
+                new Environment(Settings.builder().put("path.home", createTempDir()).build()))).check());
+    }
+
+    public void testBootstrapCheckWithPkiRealm() throws Exception {
+        Settings settings = Settings.builder()
+                .put("xpack.security.authc.realms.test_pki.type", PkiRealm.TYPE)
+                .put("path.home", createTempDir())
+                .build();
+        Environment env = new Environment(settings);
+        assertFalse(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+
+        // disable client auth default
+        settings = Settings.builder().put(settings)
+                .put("xpack.ssl.client_authentication", "none")
+                .build();
+        env = new Environment(settings);
+        assertTrue(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+
+        // enable ssl for http
+        settings = Settings.builder().put(settings)
+                .put("xpack.security.http.ssl.enabled", true)
+                .build();
+        env = new Environment(settings);
+        assertTrue(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+
+        // enable client auth for http
+        settings = Settings.builder().put(settings)
+                .put("xpack.security.http.ssl.client_authentication", randomFrom("required", "optional"))
+                .build();
+        env = new Environment(settings);
+        assertFalse(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+
+        // disable http ssl
+        settings = Settings.builder().put(settings)
+                .put("xpack.security.http.ssl.enabled", false)
+                .build();
+        env = new Environment(settings);
+        assertTrue(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+
+        // set transport client auth
+        settings = Settings.builder().put(settings)
+                .put("xpack.security.transport.client_authentication", randomFrom("required", "optional"))
+                .build();
+        env = new Environment(settings);
+        assertTrue(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+
+        // test with transport profile
+        settings = Settings.builder().put(settings)
+                .put("xpack.security.transport.client_authentication", "none")
+                .put("transport.profiles.foo.xpack.security.ssl.client_authentication", randomFrom("required", "optional"))
+                .build();
+        env = new Environment(settings);
+        assertFalse(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+    }
+
+    public void testBootstrapCheckWithDisabledRealm() throws Exception {
+        Settings settings = Settings.builder()
+                .put("xpack.security.authc.realms.test_pki.type", PkiRealm.TYPE)
+                .put("xpack.security.authc.realms.test_pki.enabled", false)
+                .put("xpack.ssl.client_authentication", "none")
+                .put("path.home", createTempDir())
+                .build();
+        Environment env = new Environment(settings);
+        assertFalse(new PkiRealmBootstrapCheck(settings, new SSLService(settings, env)).check());
+    }
+}

plugin/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java

@@ -28,13 +28,10 @@ import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
 import org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken;
 import org.elasticsearch.xpack.security.support.NoOpLogger;
 import org.elasticsearch.xpack.security.user.User;
-import org.elasticsearch.xpack.ssl.SSLClientAuth;
-import org.elasticsearch.xpack.ssl.SSLService;
 import org.junit.Before;
 import org.mockito.Mockito;
 
 import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
-import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -46,22 +43,17 @@ import static org.mockito.Mockito.when;
 public class PkiRealmTests extends ESTestCase {
 
     private Settings globalSettings;
-    private SSLService sslService;
 
     @Before
     public void setup() throws Exception {
-        Path testnodeStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks");
         globalSettings = Settings.builder()
                 .put("path.home", createTempDir())
-                .put("xpack.ssl.keystore.path", testnodeStore)
-                .put("xpack.ssl.keystore.password", "testnode")
                 .build();
-        sslService = new SSLService(globalSettings, new Environment(globalSettings));
     }
 
     public void testTokenSupport() {
         RealmConfig config = new RealmConfig("", Settings.EMPTY, globalSettings, new Environment(globalSettings), new ThreadContext(globalSettings));
-        PkiRealm realm = new PkiRealm(config, mock(UserRoleMapper.class), sslService);
+        PkiRealm realm = new PkiRealm(config, mock(UserRoleMapper.class));
 
         assertThat(realm.supports(null), is(false));
         assertThat(realm.supports(new UsernamePasswordToken("", new SecureString(new char[0]))), is(false));
@@ -73,7 +65,7 @@ public class PkiRealmTests extends ESTestCase {
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
         threadContext.putTransient(PkiRealm.PKI_CERT_HEADER_NAME, new X509Certificate[] { certificate });
         PkiRealm realm = new PkiRealm(new RealmConfig("", Settings.EMPTY, globalSettings, new Environment(globalSettings), 
-                new ThreadContext(globalSettings)), mock(UserRoleMapper.class), sslService);
+                new ThreadContext(globalSettings)), mock(UserRoleMapper.class));
 
         X509AuthenticationToken token = realm.token(threadContext);
         assertThat(token, is(notNullValue()));
@@ -98,7 +90,7 @@ public class PkiRealmTests extends ESTestCase {
         X509AuthenticationToken token = new X509AuthenticationToken(new X509Certificate[] { certificate }, "Elasticsearch Test Node", dn);
         UserRoleMapper roleMapper = mock(UserRoleMapper.class);
         PkiRealm realm = new PkiRealm(new RealmConfig("", Settings.EMPTY, globalSettings, new Environment(globalSettings),
-            new ThreadContext(globalSettings)), roleMapper, sslService);
+            new ThreadContext(globalSettings)), roleMapper);
         Mockito.doAnswer(invocation -> {
             final UserRoleMapper.UserData userData = (UserRoleMapper.UserData) invocation.getArguments()[0];
             final ActionListener<Set<String>> listener = (ActionListener<Set<String>>) invocation.getArguments()[1];
@@ -124,7 +116,7 @@ public class PkiRealmTests extends ESTestCase {
         X509Certificate certificate = readCert(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"));
         UserRoleMapper roleMapper = mock(UserRoleMapper.class);
         PkiRealm realm = new PkiRealm(new RealmConfig("", Settings.builder().put("username_pattern", "OU=(.*?),").build(), globalSettings, new Environment(globalSettings), new ThreadContext(globalSettings)),
-                roleMapper, sslService);
+                roleMapper);
         Mockito.doAnswer(invocation -> {
             ActionListener<Set<String>> listener = (ActionListener<Set<String>>) invocation.getArguments()[1];
             listener.onResponse(Collections.emptySet());
@@ -152,7 +144,7 @@ public class PkiRealmTests extends ESTestCase {
                 .put("truststore.password", "testnode")
                 .build();
         PkiRealm realm = new PkiRealm(new RealmConfig("", settings, globalSettings, new Environment(globalSettings),
-                new ThreadContext(globalSettings)), roleMapper, sslService);
+                new ThreadContext(globalSettings)), roleMapper);
         Mockito.doAnswer(invocation -> {
             ActionListener<Set<String>> listener = (ActionListener<Set<String>>) invocation.getArguments()[1];
             listener.onResponse(Collections.emptySet());
@@ -181,7 +173,7 @@ public class PkiRealmTests extends ESTestCase {
                 .put("truststore.password", "testnode-client-profile")
                 .build();
         PkiRealm realm = new PkiRealm(new RealmConfig("", settings, globalSettings, new Environment(globalSettings),
-                new ThreadContext(globalSettings)), roleMapper, sslService);
+                new ThreadContext(globalSettings)), roleMapper);
         Mockito.doAnswer(invocation -> {
             ActionListener<Set<String>> listener = (ActionListener<Set<String>>) invocation.getArguments()[1];
             listener.onResponse(Collections.emptySet());
@@ -205,7 +197,7 @@ public class PkiRealmTests extends ESTestCase {
                 .build();
         try {
             new PkiRealm(new RealmConfig("mypki", settings, globalSettings, new Environment(globalSettings),
-                    new ThreadContext(globalSettings)), mock(UserRoleMapper.class), sslService);
+                    new ThreadContext(globalSettings)), mock(UserRoleMapper.class));
             fail("exception should have been thrown");
         } catch (IllegalArgumentException e) {
             assertThat(e.getMessage(), containsString("[xpack.security.authc.realms.mypki.truststore.password] is not configured"));
@@ -248,29 +240,6 @@ public class PkiRealmTests extends ESTestCase {
         assertThat(token.dn(), is("EMAILADDRESS=pki@elastic.co, CN=PKI Client, OU=Security"));
     }
 
-    public void testNoClientAuthThrowsException() throws Exception {
-        Settings settings = Settings.builder()
-                .put(globalSettings)
-                .put("xpack.ssl.client_authentication", "none")
-                .build();
-
-        IllegalStateException e = expectThrows(IllegalStateException.class,
-                    () -> new PkiRealm(new RealmConfig("", Settings.EMPTY, settings, new Environment(settings), new ThreadContext(settings)),
-                        mock(UserRoleMapper.class), new SSLService(settings, new Environment(settings))));
-        assertThat(e.getMessage(), containsString("has SSL with client authentication enabled"));
-    }
-
-    public void testHttpClientAuthOnly() throws Exception {
-        Settings settings = Settings.builder()
-                .put(globalSettings)
-                .put("xpack.ssl.client_authentication", "none")
-                .put("xpack.security.http.ssl.enabled", true)
-                .put("xpack.security.http.ssl.client_authentication", randomFrom(SSLClientAuth.OPTIONAL, SSLClientAuth.REQUIRED))
-                .build();
-        new PkiRealm(new RealmConfig("", Settings.EMPTY, settings, new Environment(settings), new ThreadContext(settings)),
-                mock(UserRoleMapper.class), new SSLService(settings, new Environment(settings)));
-    }
-
     static X509Certificate readCert(Path path) throws Exception {
         try (InputStream in = Files.newInputStream(path)) {
             CertificateFactory factory = CertificateFactory.getInstance("X.509");