From 4c3cf76cb9dac8c54ce104dc961be41096a9d5d4 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 15 Mar 2017 10:36:17 -0700 Subject: [PATCH] Enable explicitly enforcing bootstrap checks This commit adds a system property that enables end-users to explicitly enforce the bootstrap checks, independently of the binding of the transport protocol. This can be useful for single-node production systems that do not bind the transport protocol (and thus the bootstrap checks would not be enforced). Relates #23585 --- .../bootstrap/BootstrapChecks.java | 43 +++++-- ...ckTests.java => BootstrapChecksTests.java} | 2 +- .../reference/setup/bootstrap-checks.asciidoc | 7 +- .../bootstrap/EvilBootstrapChecksTests.java | 116 ++++++++++++++++++ 4 files changed, 158 insertions(+), 10 deletions(-) rename core/src/test/java/org/elasticsearch/bootstrap/{BootstrapCheckTests.java => BootstrapChecksTests.java} (99%) create mode 100644 qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapChecksTests.java diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java index f57216418354e..775671c229352 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java @@ -43,20 +43,26 @@ import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; /** - * We enforce bootstrap checks once a node has the transport protocol bound to a non-loopback interface. In this case we assume the node is - * running in production and all bootstrap checks must pass. + * We enforce bootstrap checks once a node has the transport protocol bound to a non-loopback interface or if the system property {@code + * es.enforce.bootstrap.checks} is set to {@true}. In this case we assume the node is running in production and all bootstrap checks must + * pass. */ final class BootstrapChecks { private BootstrapChecks() { } + static final String ES_ENFORCE_BOOTSTRAP_CHECKS = "es.enforce.bootstrap.checks"; + /** - * Executes the bootstrap checks if the node has the transport protocol bound to a non-loopback interface. + * Executes the bootstrap checks if the node has the transport protocol bound to a non-loopback interface. If the system property + * {@code es.enforce.bootstrap.checks} is set to {@code true} then the bootstrap checks will be enforced regardless of whether or not + * the transport protocol is bound to a non-loopback interface. * * @param settings the current node settings * @param boundTransportAddress the node network bindings @@ -73,7 +79,9 @@ static void check(final Settings settings, final BoundTransportAddress boundTran } /** - * Executes the provided checks and fails the node if {@code enforceLimits} is {@code true}, otherwise logs warnings. + * Executes the provided checks and fails the node if {@code enforceLimits} is {@code true}, otherwise logs warnings. If the system + * property {@code es.enforce.bootstrap.checks} is set to {@code true} then the bootstrap checks will be enforced regardless of whether + * or not the transport protocol is bound to a non-loopback interface. * * @param enforceLimits {@code true} if the checks should be enforced or otherwise warned * @param checks the checks to execute @@ -87,7 +95,9 @@ static void check( } /** - * Executes the provided checks and fails the node if {@code enforceLimits} is {@code true}, otherwise logs warnings. + * Executes the provided checks and fails the node if {@code enforceLimits} is {@code true}, otherwise logs warnings. If the system + * property {@code es.enforce.bootstrap.checks }is set to {@code true} then the bootstrap checks will be enforced regardless of whether + * or not the transport protocol is bound to a non-loopback interface. * * @param enforceLimits {@code true} if the checks should be enforced or otherwise warned * @param checks the checks to execute @@ -100,13 +110,31 @@ static void check( final List errors = new ArrayList<>(); final List ignoredErrors = new ArrayList<>(); + final String esEnforceBootstrapChecks = System.getProperty(ES_ENFORCE_BOOTSTRAP_CHECKS); + final boolean enforceBootstrapChecks; + if (esEnforceBootstrapChecks == null) { + enforceBootstrapChecks = false; + } else if (Boolean.TRUE.toString().equals(esEnforceBootstrapChecks)) { + enforceBootstrapChecks = true; + } else { + final String message = + String.format( + Locale.ROOT, + "[%s] must be [true] but was [%s]", + ES_ENFORCE_BOOTSTRAP_CHECKS, + esEnforceBootstrapChecks); + throw new IllegalArgumentException(message); + } + if (enforceLimits) { logger.info("bound or publishing to a non-loopback or non-link-local address, enforcing bootstrap checks"); + } else if (enforceBootstrapChecks) { + logger.info("explicitly enforcing bootstrap checks"); } for (final BootstrapCheck check : checks) { if (check.check()) { - if (!enforceLimits && !check.alwaysEnforce()) { + if (!(enforceLimits || enforceBootstrapChecks) && !check.alwaysEnforce()) { ignoredErrors.add(check.errorMessage()); } else { errors.add(check.errorMessage()); @@ -126,7 +154,6 @@ static void check( errors.stream().map(IllegalStateException::new).forEach(ne::addSuppressed); throw ne; } - } static void log(final Logger logger, final String error) { @@ -139,7 +166,7 @@ static void log(final Logger logger, final String error) { * @param boundTransportAddress the node network bindings * @return {@code true} if the checks should be enforced */ - static boolean enforceLimits(BoundTransportAddress boundTransportAddress) { + static boolean enforceLimits(final BoundTransportAddress boundTransportAddress) { return !(Arrays.stream(boundTransportAddress.boundAddresses()).allMatch(TransportAddress::isLoopbackOrLinkLocalAddress) && boundTransportAddress.publishAddress().isLoopbackOrLinkLocalAddress()); } diff --git a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapChecksTests.java similarity index 99% rename from core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java rename to core/src/test/java/org/elasticsearch/bootstrap/BootstrapChecksTests.java index b943d67491db1..c3eb60ceec3c2 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapChecksTests.java @@ -48,7 +48,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -public class BootstrapCheckTests extends ESTestCase { +public class BootstrapChecksTests extends ESTestCase { public void testNonProductionMode() throws NodeValidationException { // nothing should happen since we are in non-production mode diff --git a/docs/reference/setup/bootstrap-checks.asciidoc b/docs/reference/setup/bootstrap-checks.asciidoc index 0a7ca6989e25d..8c30474117c96 100644 --- a/docs/reference/setup/bootstrap-checks.asciidoc +++ b/docs/reference/setup/bootstrap-checks.asciidoc @@ -35,7 +35,12 @@ production mode if it does bind transport to an external interface. Note that HTTP can be configured independently of transport via <> and <>; this can be useful for configuring a single instance to be reachable via -HTTP for testing purposes without triggering production mode. +HTTP for testing purposes without triggering production mode. If you do +want to force enforcement of the bootstrap checks independent of the +binding of the transport protocal, you can set the system property +`es.enforce.bootstrap.checks` to `true` (this can be useful on a +single-node production system that does not bind transport to an external +interface). === Heap size check diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapChecksTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapChecksTests.java new file mode 100644 index 0000000000000..372ae3a8885a9 --- /dev/null +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapChecksTests.java @@ -0,0 +1,116 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +package org.elasticsearch.bootstrap; + +import org.apache.logging.log4j.Logger; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.node.NodeValidationException; +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matcher; +import org.junit.After; +import org.junit.Before; + +import java.util.Collections; +import java.util.List; + +import static java.util.Collections.emptyList; +import static org.elasticsearch.bootstrap.BootstrapChecks.ES_ENFORCE_BOOTSTRAP_CHECKS; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasToString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +public class EvilBootstrapChecksTests extends ESTestCase { + + private String esEnforceBootstrapChecks = System.getProperty(ES_ENFORCE_BOOTSTRAP_CHECKS); + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + } + + @Override + @After + public void tearDown() throws Exception { + setEsEnforceBootstrapChecks(esEnforceBootstrapChecks); + super.tearDown(); + } + + public void testEnforceBootstrapChecks() throws NodeValidationException { + setEsEnforceBootstrapChecks("true"); + final List checks = Collections.singletonList( + new BootstrapCheck() { + @Override + public boolean check() { + return true; + } + + @Override + public String errorMessage() { + return "error"; + } + } + ); + final Logger logger = mock(Logger.class); + + final NodeValidationException e = expectThrows( + NodeValidationException.class, + () -> BootstrapChecks.check(false, checks, logger)); + final Matcher allOf = + allOf(containsString("bootstrap checks failed"), containsString("error")); + assertThat(e, hasToString(allOf)); + verify(logger).info("explicitly enforcing bootstrap checks"); + verifyNoMoreInteractions(logger); + } + + public void testNonEnforcedBootstrapChecks() throws NodeValidationException { + setEsEnforceBootstrapChecks(null); + final Logger logger = mock(Logger.class); + // nothing should happen + BootstrapChecks.check(false, emptyList(), logger); + verifyNoMoreInteractions(logger); + } + + public void testInvalidValue() { + final String value = randomAsciiOfLength(8); + setEsEnforceBootstrapChecks(value); + final boolean enforceLimits = randomBoolean(); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> BootstrapChecks.check(enforceLimits, emptyList(), "testInvalidValue")); + final Matcher matcher = containsString( + "[es.enforce.bootstrap.checks] must be [true] but was [" + value + "]"); + assertThat(e, hasToString(matcher)); + } + + @SuppressForbidden(reason = "set or clear system property es.enforce.bootstrap.checks") + public void setEsEnforceBootstrapChecks(final String value) { + if (value == null) { + System.clearProperty(ES_ENFORCE_BOOTSTRAP_CHECKS); + } else { + System.setProperty(ES_ENFORCE_BOOTSTRAP_CHECKS, value); + } + } + +}