Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-73305] Create .ssh directory with owner only permissions #1150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[JENKINS-73305] Create .ssh directory with owner only permissions #1150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a09e86f
[JENKINS-73305] Create .ssh directory with owner only permissions
MarkEWaite Jun 15, 2024
05e9746
Remove unused import
MarkEWaite Oct 29, 2024
d68cf59
Merge branch 'master' into create-ssh-dir-with-better-perms
MarkEWaite Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
import java.nio.charset.StandardCharsets;
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.security.GeneralSecurityException;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -203,10 +206,22 @@
public SshdSessionFactory buildSshdSessionFactory(@NonNull final HostKeyVerifierFactory hostKeyVerifierFactory) {
if (Files.notExists(hostKeyVerifierFactory.getKnownHostsFile().toPath())) {
try {
Files.createDirectories(hostKeyVerifierFactory
.getKnownHostsFile()
.getParentFile()
.toPath());
if (isWindows()) {

Check warning on line 209 in src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 209 is only partially covered, one branch is missing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here be dragons.

Don't assume that POSIX file systems == non windows systems.

You can be on Linux and have a non POSIX FS (and the inverse).

And there was (and iirc) still is a bug where when you ask for the filesystem for a path you get the default even if it should be different!

This the only reliable way to do this is to actually try, catch the exception and do a fallback unless that big is fixed.

Files.createDirectories(hostKeyVerifierFactory
.getKnownHostsFile()
.getParentFile()
.toPath());
} else {
Set<PosixFilePermission> ownerOnly = PosixFilePermissions.fromString("rwx------");
FileAttribute<Set<PosixFilePermission>> fileAttribute =
PosixFilePermissions.asFileAttribute(ownerOnly);
Files.createDirectories(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want createDirectory here (and possibly a different call to create any missing parents of this directory)?
(At least you don't want all directories up to the parent to be created with these perms do you?)

hostKeyVerifierFactory
.getKnownHostsFile()
.getParentFile()
.toPath(),
fileAttribute);
}
Files.createFile(hostKeyVerifierFactory.getKnownHostsFile().toPath());
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "could not create known hosts file", e);
Expand Down Expand Up @@ -3262,4 +3277,9 @@
}
}
}

/** inline ${@link hudson.Functions#isWindows()} to prevent a transient remote classloader issue */
private static boolean isWindows() {
return File.pathSeparatorChar == ';';

Check warning on line 3283 in src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 3283 is only partially covered, one branch is missing
}
}
32 changes: 25 additions & 7 deletions src/main/java/org/jenkinsci/plugins/gitclient/verifier/KnownHostsFileVerifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,54 +33,68 @@
};
}

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<PosixFilePermission> ownerOnly = PosixFilePermissions.fromString("rwx------");
FileAttribute<Set<PosixFilePermission>> 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.");
}
}
return new KnownHostsFileJGitHostKeyVerifier(listener, this);
}

public static class KnownHostsFileJGitHostKeyVerifier extends AbstractJGitHostKeyVerifier {

public KnownHostsFileJGitHostKeyVerifier(TaskListener listener, HostKeyVerifierFactory hostKeyVerifierFactory) {
super(listener, hostKeyVerifierFactory);
}

@Override
public ServerKeyDatabase.Configuration getServerKeyDatabaseConfiguration() {
return new AbstractJGitHostKeyVerifier.DefaultConfiguration(
this.getHostKeyVerifierFactory(),
() -> ServerKeyDatabase.Configuration.StrictHostKeyChecking.REQUIRE_MATCH);
}
}

private void logHint(TaskListener listener) {
listener.getLogger()
.println(
HyperlinkNote.encodeTo(
"https://plugins.jenkins.io/git-client/#plugin-content-ssh-host-key-verification",
"""
You're using 'Known hosts file' strategy to verify ssh host keys,\
but your known_hosts file does not exist, please go to \
'Manage Jenkins' -> 'Security' -> 'Git Host Key Verification Configuration' \
and configure host key verification.\
"""));
LOGGER.log(
Level.FINEST,
"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 == ';';

Check warning on line 98 in src/main/java/org/jenkinsci/plugins/gitclient/verifier/KnownHostsFileVerifier.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 37-98 are not covered by tests
}
}