From a09e86f09fb8bd6e7d678d971f3853de04a039f6 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 15 Jun 2024 08:43:34 -0600 Subject: [PATCH] [JENKINS-73305] Create .ssh directory with owner only permissions When the JGit implementation needs to create a `.ssh` directory, create it with permissions only allowing access to the directory owner. That is the common pattern used by the OpenSSH project and by POSIX systems to reduce access to the sensitive information stored in the directory. Testing done Ran the CredentialsTest in a debugger with a configured 'auth-data` directory and confirmed that the modified lines are executed on my RHEL 8 development computer. Confirmed that the resulting directory permissions were read, write, and execute for only the owner, with no other permissions. --- .../plugins/gitclient/JGitAPIImpl.java | 28 +++++++++++++--- .../verifier/KnownHostsFileVerifier.java | 32 +++++++++++++++---- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index 7bbccc2db9..9691c2d912 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -42,6 +42,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Arrays; @@ -201,10 +204,22 @@ public class JGitAPIImpl extends LegacyCompatibleGitAPIImpl { public SshdSessionFactory buildSshdSessionFactory(@NonNull final HostKeyVerifierFactory hostKeyVerifierFactory) { if (Files.notExists(hostKeyVerifierFactory.getKnownHostsFile().toPath())) { try { - Files.createDirectories(hostKeyVerifierFactory - .getKnownHostsFile() - .getParentFile() - .toPath()); + if (isWindows()) { + Files.createDirectories(hostKeyVerifierFactory + .getKnownHostsFile() + .getParentFile() + .toPath()); + } else { + Set ownerOnly = PosixFilePermissions.fromString("rwx------"); + FileAttribute> fileAttribute = + PosixFilePermissions.asFileAttribute(ownerOnly); + Files.createDirectories( + hostKeyVerifierFactory + .getKnownHostsFile() + .getParentFile() + .toPath(), + fileAttribute); + } Files.createFile(hostKeyVerifierFactory.getKnownHostsFile().toPath()); } catch (IOException e) { LOGGER.log(Level.SEVERE, "could not create known hosts file", e); @@ -3231,4 +3246,9 @@ public void close() { } } } + + /** inline ${@link hudson.Functions#isWindows()} to prevent a transient remote classloader issue */ + private static boolean isWindows() { + return File.pathSeparatorChar == ';'; + } } diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/verifier/KnownHostsFileVerifier.java b/src/main/java/org/jenkinsci/plugins/gitclient/verifier/KnownHostsFileVerifier.java index 1eec293e99..c0de3ff34a 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/verifier/KnownHostsFileVerifier.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/verifier/KnownHostsFileVerifier.java @@ -6,6 +6,10 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; import org.eclipse.jgit.transport.sshd.ServerKeyDatabase; @@ -29,19 +33,28 @@ public AbstractCliGitHostKeyVerifier forCliGit(TaskListener listener) { }; } + private void createKnownHostsFile(Path knowHostPath) throws IOException { + Path parent = knowHostPath.getParent(); + if (parent == null) { + throw new IllegalArgumentException("knowHostPath parent cannot be null"); + } + if (isWindows()) { + Files.createDirectories(parent); + } else { + Set ownerOnly = PosixFilePermissions.fromString("rwx------"); + FileAttribute> fileAttribute = PosixFilePermissions.asFileAttribute(ownerOnly); + Files.createDirectories(parent, fileAttribute); + } + Files.createFile(knowHostPath); + } + @Override public AbstractJGitHostKeyVerifier forJGit(TaskListener listener) { Path knowHostPath = getKnownHostsFile().toPath(); if (Files.notExists(knowHostPath)) { try { logHint(listener); - Path parent = knowHostPath.getParent(); - if (parent != null) { - Files.createDirectories(parent); - Files.createFile(knowHostPath); - } else { - throw new IllegalArgumentException("knowHostPath parent cannot be null"); - } + createKnownHostsFile(knowHostPath); } catch (IOException e) { LOGGER.log(Level.WARNING, e, () -> "Could not load known hosts."); } @@ -76,4 +89,9 @@ private void logHint(TaskListener listener) { "Known hosts file {0} not found, but verifying host keys with known hosts file", new Object[] {SshHostKeyVerificationStrategy.KNOWN_HOSTS_DEFAULT}); } + + /** inline ${@link hudson.Functions#isWindows()} to prevent a transient remote classloader issue */ + private static boolean isWindows() { + return File.pathSeparatorChar == ';'; + } }