From 66fe664efa7d471771f89edd9ea434e3b88cc81d Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Fri, 26 Jul 2024 12:19:28 -0400 Subject: [PATCH 1/2] Addresses a bug with `plugins.security.allow_unsafe_democertificates` setting (#4600) Signed-off-by: Darshit Chanpura Signed-off-by: Terry Quigley --- .../opensearch/security/OpenSearchSecurityPlugin.java | 7 +++++++ .../security/tools/democonfig/Certificates.java | 10 +++++----- .../tools/democonfig/CertificateGeneratorTests.java | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 3f1905d281..811c72f51e 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -378,6 +378,13 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) demoCertHashes.add("a2ce3f577a5031398c1b4f58761444d837b031d0aff7614f8b9b5e4a9d59dbd1"); // esnode demoCertHashes.add("cd708e8dc707ae065f7ad8582979764b497f062e273d478054ab2f49c5469c6"); // root-ca + // updates correct sha256sum + demoCertHashes.add("a3556d6bb61f7bd63cb19b1c8d0078d30c12739dedb0455c5792ac8627782042"); // kirk + demoCertHashes.add("25e34a9a5d4f1dceed1666eb624397bf3fe5787a7133cd32838ace0381bce1f7"); // kirk-key + demoCertHashes.add("a2ce3f577a5031398c1b4f58761444d837b031d0aff7614f8b9b5e4a9d59dbd1"); // esnode + demoCertHashes.add("ba9c5a61065f7f6115188128ffbdaa18fca34562b78b811f082439e2bef1d282"); // esnode-key + demoCertHashes.add("bcd708e8dc707ae065f7ad8582979764b497f062e273d478054ab2f49c5469c6"); // root-ca + final SecurityManager sm = System.getSecurityManager(); if (sm != null) { diff --git a/src/main/java/org/opensearch/security/tools/democonfig/Certificates.java b/src/main/java/org/opensearch/security/tools/democonfig/Certificates.java index baff8d7078..3ec18bc373 100644 --- a/src/main/java/org/opensearch/security/tools/democonfig/Certificates.java +++ b/src/main/java/org/opensearch/security/tools/democonfig/Certificates.java @@ -48,7 +48,7 @@ public enum Certificates { "KRVHWCFiR7bZhHGLq3br8hSu0hwjb3oGa1ZI8dui6ujyZt6nm6BoEkau3G/6+zq9", "E6vX3+8Fj4HKCAL6i0SwfGmEpTNp5WUhqibK/fMhhmMT4Mx6MxkT+OFnIjdUU0S/", "e3kgnG8qjficUr38CyEli1U0M7koIXUZI7r+LQ==", - "-----END CERTIFICATE-----" + "-----END CERTIFICATE-----\n" ) ) ), @@ -83,7 +83,7 @@ public enum Certificates { "mQGwy8vIqMjAdHGLrCS35sVYBXG13knS52LJHvbVee39AbD5/LlWvjJGlQMzCLrw", "F7oILW5kXxhb8S73GWcuMbuQMFVHFONbZAZgn+C9FW4l7XyRdkrbR1MRZ2km8YMs", "/AHmo368d4PSNRMMzLHw8Q==", - "-----END PRIVATE KEY-----" + "-----END PRIVATE KEY-----\n" ) ) ), @@ -115,7 +115,7 @@ public enum Certificates { "hUBqIEAYly1EqH/y45APiRt3Nor1yF6zEI4TnL0yNrHw6LyQkUNCHIGMJLfnJQ9L", "camMGIXOx60kXNMTigF9oXXwixWAnDM9y3QT8QXA7hej/4zkbO+vIeV/7lGUdkyg", "PAi92EvyxmsliEMyMR0VINl8emyobvfwa7oMeWMR+hg=", - "-----END CERTIFICATE-----" + "-----END CERTIFICATE-----\n" ) ) ), @@ -150,7 +150,7 @@ public enum Certificates { "tu49A/0KZu4PBjrFMYTSEWGNJez3Fb2VsJwylVl6HivwbP61FhlYfyksCzQQFU71", "+x7Nmybp7PmpEBECr3deoZKQ/acNHn0iwb0It+YqV5+TquQebqgwK6WCLsMuiYKT", "bg/ch9Rhxbq22yrVgWHh6epp", - "-----END PRIVATE KEY-----" + "-----END PRIVATE KEY-----\n" ) ) ), @@ -185,7 +185,7 @@ public enum Certificates { "1yVJon6RkUGtqBqKIuLksKwEr//ELnjmXit4LQKSnqKr0FTCB7seIrKJNyb35Qnq", "qy9a/Unhokrmdda1tr6MbqU8l7HmxLuSd/Ky+L0eDNtYv6YfMewtjg0TtAnFyQov", "rdXmeq1dy9HLo3Ds4AFz3Gx9076TxcRS/iI=", - "-----END CERTIFICATE-----" + "-----END CERTIFICATE-----\n" ) ) ); diff --git a/src/test/java/org/opensearch/security/tools/democonfig/CertificateGeneratorTests.java b/src/test/java/org/opensearch/security/tools/democonfig/CertificateGeneratorTests.java index 71771f8116..f7f13988c8 100644 --- a/src/test/java/org/opensearch/security/tools/democonfig/CertificateGeneratorTests.java +++ b/src/test/java/org/opensearch/security/tools/democonfig/CertificateGeneratorTests.java @@ -165,7 +165,7 @@ private static String readPEMFile(String pemFilePath) throws Exception { try (BufferedReader reader = new BufferedReader(new FileReader(pemFilePath))) { String line; while ((line = reader.readLine()) != null) { - pemContent.append(line).append("\n"); + pemContent.append(line); } } return pemContent.toString(); From 7f8889e2a5c1246773acfd73ff2a95ef536819dd Mon Sep 17 00:00:00 2001 From: Terry Quigley Date: Sat, 27 Jul 2024 12:48:06 +0100 Subject: [PATCH 2/2] Refactor security provider instantiation Signed-off-by: Terry Quigley --- .../security/OpenSearchSecurityPlugin.java | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 811c72f51e..509b98f12e 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -36,6 +36,7 @@ import java.security.AccessController; import java.security.MessageDigest; import java.security.PrivilegedAction; +import java.security.Provider; import java.security.Security; import java.util.ArrayList; import java.util.Arrays; @@ -63,7 +64,6 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.search.QueryCachingPolicy; import org.apache.lucene.search.Weight; -import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.opensearch.OpenSearchException; import org.opensearch.OpenSearchSecurityException; @@ -385,18 +385,7 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) demoCertHashes.add("ba9c5a61065f7f6115188128ffbdaa18fca34562b78b811f082439e2bef1d282"); // esnode-key demoCertHashes.add("bcd708e8dc707ae065f7ad8582979764b497f062e273d478054ab2f49c5469c6"); // root-ca - final SecurityManager sm = System.getSecurityManager(); - - if (sm != null) { - sm.checkPermission(new SpecialPermission()); - } - - AccessController.doPrivileged((PrivilegedAction) () -> { - if (Security.getProvider("BC") == null) { - Security.addProvider(new BouncyCastleProvider()); - } - return null; - }); + tryAddSecurityProvider(); final String advancedModulesEnabledKey = ConfigConstants.SECURITY_ADVANCED_MODULES_ENABLED; if (settings.hasValue(advancedModulesEnabledKey)) { @@ -2128,6 +2117,30 @@ public Optional getSecureSettingFactory(Settings settings return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, sslExceptionHandler, securityRestHandler)); } + @SuppressWarnings("removal") + private void tryAddSecurityProvider() { + final SecurityManager sm = System.getSecurityManager(); + + if (sm != null) { + sm.checkPermission(new SpecialPermission()); + } + + // Add provider if on the classpath. + AccessController.doPrivileged((PrivilegedAction) () -> { + if (Security.getProvider("BC") == null) { + try { + Class providerClass = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider"); + Provider provider = (Provider) providerClass.getDeclaredConstructor().newInstance(); + Security.addProvider(provider); + log.debug("Bouncy Castle Provider added"); + } catch (Exception e) { + log.debug("Bouncy Castle Provider could not be added", e); + } + } + return null; + }); + } + public static class GuiceHolder implements LifecycleComponent { private static RepositoriesService repositoriesService;