Fix SoftwareProcess.dontRequireTtyForSudo - it doesn't directly edit /etc/sudoers - uses visudo to verify the prepared file - added integration test
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/af36c7f7 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/af36c7f7 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/af36c7f7 Branch: refs/heads/master Commit: af36c7f77621a428179646210302080f766b89c1 Parents: b03f77e Author: Yavor Yanchev <[email protected]> Authored: Thu Nov 26 11:30:58 2015 +0200 Committer: Yavor Yanchev <[email protected]> Committed: Thu Nov 26 13:02:39 2015 +0200 ---------------------------------------------------------------------- .../core/ssh/BashCommandsIntegrationTest.java | 29 ++++++++++++++++++++ .../resources/brooklyn/util/ssh/test_sudoers | 24 ++++++++++++++++ .../apache/brooklyn/util/ssh/BashCommands.java | 10 ++++++- 3 files changed, 62 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/af36c7f7/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java b/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java index b067bd8..3ef0337 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/ssh/BashCommandsIntegrationTest.java @@ -36,6 +36,7 @@ import java.util.List; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; +import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.core.task.BasicExecutionContext; import org.apache.brooklyn.util.core.task.ssh.SshTasks; import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper; @@ -72,6 +73,7 @@ public class BashCommandsIntegrationTest { private File sourceNonExistantFile; private File sourceFile1; private File sourceFile2; + private File tmpSudoersFile; private String sourceNonExistantFileUrl; private String sourceFileUrl1; private String sourceFileUrl2; @@ -108,6 +110,11 @@ public class BashCommandsIntegrationTest { localRepoEntityBasePath.mkdirs(); Files.write("mylocal1".getBytes(), localRepoEntityFile); + tmpSudoersFile = Os.newTempFile(getClass(), "sudoers" + Identifiers.makeRandomId(8)); + + String sudoers = ResourceUtils.create(this).getResourceAsString("classpath://brooklyn/util/ssh/test_sudoers"); + Files.write(sudoers.getBytes(), tmpSudoersFile); + loc = mgmt.getLocationManager().createLocation(LocalhostMachineProvisioningLocation.spec()).obtain(); } @@ -117,12 +124,34 @@ public class BashCommandsIntegrationTest { if (sourceFile2 != null) sourceFile2.delete(); if (destFile != null) destFile.delete(); if (localRepoEntityFile != null) localRepoEntityFile.delete(); + if (tmpSudoersFile != null) tmpSudoersFile.delete(); if (localRepoEntityBasePath != null) FileUtils.deleteDirectory(localRepoEntityBasePath); if (loc != null) loc.close(); if (mgmt != null) Entities.destroyAll(mgmt); } @Test(groups="Integration") + public void testRemoveRequireTtyFromSudoersFile() throws Exception { + String cmds = BashCommands.dontRequireTtyForSudo(); + + + ByteArrayOutputStream outStream = new ByteArrayOutputStream(); + ByteArrayOutputStream errStream = new ByteArrayOutputStream(); + + String cmdsWithReplacedSudoersName = Strings.replaceAllNonRegex(cmds, "/etc/sudoers", tmpSudoersFile.getAbsolutePath()); + int exitcode = loc.execCommands(ImmutableMap.of("out", outStream, "err", errStream), "removeRequireTtyFromSudoersFile", ImmutableList.of(cmdsWithReplacedSudoersName)); + + String outstr = new String(outStream.toByteArray()); + String errstr = new String(errStream.toByteArray()); + + assertEquals(0, exitcode); + + // visudo returns "parsed OK" + assertTrue(outstr.contains("parsed OK"), "out="+outstr+"; err="+errstr); + assertTrue(errstr.isEmpty(), "out="+outstr+"; err="+errstr); + } + + @Test(groups="Integration") public void testSudo() throws Exception { ByteArrayOutputStream outStream = new ByteArrayOutputStream(); ByteArrayOutputStream errStream = new ByteArrayOutputStream(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/af36c7f7/core/src/test/resources/brooklyn/util/ssh/test_sudoers ---------------------------------------------------------------------- diff --git a/core/src/test/resources/brooklyn/util/ssh/test_sudoers b/core/src/test/resources/brooklyn/util/ssh/test_sudoers new file mode 100644 index 0000000..fe2fa74 --- /dev/null +++ b/core/src/test/resources/brooklyn/util/ssh/test_sudoers @@ -0,0 +1,24 @@ +# 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. + +# Defaults specification + +# +# Disable "ssh hostname sudo <cmd>", because it will show the password in clear. +# You have to run "ssh -t hostname sudo <cmd>". +# +Defaults requiretty http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/af36c7f7/utils/common/src/main/java/org/apache/brooklyn/util/ssh/BashCommands.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/ssh/BashCommands.java b/utils/common/src/main/java/org/apache/brooklyn/util/ssh/BashCommands.java index eb082c2..f73ebc2 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/ssh/BashCommands.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/ssh/BashCommands.java @@ -162,7 +162,15 @@ public class BashCommands { * (having a tty for sudo seems like another case of imaginary security which is just irritating. * like water restrictions at airport security.) */ public static String dontRequireTtyForSudo() { - return ifFileExistsElse0("/etc/sudoers", sudo("sed -i.brooklyn.bak 's/.*requiretty.*/#brooklyn-removed-require-tty/' /etc/sudoers")); + String sudoersFileName = "/etc/sudoers"; + + // Visudo's quiet mode (-q) is not enabled. visudo's output is used for diagnostic purposes + return ifFileExistsElse0(sudoersFileName, + chainGroup( + sudo(format("cp %1$s %1$s.tmp", sudoersFileName)), + sudo(format("sed -i.brooklyn.bak 's/.*requiretty.*/#brooklyn-removed-require-tty/' %1$s.tmp", sudoersFileName)), + sudo(format("visudo -c -f %1$s.tmp", sudoersFileName)), + sudo(format("mv %1$s.tmp %1$s", sudoersFileName)))); } /** generates ~/.ssh/id_rsa if that file does not exist */
