Skip to content

Commit eb68868

Browse files
author
Boris Yao
committed
SECURITY-3342
1 parent 20c48ed commit eb68868

File tree

4 files changed

+146
-1
lines changed

4 files changed

+146
-1
lines changed

‎pom.xml‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,24 @@
5757
<groupId>org.jenkins-ci.modules</groupId>
5858
<artifactId>sshd</artifactId>
5959
</dependency>
60-
6160
<dependency>
6261
<groupId>org.jenkins-ci.plugins</groupId>
6362
<artifactId>git-client</artifactId>
6463
</dependency>
6564

65+
<dependency>
66+
<groupId>org.eclipse.jgit</groupId>
67+
<artifactId>org.eclipse.jgit.ssh.apache</artifactId>
68+
<version>6.9.0.202403050737-r</version>
69+
<scope>test</scope>
70+
</dependency>
71+
<dependency>
72+
<groupId>org.jenkins-ci.plugins</groupId>
73+
<artifactId>git-userContent</artifactId>
74+
<version>1.4</version>
75+
<scope>test</scope>
76+
</dependency>
77+
6678
</dependencies>
6779

6880
<repositories>

‎src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ protected FileBackedHttpGitRepository(File workspace) {
5050

5151
@Override
5252
public Repository openRepository() throws IOException {
53+
checkPullPermission();
5354
Repository r = new FileRepositoryBuilder().setWorkTree(workspace).build();
5455

5556
// if the repository doesn't exist, create it

‎src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ public HttpGitRepository() {
7575
*/
7676
public abstract UploadPack createUploadPack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException;
7777

78+
/**
79+
* to make sure the user has the permission to pull.
80+
*/
81+
public void checkPullPermission() {
82+
Jenkins.getInstance().checkPermission(Jenkins.READ);
83+
}
84+
7885
protected GitServlet init() {
7986
GitServlet g = new GitServlet();
8087
g.setRepositoryResolver(new org.eclipse.jgit.transport.resolver.RepositoryResolver<HttpServletRequest>() {

‎src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package org.jenkinsci.plugins.gitserver.ssh;
2+
3+
import hudson.Functions;
4+
import java.net.InetSocketAddress;
5+
import java.security.PublicKey;
6+
import java.util.Collections;
7+
import java.util.List;
8+
import jenkins.model.Jenkins;
9+
import org.eclipse.jgit.api.CloneCommand;
10+
import org.eclipse.jgit.api.Git;
11+
import org.eclipse.jgit.transport.CredentialsProvider;
12+
import org.eclipse.jgit.transport.SshSessionFactory;
13+
import org.eclipse.jgit.transport.sshd.ServerKeyDatabase;
14+
import org.eclipse.jgit.transport.sshd.SshdSessionFactory;
15+
import org.jenkinsci.main.modules.cli.auth.ssh.PublicKeySignatureWriter;
16+
import org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl;
17+
import org.jenkinsci.main.modules.sshd.SSHD;
18+
import org.junit.Before;
19+
import org.junit.Rule;
20+
import org.junit.Test;
21+
import org.junit.rules.TemporaryFolder;
22+
import org.jvnet.hudson.test.Issue;
23+
import org.jvnet.hudson.test.JenkinsRule;
24+
import org.jvnet.hudson.test.MockAuthorizationStrategy;
25+
26+
import java.io.File;
27+
import java.io.IOException;
28+
import java.security.KeyPair;
29+
import java.security.KeyPairGenerator;
30+
import java.security.NoSuchAlgorithmException;
31+
32+
import static org.junit.Assert.assertFalse;
33+
import static org.junit.Assert.assertTrue;
34+
import static org.junit.Assume.assumeFalse;
35+
36+
public class Security3342Test {
37+
38+
@Rule
39+
public JenkinsRule j = new JenkinsRule();
40+
41+
@Rule
42+
public TemporaryFolder tmp = new TemporaryFolder();
43+
44+
@Before
45+
public void setUp() {
46+
assumeFalse(Functions.isWindows());
47+
}
48+
49+
@Test
50+
@Issue("SECURITY-3342")
51+
public void openRepositoryPermissionCheckTest() throws Exception {
52+
53+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
54+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().to("tester"));
55+
hudson.model.User tester = hudson.model.User.getOrCreateByIdOrFullName("tester");
56+
KeyPair keyPair = generateKeys(tester);
57+
j.jenkins.save();
58+
59+
// Fixed ssh port for Jenkins ssh server
60+
SSHD server = SSHD.get();
61+
server.setPort(2222);
62+
63+
final SshdSessionFactory instance = new SshdSessionFactory() {
64+
@Override
65+
protected Iterable<KeyPair> getDefaultKeys(File sshDir) {
66+
return List.of(keyPair);
67+
}
68+
69+
@Override
70+
protected ServerKeyDatabase getServerKeyDatabase(File homeDir, File sshDir) {
71+
return new ServerKeyDatabase() {
72+
@Override
73+
public List<PublicKey> lookup(String connectAddress, InetSocketAddress remoteAddress, Configuration config) {
74+
return Collections.emptyList();
75+
}
76+
77+
@Override
78+
public boolean accept(String connectAddress, InetSocketAddress remoteAddress, PublicKey serverKey, Configuration config, CredentialsProvider provider) {
79+
return true;
80+
}
81+
};
82+
}
83+
};
84+
SshSessionFactory.setInstance(instance);
85+
86+
CloneCommand clone = Git.cloneRepository();
87+
clone.setURI("ssh://tester@localhost:2222/userContent.git");
88+
File dir1 = tmp.newFolder();
89+
clone.setDirectory(dir1);
90+
91+
// Do the git clone for a user with Jenkins.READ permission
92+
Git gitClone1 = clone.call();
93+
File gitDir1 = new File(dir1, ".git");
94+
assertTrue(".git directory exist, clone operation succeed", gitDir1.exists());
95+
96+
// Do the git clone for a user without Jenkins.READ permission
97+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy());
98+
j.jenkins.save();
99+
100+
File dir2 = tmp.newFolder();
101+
clone.setDirectory(dir2);
102+
103+
try {
104+
Git gitClone2 = clone.call();
105+
} catch (Exception e) {
106+
// Verify that the expected exception was caught with the correct message
107+
assertTrue(e.getCause() != null && e.getCause().getMessage().contains("hudson.security.AccessDeniedException3: tester is missing the Overall/Read permission"));
108+
}
109+
// Verify that the .git directory is not created
110+
File gitDir2 = new File(dir2, ".git");
111+
assertFalse(".git directory exist, clone operation succeed", gitDir2.exists());
112+
113+
}
114+
115+
private static KeyPair generateKeys(hudson.model.User user) throws NoSuchAlgorithmException, IOException {
116+
// I'd prefer to generate Ed25519 keys here, but the API is too awkward currently
117+
// ECDSA keys would be even more awkward as we'd need a copy of the curve parameters
118+
KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");
119+
generator.initialize(2048);
120+
KeyPair keyPair = generator.generateKeyPair();
121+
String encodedPublicKey = "ssh-rsa " + new PublicKeySignatureWriter().asString(keyPair.getPublic());
122+
user.addProperty(new UserPropertyImpl(encodedPublicKey));
123+
return keyPair;
124+
}
125+
}

0 commit comments

Comments
 (0)