From 918e1823faa20f07a8aab11d883f8fc9b6b5b8cb Mon Sep 17 00:00:00 2001 From: Domenico Sibilio Date: Wed, 16 Feb 2022 15:53:24 +0100 Subject: [PATCH] ACS-2563 Disallow alfresco.secureComms=none --- e2e-test/helpers/wait-service-to-start.sh | 7 +- search-services/README.md | 4 +- .../solr/security/SecretSharedAuthPlugin.java | 27 +++- .../SecretSharedPropertyCollector.java | 118 ++++++++++++------ .../security/SecretSharedPropertyHelper.java | 9 +- .../SecretSharedPropertyCollectorTest.java | 118 ++++++++++++++++-- .../src/docker/search_config_setup.sh | 10 -- 7 files changed, 222 insertions(+), 71 deletions(-) diff --git a/e2e-test/helpers/wait-service-to-start.sh b/e2e-test/helpers/wait-service-to-start.sh index 2f599ba94..bbe1ac48f 100755 --- a/e2e-test/helpers/wait-service-to-start.sh +++ b/e2e-test/helpers/wait-service-to-start.sh @@ -14,7 +14,12 @@ do echo "Waiting for Service to start using endpoint: ${endpoint}" - until [[ "$(curl --output /dev/null -w ''%{http_code}'' --silent --head --fail ${endpoint})" == 200 ]] || [ "$COUNTER" -eq "$TIMEOUT" ]; do + additional_args=() + if [[ $endpoint == *"solr"* ]]; then + additional_args+=(-H "X-Alfresco-Search-Secret: secret") + fi + + until [[ "$(curl --output /dev/null -w ''%{http_code}'' "${additional_args[@]}" --silent --head --fail ${endpoint})" == 200 ]] || [ "$COUNTER" -eq "$TIMEOUT" ]; do printf '.' sleep $WAIT_INTERVAL COUNTER=$(($COUNTER+$WAIT_INTERVAL)) diff --git a/search-services/README.md b/search-services/README.md index ee27c57bb..4afff2545 100644 --- a/search-services/README.md +++ b/search-services/README.md @@ -250,10 +250,10 @@ $ unzip alfresco-search-services-*.zip $ cd alfresco-search-services ``` -Change default Alfresco Communication protocol to `none`. +Change default Alfresco Communication protocol to `none`, and set `alfresco.allowUnauthenticatedSolrEndpoint` to `true`: ```bash -$ sed -i 's/alfresco.secureComms=https/alfresco.secureComms=none/' solrhome/templates/rerank/conf/solrcore.properties +$ sed -i 's/alfresco.secureComms=https/alfresco.secureComms=none\nalfresco.allowUnauthenticatedSolrEndpoint=true/' solrhome/templates/rerank/conf/solrcore.properties ``` *Note* Above line is written in GNU sed, you can use `gsed` from Mac OS X or just edit the file with a Text Editor. diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedAuthPlugin.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedAuthPlugin.java index 302459b63..ff05bea2e 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedAuthPlugin.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedAuthPlugin.java @@ -2,7 +2,7 @@ * #%L * Alfresco Search Services * %% - * Copyright (C) 2005 - 2020 Alfresco Software Limited + * Copyright (C) 2005 - 2022 Alfresco Software Limited * %% * This file is part of the Alfresco software. * If the software was purchased under a paid Alfresco license, the terms of @@ -26,6 +26,8 @@ package org.alfresco.solr.security; +import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECURE_COMMS_PROPERTY; + import java.io.IOException; import java.util.Map; import java.util.Objects; @@ -49,6 +51,8 @@ import org.apache.solr.security.AuthenticationPlugin; public class SecretSharedAuthPlugin extends AuthenticationPlugin { + private static final String SECURE_COMMS_NONE = "none"; + /** * Verify that request header includes "secret" word when using "secret" communication method. * "alfresco.secureComms.secret" value is expected as Java environment variable. @@ -69,10 +73,17 @@ public class SecretSharedAuthPlugin extends AuthenticationPlugin return true; } - HttpServletResponse httpResponse = (HttpServletResponse) response; - httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, - "Authentication failure: \"" + SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY - + "\" method has been selected, use the right request header with the secret word"); + String errorMessage = "Authentication failure: \"" + SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY + + "\" method has been selected, use the right request header with the secret word"; + setErrorResponse(response, errorMessage); + return false; + } + else if (SECURE_COMMS_NONE.equals(SecretSharedPropertyCollector.getCommsMethod()) + && !SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()) + { + String errorMessage = "Authentication failure: \"" + SECURE_COMMS_PROPERTY + + "=none\" is no longer supported. Please use \"https\" or \"secret\" instead."; + setErrorResponse(response, errorMessage); return false; } @@ -81,6 +92,12 @@ public class SecretSharedAuthPlugin extends AuthenticationPlugin } + private void setErrorResponse(ServletResponse response, String errorMessage) throws IOException + { + HttpServletResponse httpResponse = (HttpServletResponse) response; + httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, errorMessage); + } + @Override public void init(Map parameters) { diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java index 3a946df1d..d56aa9636 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java @@ -26,13 +26,21 @@ package org.alfresco.solr.security; +import static java.util.function.Predicate.not; + import org.alfresco.httpclient.HttpClientFactory; import org.alfresco.solr.AlfrescoSolrDataModel; import org.alfresco.solr.config.ConfigUtil; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Map; import java.util.Objects; import java.util.Properties; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BiFunction; +import java.util.function.Consumer; /** * Provides property values for Alfresco Communication using "secret" method: @@ -50,10 +58,23 @@ public class SecretSharedPropertyCollector // Property names for "secret" communication method static final String SECURE_COMMS_PROPERTY = "alfresco.secureComms"; static final String SHARED_SECRET = "alfresco.secureComms.secret"; + static final String ALLOW_UNAUTHENTICATED_SOLR_PROPERTY = "alfresco.allowUnauthenticatedSolrEndpoint"; private static final String SHARED_SECRET_HEADER = "alfresco.secureComms.secret.header"; - // Save communication method as static value in order to improve performance - static String commsMethod; + // Memoize read properties to improve performance + static final Map PROPS_CACHE = new ConcurrentHashMap<>(); + // Ordered list of property location functions + private static final ArrayList>> PROPERTY_LOCATORS = new ArrayList<>(); + + static + { + // Environment variables + PROPERTY_LOCATORS.add((name, defaultValue) -> toSet(ConfigUtil.locateProperty(name, null))); + // Shared configuration (shared.properties file) + PROPERTY_LOCATORS.add((name, defaultValue) -> toSet(AlfrescoSolrDataModel.getCommonConfig().getProperty(name))); + // Configuration for each deployed SOLR Core + PROPERTY_LOCATORS.add((name, defaultValue) -> SecretSharedPropertyHelper.getPropertyFromCores(name, defaultValue)); + } /** * Check if communications method is "secret" @@ -65,50 +86,63 @@ public class SecretSharedPropertyCollector SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY); } + /** + * Check if unauthenticated Solr access is allowed + * @return true if unauthenticated Solr access is allowed + */ + public static boolean isAllowUnauthenticatedSolrEndpoint() + { + return Boolean.parseBoolean(PROPS_CACHE.computeIfAbsent(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, + key -> getProperty(key, "false"))); + } + /** * Get communication method from environment variables, shared properties or core properties. * @return Communication method: none, https, secret */ static String getCommsMethod() { - if (commsMethod == null) + return PROPS_CACHE.computeIfAbsent(SECURE_COMMS_PROPERTY, + key -> getProperty(key, "none", uniqueSecureCommsValidator())); + } + + private static String getProperty(String name, String defaultValue) + { + return getProperty(name, defaultValue, null); + } + + private static String getProperty(String name, String defaultValue, Consumer> propertySetValidator) + { + // Loop orderly through the property locators until the property is found + Set propertySet = PROPERTY_LOCATORS.stream() + .map(propertyLocator -> propertyLocator.apply(name, defaultValue)) + .filter(not(Set::isEmpty)) + .findFirst() + .orElse(Set.of()); + + if (propertySetValidator != null) { - - // Environment variable - commsMethod = ConfigUtil.locateProperty(SECURE_COMMS_PROPERTY, null); - - if (commsMethod == null) - { - // Shared configuration (shared.properties file) - commsMethod = AlfrescoSolrDataModel.getCommonConfig().getProperty(SECURE_COMMS_PROPERTY); - - if (commsMethod == null) - { - // Get configuration from deployed SOLR Cores - Set secureCommsSet = SecretSharedPropertyHelper.getCommsFromCores(); - - // In case of multiple cores, *all* of them must have the same secureComms value. - // From that perspective, you may find the second clause in the conditional statement - // below not strictly necessary. The reason is that the check below is in charge to make - // sure a consistent configuration about the secret shared property has been defined in all cores. - if (secureCommsSet.size() > 1 && secureCommsSet.contains(SECRET_SHARED_METHOD_KEY)) - { - throw new RuntimeException( - "No valid secure comms values: all the cores must be using \"secret\" communication method but found: " - + secureCommsSet); - } - - return commsMethod = - secureCommsSet.isEmpty() - ? null - : secureCommsSet.iterator().next(); - - } - } + // Run the propertySetValidator to eg. verify value uniqueness among multiple cores + propertySetValidator.accept(propertySet); } - return commsMethod; + return propertySet.isEmpty() ? null : propertySet.iterator().next(); + } + private static Consumer> uniqueSecureCommsValidator() + { + // In case of multiple cores, *all* of them must have the same secureComms value. + // From that perspective, you may find the second clause in the conditional statement + // below not strictly necessary. The reason is that the check below is in charge to make + // sure a consistent configuration about the secret shared property has been defined in all cores. + return secureCommsSet -> { + if (secureCommsSet.size() > 1 && secureCommsSet.contains(SECRET_SHARED_METHOD_KEY)) + { + throw new RuntimeException( + "No valid secure comms values: all the cores must be using \"secret\" communication method but found: " + + secureCommsSet); + } + }; } /** @@ -168,4 +202,16 @@ public class SecretSharedPropertyCollector return properties; } + private static Set toSet(String value) + { + Set propertySet = new HashSet<>(); + + if (value != null) + { + propertySet.add(value); + } + + return propertySet; + } + } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java index 785e931e5..4a7caec03 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java @@ -58,11 +58,12 @@ class SecretSharedPropertyHelper }; /** - * Read different values of "alfresco.secureComms" property from every "solrcore.properties" files. - * + * Read different values of the specified property from every "solrcore.properties" file. + * @param name The name of the property to read + * @param defaultValue The default value for the given property * @return List of different communication methods declared in SOLR Cores. */ - static Set getCommsFromCores() + static Set getPropertyFromCores(String name, String defaultValue) { try (Stream walk = Files.walk(Paths.get(SolrResourceLoader.locateSolrHome().toString()))) { @@ -74,7 +75,7 @@ class SecretSharedPropertyHelper return solrCorePropertiesFiles.stream() .map(toProperties) - .map(properties -> properties.getProperty(SECURE_COMMS_PROPERTY, "none")) + .map(properties -> properties.getProperty(name, defaultValue)) .collect(toSet()); } catch (IOException e) diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java index 29e52a4c0..97bf0a388 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java @@ -35,6 +35,9 @@ import java.util.Properties; import java.util.Set; import static java.util.Collections.emptySet; + +import static org.alfresco.solr.security.SecretSharedPropertyCollector.ALLOW_UNAUTHENTICATED_SOLR_PROPERTY; +import static org.alfresco.solr.security.SecretSharedPropertyCollector.PROPS_CACHE; import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY; import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECURE_COMMS_PROPERTY; import static org.alfresco.solr.security.SecretSharedPropertyCollector.SHARED_SECRET; @@ -51,22 +54,32 @@ public class SecretSharedPropertyCollectorTest private static final String SET_THROUGH_ALFRESCO_COMMON_CONFIG = "aCommsMethod_SetThroughAlfrescoCommonConfig"; private static final String COMMS_METHOD_FROM_SOLRCORE = "aCommsMethod_FromSolrCore"; private static final String SECRET_VALUE = "my-secret"; + private static final String SECURE_COMMS_NONE = "none"; + private static final String TRUE = "true"; + private static final String FALSE = "false"; + + private static final Set PROPS_TO_CLEAR = Set.of(SHARED_SECRET, SECURE_COMMS_PROPERTY, ALLOW_UNAUTHENTICATED_SOLR_PROPERTY); @Before public void setUp() { - SecretSharedPropertyCollector.commsMethod = null; - assertNull(System.getProperty(SHARED_SECRET)); - assertNull(System.getProperty(SECURE_COMMS_PROPERTY)); - assertNull(AlfrescoSolrDataModel.getCommonConfig().getProperty(SECURE_COMMS_PROPERTY)); + PROPS_CACHE.clear(); + + for (String property : PROPS_TO_CLEAR) + { + assertNull(System.getProperty(property)); + assertNull(AlfrescoSolrDataModel.getCommonConfig().getProperty(property)); + } } @After public void tearDown() { - System.clearProperty(SHARED_SECRET); - System.clearProperty(SECURE_COMMS_PROPERTY); - AlfrescoSolrDataModel.getCommonConfig().remove(SECURE_COMMS_PROPERTY); + for (String property : PROPS_TO_CLEAR) + { + System.clearProperty(property); + AlfrescoSolrDataModel.getCommonConfig().remove(property); + } } @Test @@ -82,10 +95,87 @@ public class SecretSharedPropertyCollectorTest SecretSharedPropertyCollector.getSecret(); } + @Test + public void allowUnauthenticatedSolrIsNotSet_shouldReturnFalse() + { + try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) + { + mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE)) + .thenReturn(emptySet()); + assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + } + + @Test + public void allowUnauthenticatedSolrIsTrueThroughSystemProperty_shouldReturnTrue() + { + System.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, TRUE); + assertTrue(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + + @Test + public void allowUnauthenticatedSolrIsFalseThroughSystemProperty_shouldReturnFalse() + { + System.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE); + assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + + @Test + public void allowUnauthenticatedSolrIsTrueThroughAlfrescoProperties_shouldReturnTrue() + { + try(MockedStatic mock = mockStatic(AlfrescoSolrDataModel.class)) + { + var alfrescoCommonConfig = new Properties(); + alfrescoCommonConfig.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, TRUE); + + mock.when(AlfrescoSolrDataModel::getCommonConfig).thenReturn(alfrescoCommonConfig); + + assertTrue(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + } + + @Test + public void allowUnauthenticatedSolrIsFalseThroughAlfrescoProperties_shouldReturnFalse() + { + try(MockedStatic mock = mockStatic(AlfrescoSolrDataModel.class)) + { + var alfrescoCommonConfig = new Properties(); + alfrescoCommonConfig.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE); + + mock.when(AlfrescoSolrDataModel::getCommonConfig).thenReturn(alfrescoCommonConfig); + + assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + } + + @Test + public void allowUnauthenticatedSolrIsTrueThroughSolrCores_shouldReturnTrue() + { + try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) + { + mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE)) + .thenReturn(Set.of(TRUE)); + + assertTrue(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + } + + @Test + public void allowUnauthenticatedSolrIsFalseThroughSolrCores_shouldReturnFalse() + { + try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) + { + mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE)) + .thenReturn(Set.of(FALSE)); + + assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint()); + } + } + @Test public void commsMethodIsNotNull_shouldReturnThatValue() { - SecretSharedPropertyCollector.commsMethod = A_COMMS_METHOD; + PROPS_CACHE.put(SECURE_COMMS_PROPERTY, A_COMMS_METHOD); assertEquals(A_COMMS_METHOD, SecretSharedPropertyCollector.getCommsMethod()); assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); @@ -94,7 +184,7 @@ public class SecretSharedPropertyCollectorTest @Test public void commsMethodIsNotNullAndIsSecret_shouldReturnThatValue() { - SecretSharedPropertyCollector.commsMethod = SECRET_SHARED_METHOD_KEY; + PROPS_CACHE.put(SECURE_COMMS_PROPERTY, SECRET_SHARED_METHOD_KEY); assertEquals(SECRET_SHARED_METHOD_KEY, SecretSharedPropertyCollector.getCommsMethod()); assertTrue(SecretSharedPropertyCollector.isCommsSecretShared()); @@ -138,7 +228,8 @@ public class SecretSharedPropertyCollectorTest { try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) { - mock.when(SecretSharedPropertyHelper::getCommsFromCores).thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE)); + mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(SECURE_COMMS_PROPERTY, SECURE_COMMS_NONE)) + .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE)); assertEquals(COMMS_METHOD_FROM_SOLRCORE, SecretSharedPropertyCollector.getCommsMethod()); assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); @@ -157,7 +248,8 @@ public class SecretSharedPropertyCollectorTest { try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) { - mock.when(SecretSharedPropertyHelper::getCommsFromCores).thenReturn(emptySet()); + mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(SECURE_COMMS_PROPERTY, SECURE_COMMS_NONE)) + .thenReturn(emptySet()); assertNull(SecretSharedPropertyCollector.getCommsMethod()); assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); @@ -173,8 +265,8 @@ public class SecretSharedPropertyCollectorTest { try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) { - mock.when(SecretSharedPropertyHelper::getCommsFromCores) - .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE, SECRET_SHARED_METHOD_KEY)); + mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(SECURE_COMMS_PROPERTY, SECURE_COMMS_NONE)) + .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE, SECRET_SHARED_METHOD_KEY)); SecretSharedPropertyCollector.getCommsMethod(); } diff --git a/search-services/packaging/src/docker/search_config_setup.sh b/search-services/packaging/src/docker/search_config_setup.sh index 8dd47a252..89688e905 100644 --- a/search-services/packaging/src/docker/search_config_setup.sh +++ b/search-services/packaging/src/docker/search_config_setup.sh @@ -90,7 +90,6 @@ if [[ ! -z "$SOLR_JAVA_MEM" ]]; then fi # By default Docker Image is using TLS Mutual Authentication (SSL) for communications with Repository -# Plain HTTP can be enabled by setting ALFRESCO_SECURE_COMMS to 'none' # Plain HTTP with a secret word in the request header can be enabled by setting ALFRESCO_SECURE_COMMS to 'secret', # the secret word should be defined as a JVM argument like so: JAVA_TOOL_OPTIONS="-Dalfresco.secureComms.secret=my-secret-value" case "$ALFRESCO_SECURE_COMMS" in @@ -103,15 +102,6 @@ case "$ALFRESCO_SECURE_COMMS" in sed -i "s/alfresco.secureComms=https/alfresco.secureComms=secret\n/" ${PWD}/solrhome/archive/conf/solrcore.properties fi ;; - none) - sed -i "s/alfresco.secureComms=https/alfresco.secureComms=none\n/" $SOLR_RERANK_CORE_FILE $SOLR_NORERANK_CORE_FILE - if [[ -f ${PWD}/solrhome/alfresco/conf/solrcore.properties ]]; then - sed -i "s/alfresco.secureComms=https/alfresco.secureComms=none\n/" ${PWD}/solrhome/alfresco/conf/solrcore.properties - fi - if [[ -f ${PWD}/solrhome/archive/conf/solrcore.properties ]]; then - sed -i "s/alfresco.secureComms=https/alfresco.secureComms=none\n/" ${PWD}/solrhome/archive/conf/solrcore.properties - fi - ;; https|'') ;; *)