Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 7C9FF200B50 for ; Fri, 29 Jul 2016 14:04:54 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 7B3B2160A79; Fri, 29 Jul 2016 12:04:54 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 4C78D160A61 for ; Fri, 29 Jul 2016 14:04:53 +0200 (CEST) Received: (qmail 12174 invoked by uid 500); 29 Jul 2016 12:04:52 -0000 Mailing-List: contact commits-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: ambari-dev@ambari.apache.org Delivered-To: mailing list commits@ambari.apache.org Received: (qmail 12165 invoked by uid 99); 29 Jul 2016 12:04:52 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 29 Jul 2016 12:04:52 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 61B5BE04BA; Fri, 29 Jul 2016 12:04:52 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: rlevas@apache.org To: commits@ambari.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: ambari git commit: AMBARI-17908. Ensure the supplied hostname is a valid hostname when signing agent-side host certs (rlevas) Date: Fri, 29 Jul 2016 12:04:52 +0000 (UTC) archived-at: Fri, 29 Jul 2016 12:04:54 -0000 Repository: ambari Updated Branches: refs/heads/trunk 296365a17 -> ad24dda21 AMBARI-17908. Ensure the supplied hostname is a valid hostname when signing agent-side host certs (rlevas) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/ad24dda2 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ad24dda2 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ad24dda2 Branch: refs/heads/trunk Commit: ad24dda214d189fd325f1c555b1b631c4b896ee8 Parents: 296365a Author: Robert Levas Authored: Fri Jul 29 08:04:39 2016 -0400 Committer: Robert Levas Committed: Fri Jul 29 08:04:39 2016 -0400 ---------------------------------------------------------------------- .../server/configuration/Configuration.java | 14 ++ .../server/security/CertificateManager.java | 46 ++++- .../apache/ambari/server/utils/HostUtils.java | 47 +++++ .../server/configuration/ConfigurationTest.java | 32 ++++ .../server/security/CertificateManagerTest.java | 175 +++++++++++++++++++ .../ambari/server/utils/HostUtilsTest.java | 47 +++++ 6 files changed, 354 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/ad24dda2/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java index 9655bf9..6e3eef7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java @@ -116,6 +116,7 @@ public class Configuration { public static final String API_GZIP_MIN_COMPRESSION_SIZE_KEY = "api.gzip.compression.min.size"; public static final String AGENT_API_GZIP_COMPRESSION_ENABLED_KEY = "agent.api.gzip.compression.enabled"; public static final String AGENT_USE_SSL = "agent.ssl"; + public static final String SRVR_AGENT_HOSTNAME_VALIDATE_KEY = "security.agent.hostname.validate"; public static final String SRVR_TWO_WAY_SSL_KEY = "security.server.two_way_ssl"; public static final String SRVR_TWO_WAY_SSL_PORT_KEY = "security.server.two_way_ssl.port"; public static final String SRVR_ONE_WAY_SSL_PORT_KEY = "security.server.one_way_ssl.port"; @@ -477,6 +478,7 @@ public class Configuration { private static final String SERVER_JDBC_USER_PASSWD_DEFAULT = "bigdata"; private static final String SERVER_JDBC_RCA_USER_NAME_DEFAULT = "mapred"; private static final String SERVER_JDBC_RCA_USER_PASSWD_DEFAULT = "mapred"; + private static final String SRVR_AGENT_HOSTNAME_VALIDATE_DEFAULT = "true"; private static final String SRVR_TWO_WAY_SSL_DEFAULT = "false"; private static final String SRVR_KSTR_DIR_DEFAULT = "."; private static final String API_CSRF_PREVENTION_DEFAULT = "true"; @@ -928,6 +930,8 @@ public class Configuration { configsMap.putAll(agentConfigsMap); configsMap.put(AMBARI_PYTHON_WRAP_KEY, properties.getProperty( AMBARI_PYTHON_WRAP_KEY, AMBARI_PYTHON_WRAP_DEFAULT)); + configsMap.put(SRVR_AGENT_HOSTNAME_VALIDATE_KEY, properties.getProperty( + SRVR_AGENT_HOSTNAME_VALIDATE_KEY, SRVR_AGENT_HOSTNAME_VALIDATE_DEFAULT)); configsMap.put(SRVR_TWO_WAY_SSL_KEY, properties.getProperty( SRVR_TWO_WAY_SSL_KEY, SRVR_TWO_WAY_SSL_DEFAULT)); configsMap.put(SRVR_TWO_WAY_SSL_PORT_KEY, properties.getProperty( @@ -1674,6 +1678,16 @@ public class Configuration { } /** + * Check to see if the hostname of the agent is to be validated as a proper hostname or not + * + * @return true if agent hostnames should be checked as a valid hostnames; otherwise false + */ + public boolean validateAgentHostnames() { + return ("true".equals(properties.getProperty(SRVR_AGENT_HOSTNAME_VALIDATE_KEY, + SRVR_AGENT_HOSTNAME_VALIDATE_DEFAULT))); + } + + /** * Check to see if two-way SSL auth should be used between server and agents * or not * http://git-wip-us.apache.org/repos/asf/ambari/blob/ad24dda2/ambari-server/src/main/java/org/apache/ambari/server/security/CertificateManager.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/CertificateManager.java b/ambari-server/src/main/java/org/apache/ambari/server/security/CertificateManager.java index b698ef3..9f70dc0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/CertificateManager.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/CertificateManager.java @@ -20,10 +20,12 @@ package org.apache.ambari.server.security; import com.google.inject.Inject; import com.google.inject.Singleton; import org.apache.ambari.server.configuration.Configuration; +import org.apache.ambari.server.utils.HostUtils; import org.apache.ambari.server.utils.ShellCommandUtil; import org.apache.commons.io.FileUtils; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; +import org.apache.commons.lang.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.BufferedReader; import java.io.File; @@ -40,10 +42,9 @@ import java.util.Map; @Singleton public class CertificateManager { - @Inject Configuration configs; - - private static final Log LOG = LogFactory.getLog(CertificateManager.class); + private static final Logger LOG = LoggerFactory.getLogger(CertificateManager.class); + @Inject Configuration configs; private static final String GEN_SRVR_KEY = "openssl genrsa -des3 " + "-passout pass:{0} -out {1}" + File.separator + "{2} 4096 "; @@ -97,7 +98,7 @@ public class CertificateManager { * * @return command execution exit code */ - private int runCommand(String command) { + protected int runCommand(String command) { String line = null; Process process = null; BufferedReader br= null; @@ -185,7 +186,38 @@ public class CertificateManager { */ public synchronized SignCertResponse signAgentCrt(String agentHostname, String agentCrtReqContent, String passphraseAgent) { SignCertResponse response = new SignCertResponse(); - LOG.info("Signing of agent certificate"); + LOG.info("Signing agent certificate"); + + // Ensure the hostname is not empty or null... + agentHostname = StringUtils.trim(agentHostname); + + if(StringUtils.isEmpty(agentHostname)) { + LOG.warn("The agent hostname is missing"); + response.setResult(SignCertResponse.ERROR_STATUS); + response.setMessage("The agent hostname is missing"); + return response; + } + + // Optionally check the supplied hostname to make sure it is a valid hostname. + // By default, this feature is turned on. If this check is not desired (maybe the validation + // rules are too strict), the feature may be turned off by setting the following + // property in the ambari.properties file: + // + // security.agent.hostname.validate = "false" + // + if(configs.validateAgentHostnames()) { + LOG.info("Validating agent hostname: {}", agentHostname); + if(!HostUtils.isValidHostname(agentHostname)) { + LOG.warn("The agent hostname is not a valid hostname"); + response.setResult(SignCertResponse.ERROR_STATUS); + response.setMessage("The agent hostname is not a valid hostname"); + return response; + } + } + else { + LOG.info("Skipping validation of agent hostname: {}", agentHostname); + } + LOG.info("Verifying passphrase"); String passphraseSrvr = configs.getConfigsMap().get(Configuration. http://git-wip-us.apache.org/repos/asf/ambari/blob/ad24dda2/ambari-server/src/main/java/org/apache/ambari/server/utils/HostUtils.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/utils/HostUtils.java b/ambari-server/src/main/java/org/apache/ambari/server/utils/HostUtils.java new file mode 100644 index 0000000..36c16f5 --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/utils/HostUtils.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.ambari.server.utils; + +import java.util.regex.Pattern; + +/** + * HostUtils provides utilities related to processing and validating hosts and host names. + */ +public class HostUtils { + /** + * A regular expression to validate that a hostname meets the specifications described in RFC 1123. + *

+ * Various sources on the Internet specific this regular expression for hostname validation. + */ + private static final Pattern REGEX_VALID_HOSTNAME = Pattern.compile("^(?:(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\\-]*[a-zA-Z0-9])\\.)*(?:[A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\\-]*[A-Za-z0-9])$"); + + /** + * Validates a hostname to ensure it meets the specifications described in RFC 1123. + *

+ * Note: this does not validate whether the host exists or if the name is a valid DNS name. + * For example, host.example.anything is a valid host name however may be an invalid DNS name + * since "anything" may not be a valid top-level domain. + * + * @param hostname a hostname + * @return true if the hostname is valid; false otherwise + */ + public static boolean isValidHostname(String hostname) { + return (hostname != null) && REGEX_VALID_HOSTNAME.matcher(hostname).matches(); + } +} http://git-wip-us.apache.org/repos/asf/ambari/blob/ad24dda2/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java b/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java index b5d0e02..e9aebc0 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java @@ -72,6 +72,38 @@ public class ConfigurationTest { } /** + * ambari.properties doesn't contain "security.agent.hostname.validate" option + */ + @Test + public void testValidateAgentHostnames() { + Assert.assertTrue(new Configuration().validateAgentHostnames()); + } + + /** + * ambari.properties contains "security.agent.hostname.validate=true" option + */ + @Test + public void testValidateAgentHostnamesOn() { + Properties ambariProperties = new Properties(); + ambariProperties.setProperty(Configuration.SRVR_AGENT_HOSTNAME_VALIDATE_KEY, "true"); + Configuration conf = new Configuration(ambariProperties); + Assert.assertTrue(conf.validateAgentHostnames()); + Assert.assertEquals("true", conf.getConfigsMap().get(Configuration.SRVR_AGENT_HOSTNAME_VALIDATE_KEY)); + } + + /** + * ambari.properties contains "security.agent.hostname.validate=false" option + */ + @Test + public void testValidateAgentHostnamesOff() { + Properties ambariProperties = new Properties(); + ambariProperties.setProperty(Configuration.SRVR_AGENT_HOSTNAME_VALIDATE_KEY, "false"); + Configuration conf = new Configuration(ambariProperties); + Assert.assertFalse(conf.validateAgentHostnames()); + Assert.assertEquals("false", conf.getConfigsMap().get(Configuration.SRVR_AGENT_HOSTNAME_VALIDATE_KEY)); + } + + /** * ambari.properties doesn't contain "security.server.two_way_ssl" option * @throws Exception */ http://git-wip-us.apache.org/repos/asf/ambari/blob/ad24dda2/ambari-server/src/test/java/org/apache/ambari/server/security/CertificateManagerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/CertificateManagerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/CertificateManagerTest.java new file mode 100644 index 0000000..f28f234 --- /dev/null +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/CertificateManagerTest.java @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.ambari.server.security; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import junit.framework.Assert; +import org.apache.ambari.server.configuration.Configuration; +import org.apache.ambari.server.state.stack.OsFamily; +import org.easymock.EasyMockSupport; +import org.easymock.IAnswer; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import static org.easymock.EasyMock.expect; + +public class CertificateManagerTest extends EasyMockSupport { + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + @Test + public void testSignAgentCrt() throws Exception { + Injector injector = getInjector(); + + File directory = folder.newFolder(); + + String hostname = "host1.example.com"; + + Map configurationMap = new HashMap(); + configurationMap.put(Configuration.SRVR_KSTR_DIR_KEY, directory.getAbsolutePath()); + configurationMap.put(Configuration.SRVR_CRT_PASS_KEY, "server_cert_pass"); + configurationMap.put(Configuration.SRVR_CRT_NAME_KEY, "server_cert_name"); + configurationMap.put(Configuration.SRVR_KEY_NAME_KEY, "server_key_name"); + configurationMap.put(Configuration.PASSPHRASE_KEY, "passphrase"); + + Configuration configuration = injector.getInstance(Configuration.class); + expect(configuration.validateAgentHostnames()).andReturn(true).once(); + expect(configuration.getConfigsMap()).andReturn(configurationMap).anyTimes(); + + Method runCommand = CertificateManager.class.getDeclaredMethod("runCommand", String.class); + + final File agentCrtFile = new File(directory, String.format("%s.crt", hostname)); + + String expectedCommand = String.format("openssl ca -config %s/ca.config -in %s/%s.csr -out %s -batch -passin pass:%s -keyfile %s/%s -cert %s/%s", + directory.getAbsolutePath(), + directory.getAbsolutePath(), + hostname, + agentCrtFile.getAbsolutePath(), + configurationMap.get(Configuration.SRVR_CRT_PASS_KEY), + directory.getAbsolutePath(), + configurationMap.get(Configuration.SRVR_KEY_NAME_KEY), + directory.getAbsolutePath(), + configurationMap.get(Configuration.SRVR_CRT_NAME_KEY)); + + CertificateManager certificateManager = createMockBuilder(CertificateManager.class) + .addMockedMethod(runCommand) + .createMock(); + expect(certificateManager.runCommand(expectedCommand)) + .andAnswer(new IAnswer() { + @Override + public Integer answer() throws Throwable { + return (agentCrtFile.createNewFile()) ? 0 : 1; + } + }) + .once(); + + injector.injectMembers(certificateManager); + + replayAll(); + + SignCertResponse response = certificateManager.signAgentCrt(hostname, "crtContent", "passphrase"); + + verifyAll(); + + Assert.assertEquals(SignCertResponse.OK_STATUS, response.getResult()); + } + + @Test + public void testSignAgentCrtInvalidHostname() throws Exception { + Injector injector = getInjector(); + + Configuration configuration = injector.getInstance(Configuration.class); + expect(configuration.validateAgentHostnames()).andReturn(true).once(); + + replayAll(); + + CertificateManager certificateManager = new CertificateManager(); + injector.injectMembers(certificateManager); + + SignCertResponse response = certificateManager.signAgentCrt("hostname; echo \"hello\" > /tmp/hello.txt;", "crtContent", "passphrase"); + + verifyAll(); + + Assert.assertEquals(SignCertResponse.ERROR_STATUS, response.getResult()); + Assert.assertEquals("The agent hostname is not a valid hostname", response.getMessage()); + } + + @Test + public void testSignAgentCrtBadPassphrase() throws Exception { + Injector injector = getInjector(); + + Configuration configuration = injector.getInstance(Configuration.class); + expect(configuration.validateAgentHostnames()).andReturn(true).once(); + expect(configuration.getConfigsMap()).andReturn(Collections.singletonMap(Configuration.PASSPHRASE_KEY, "some_passphrase")).once(); + + replayAll(); + + CertificateManager certificateManager = new CertificateManager(); + injector.injectMembers(certificateManager); + + SignCertResponse response = certificateManager.signAgentCrt("host1.example.com", "crtContent", "passphrase"); + + verifyAll(); + + Assert.assertEquals(SignCertResponse.ERROR_STATUS, response.getResult()); + Assert.assertEquals("Incorrect passphrase from the agent", response.getMessage()); + } + + @Test + public void testSignAgentCrtInvalidHostnameIgnoreBadPassphrase() throws Exception { + Injector injector = getInjector(); + + Configuration configuration = injector.getInstance(Configuration.class); + expect(configuration.validateAgentHostnames()).andReturn(false).once(); + expect(configuration.getConfigsMap()).andReturn(Collections.singletonMap(Configuration.PASSPHRASE_KEY, "some_passphrase")).once(); + + replayAll(); + + CertificateManager certificateManager = new CertificateManager(); + injector.injectMembers(certificateManager); + + SignCertResponse response = certificateManager.signAgentCrt("hostname; echo \"hello\" > /tmp/hello.txt;", "crtContent", "passphrase"); + + verifyAll(); + + Assert.assertEquals(SignCertResponse.ERROR_STATUS, response.getResult()); + Assert.assertEquals("Incorrect passphrase from the agent", response.getMessage()); + } + + private Injector getInjector() { + return Guice.createInjector(new AbstractModule() { + + @Override + protected void configure() { + bind(OsFamily.class).toInstance(createNiceMock(OsFamily.class)); + bind(Configuration.class).toInstance(createMock(Configuration.class)); + } + }); + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ambari/blob/ad24dda2/ambari-server/src/test/java/org/apache/ambari/server/utils/HostUtilsTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/utils/HostUtilsTest.java b/ambari-server/src/test/java/org/apache/ambari/server/utils/HostUtilsTest.java new file mode 100644 index 0000000..341e214 --- /dev/null +++ b/ambari-server/src/test/java/org/apache/ambari/server/utils/HostUtilsTest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.ambari.server.utils; + +import junit.framework.Assert; +import org.junit.Test; + +public class HostUtilsTest { + @Test + public void testIsValidHostname() throws Exception { + + // Valid host names + Assert.assertTrue(HostUtils.isValidHostname("localhost")); + Assert.assertTrue(HostUtils.isValidHostname("localhost.localdomain")); + Assert.assertTrue(HostUtils.isValidHostname("host1.example.com")); + Assert.assertTrue(HostUtils.isValidHostname("Host1.eXample.coM")); + Assert.assertTrue(HostUtils.isValidHostname("host-name.example.com")); + Assert.assertTrue(HostUtils.isValidHostname("123.456.789")); + Assert.assertTrue(HostUtils.isValidHostname("host-123-name.ex4mpl3.c0m")); + + // Invalid host names + Assert.assertFalse(HostUtils.isValidHostname("host_name.example.com")); + Assert.assertFalse(HostUtils.isValidHostname("host;name.example.com")); + Assert.assertFalse(HostUtils.isValidHostname("host?name.example.com")); + Assert.assertFalse(HostUtils.isValidHostname("host@name.example.com")); + Assert.assertFalse(HostUtils.isValidHostname("host=name.example.com")); + Assert.assertFalse(HostUtils.isValidHostname("host+name.example.com")); + Assert.assertFalse(HostUtils.isValidHostname("host)name).example.com")); + } + +} \ No newline at end of file