Explorar el Código

Fix failing CI due to warning in Secure Settings Validation (#103307)

* Fix failing CI due to warning in Secure Settings Validation

* Validate settings in ReloadSecureSettings API (#103176)
Johannes Fredén hace 1 año
padre
commit
6a4000cec6

+ 5 - 0
docs/changelog/103176.yaml

@@ -0,0 +1,5 @@
+pr: 103176
+summary: Validate settings in `ReloadSecureSettings` API
+area: Client
+type: bug
+issues: []

+ 49 - 1
server/src/internalClusterTest/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java

@@ -11,10 +11,13 @@ package org.elasticsearch.action.admin;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse;
+import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.KeyStoreWrapper;
+import org.elasticsearch.common.settings.SecureSetting;
 import org.elasticsearch.common.settings.SecureSettings;
 import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.plugins.Plugin;
@@ -43,6 +46,8 @@ import static org.hamcrest.Matchers.nullValue;
 @ESIntegTestCase.ClusterScope(minNumDataNodes = 2)
 public class ReloadSecureSettingsIT extends ESIntegTestCase {
 
+    private static final String VALID_SECURE_SETTING_NAME = "some.setting.that.exists";
+
     @BeforeClass
     public static void disableInFips() {
         // Reload secure settings with a password protected keystore is tested in ReloadSecureSettingsWithPasswordProtectedKeystoreRestIT
@@ -350,9 +355,46 @@ public class ReloadSecureSettingsIT extends ESIntegTestCase {
         }
     }
 
+    public void testInvalidKeyInSettings() throws Exception {
+        final Environment environment = internalCluster().getInstance(Environment.class);
+
+        try (KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create()) {
+            keyStoreWrapper.setString(VALID_SECURE_SETTING_NAME, new char[0]);
+            keyStoreWrapper.save(environment.configFile(), new char[0], false);
+        }
+
+        PlainActionFuture<NodesReloadSecureSettingsResponse> actionFuture = new PlainActionFuture<>();
+        clusterAdmin().prepareReloadSecureSettings()
+            .setSecureStorePassword(new SecureString(new char[0]))
+            .setNodesIds(Strings.EMPTY_ARRAY)
+            .execute(actionFuture);
+
+        actionFuture.get().getNodes().forEach(nodeResponse -> assertThat(nodeResponse.reloadException(), nullValue()));
+
+        try (KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create()) {
+            assertThat(keyStoreWrapper, notNullValue());
+            keyStoreWrapper.setString("some.setting.that.does.not.exist", new char[0]);
+            keyStoreWrapper.save(environment.configFile(), new char[0], false);
+        }
+
+        actionFuture = new PlainActionFuture<>();
+        clusterAdmin().prepareReloadSecureSettings()
+            .setSecureStorePassword(new SecureString(new char[0]))
+            .setNodesIds(Strings.EMPTY_ARRAY)
+            .execute(actionFuture);
+
+        actionFuture.get()
+            .getNodes()
+            .forEach(nodeResponse -> assertThat(nodeResponse.reloadException(), instanceOf(IllegalArgumentException.class)));
+    }
+
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        final List<Class<? extends Plugin>> plugins = Arrays.asList(MockReloadablePlugin.class, MisbehavingReloadablePlugin.class);
+        final List<Class<? extends Plugin>> plugins = Arrays.asList(
+            MockWithSecureSettingPlugin.class,
+            MockReloadablePlugin.class,
+            MisbehavingReloadablePlugin.class
+        );
         // shuffle as reload is called in order
         Collections.shuffle(plugins, random());
         return plugins;
@@ -455,4 +497,10 @@ public class ReloadSecureSettingsIT extends ESIntegTestCase {
         }
     }
 
+    public static class MockWithSecureSettingPlugin extends Plugin {
+        public List<Setting<?>> getSettings() {
+            return List.of(SecureSetting.secureString(VALID_SECURE_SETTING_NAME, null));
+        }
+    };
+
 }

+ 2 - 0
server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java

@@ -122,6 +122,8 @@ public class TransportNodesReloadSecureSettingsAction extends TransportNodesActi
             keystore.decrypt(request.hasPassword() ? request.getSecureSettingsPassword().getChars() : new char[0]);
             // add the keystore to the original node settings object
             final Settings settingsWithKeystore = Settings.builder().put(environment.settings(), false).setSecureSettings(keystore).build();
+            clusterService.getClusterSettings().validate(settingsWithKeystore, true);
+
             final List<Exception> exceptions = new ArrayList<>();
             // broadcast the new settings object (with the open embedded keystore) to all reloadable plugins
             pluginsService.filterPlugins(ReloadablePlugin.class).forEach(p -> {

+ 2 - 3
x-pack/plugin/security/qa/jwt-realm/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtRestIT.java

@@ -105,16 +105,15 @@ public class JwtRestIT extends ESRestTestCase {
         .setting("xpack.security.transport.ssl.enabled", "false")
         .setting("xpack.security.authc.token.enabled", "true")
         .setting("xpack.security.authc.api_key.enabled", "true")
-
         .setting("xpack.security.http.ssl.enabled", "true")
         .setting("xpack.security.http.ssl.certificate", "http.crt")
         .setting("xpack.security.http.ssl.key", "http.key")
-        .setting("xpack.security.http.ssl.key_passphrase", "http-password")
         .setting("xpack.security.http.ssl.certificate_authorities", "ca.crt")
         .setting("xpack.security.http.ssl.client_authentication", "optional")
         .settings(JwtRestIT::realmSettings)
         .keystore("xpack.security.authc.realms.jwt.jwt2.hmac_key", HMAC_PASSPHRASE)
         .keystore("xpack.security.authc.realms.jwt.jwt3.hmac_jwkset", HMAC_JWKSET)
+        .keystore("xpack.security.http.ssl.secure_key_passphrase", "http-password")
         .keystore("xpack.security.authc.realms.jwt.jwt3.client_authentication.shared_secret", VALID_SHARED_SECRET)
         .keystore(keystoreSettings)
         .user("admin_user", "admin-password")
@@ -508,8 +507,8 @@ public class JwtRestIT extends ESRestTestCase {
         }
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/103308")
     public void testReloadClientSecret() throws Exception {
+        assumeFalse("Cannot run in FIPS mode because this test tampers with the keystore", inFipsJvm());
         final String principal = SERVICE_SUBJECT.get();
         final String username = getUsernameFromPrincipal(principal);
         final List<String> roles = randomRoles();

+ 1 - 1
x-pack/qa/password-protected-keystore/src/javaRestTest/java/org/elasticsearch/password_protected_keystore/ReloadSecureSettingsWithPasswordProtectedKeystoreRestIT.java

@@ -37,12 +37,12 @@ public class ReloadSecureSettingsWithPasswordProtectedKeystoreRestIT extends ESR
         .nodes(NUM_NODES)
         .keystorePassword(KEYSTORE_PASSWORD)
         .name("javaRestTest")
+        .keystore(nodeSpec -> Map.of("xpack.security.transport.ssl.secure_key_passphrase", "transport-password"))
         .setting("xpack.security.enabled", "true")
         .setting("xpack.security.authc.anonymous.roles", "anonymous")
         .setting("xpack.security.transport.ssl.enabled", "true")
         .setting("xpack.security.transport.ssl.certificate", "transport.crt")
         .setting("xpack.security.transport.ssl.key", "transport.key")
-        .setting("xpack.security.transport.ssl.key_passphrase", "transport-password")
         .setting("xpack.security.transport.ssl.certificate_authorities", "ca.crt")
         .rolesFile(Resource.fromClasspath("roles.yml"))
         .configFile("transport.key", Resource.fromClasspath("ssl/transport.key"))