From 182858b32d35f3f070d4c5a1052e4862a7da550e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 23 Jul 2018 13:13:38 -0700 Subject: [PATCH 1/2] Tribe: Add error with secure settings copied to tribe This commit adds a clear error message when tribe setup attempts to copy a secure setting into tribe settings. This behavior has never worked, but the previous error message was very confusing, complaining about a source key not being found later when trying to read the setting. closes #32117 --- .../org/elasticsearch/xpack/security/Security.java | 10 ++++++++-- .../xpack/security/SecurityTribeTests.java | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 0121b1ba69ccf..a026a9524fcbe 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -34,6 +34,7 @@ import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.IndexScopedSettings; +import org.elasticsearch.common.settings.SecureSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -786,11 +787,16 @@ private static void addTribeSettings(Settings settings, Settings.Builder setting } // we passed all the checks now we need to copy in all of the x-pack security settings - settings.keySet().forEach(k -> { + SecureSettings secureSettings = Settings.builder().put(settings).getSecureSettings(); // hack to get at secure settings... + Set secureSettingKeys = secureSettings == null ? Collections.emptySet() : secureSettings.getSettingNames(); + for (String k : settings.keySet()) { if (k.startsWith("xpack.security.")) { + if (secureSettingKeys.contains(k)) { + throw new IllegalArgumentException("Secure setting [" + k + "] cannot be used with tribe client node"); + } settingsBuilder.copy(tribePrefix + k, k, settings); } - }); + } } Map realmsSettings = settings.getGroups(SecurityField.setting("authc.realms"), true); diff --git a/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java b/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java index edf725e53939a..261416c7b0824 100644 --- a/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java +++ b/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java @@ -556,6 +556,20 @@ public void testTribeSettingNames() throws Exception { s, anyOf(startsWith("tribe.blocks"), startsWith("tribe.name"), startsWith("tribe.on_conflict")))); } + public void testNoTribeSecureSettings() throws Exception { + MockSecureSettings secureSettings = new MockSecureSettings(); + Path home = createTempDir(); + secureSettings.setString("xpack.security.http.ssl.keystore.secure_password", "dummypass"); + Settings settings = Settings.builder().setSecureSettings(secureSettings) + .put("path.home", home) + .put("tribe.t1.cluster.name", "foo") + .put("xpack.security.enabled", true).build(); + Security security = new Security(settings, home.resolve("config")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, security::additionalSettings); + assertThat(e.getMessage(), + equalTo("Secure setting [xpack.security.http.ssl.keystore.secure_password] cannot be used with tribe client node")); + } + private void assertTribeNodeHasAllIndices() throws Exception { assertBusy(() -> { Set indices = new HashSet<>(); From 80b15fe5a98f327d485fc3038b9d1311466dcb43 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 24 Jul 2018 08:42:24 -0700 Subject: [PATCH 2/2] address feedback --- .../org/elasticsearch/xpack/security/Security.java | 10 ++++++++-- .../xpack/security/SecurityTribeTests.java | 6 ++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index a026a9524fcbe..ad81e2ae1d727 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -789,14 +789,20 @@ private static void addTribeSettings(Settings settings, Settings.Builder setting // we passed all the checks now we need to copy in all of the x-pack security settings SecureSettings secureSettings = Settings.builder().put(settings).getSecureSettings(); // hack to get at secure settings... Set secureSettingKeys = secureSettings == null ? Collections.emptySet() : secureSettings.getSettingNames(); + List invalidSettings = new ArrayList<>(); for (String k : settings.keySet()) { if (k.startsWith("xpack.security.")) { if (secureSettingKeys.contains(k)) { - throw new IllegalArgumentException("Secure setting [" + k + "] cannot be used with tribe client node"); + invalidSettings.add(k); + } else { + settingsBuilder.copy(tribePrefix + k, k, settings); } - settingsBuilder.copy(tribePrefix + k, k, settings); } } + if (invalidSettings.isEmpty() == false) { + throw new IllegalArgumentException("Secure settings " + invalidSettings.toString() + + " cannot be used with tribe client node"); + } } Map realmsSettings = settings.getGroups(SecurityField.setting("authc.realms"), true); diff --git a/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java b/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java index 261416c7b0824..7165877199382 100644 --- a/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java +++ b/x-pack/qa/tribe-tests-with-security/src/test/java/org/elasticsearch/xpack/security/SecurityTribeTests.java @@ -560,14 +560,16 @@ public void testNoTribeSecureSettings() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); Path home = createTempDir(); secureSettings.setString("xpack.security.http.ssl.keystore.secure_password", "dummypass"); + secureSettings.setString("xpack.security.authc.token.passphrase", "dummypass"); Settings settings = Settings.builder().setSecureSettings(secureSettings) .put("path.home", home) .put("tribe.t1.cluster.name", "foo") .put("xpack.security.enabled", true).build(); Security security = new Security(settings, home.resolve("config")); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, security::additionalSettings); - assertThat(e.getMessage(), - equalTo("Secure setting [xpack.security.http.ssl.keystore.secure_password] cannot be used with tribe client node")); + // can't rely on order of the strings printed in the exception message + assertThat(e.getMessage(), containsString("xpack.security.http.ssl.keystore.secure_password")); + assertThat(e.getMessage(), containsString("xpack.security.authc.token.passphrase")); } private void assertTribeNodeHasAllIndices() throws Exception {