Skip to content

Commit e80554d

Browse files
nfalco79nfalco79
committed
[JENKINS-75676] Client certificate credentials not handled properly
Fix using a library that allow to hotswap client certificates for an existing SSLContext.
1 parent 511b1ff commit e80554d

File tree

3 files changed

+116
-28
lines changed

3 files changed

+116
-28
lines changed

‎pom.xml

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
<revision>936.4.3</revision>
2828
<changelist>-SNAPSHOT</changelist>
2929
<gitHubRepo>jenkinsci/bitbucket-branch-source-plugin</gitHubRepo>
30-
<jenkins.baseline>2.479</jenkins.baseline>
31-
<jenkins.version>${jenkins.baseline}.3</jenkins.version>
30+
<jenkins.baseline>2.504</jenkins.baseline>
31+
<jenkins.version>${jenkins.baseline}.1</jenkins.version>
3232
<hpi.compatibleSinceVersion>936.0.0</hpi.compatibleSinceVersion>
3333
<!-- Jenkins.MANAGE is still in Beta -->
3434
<useBeta>true</useBeta>
@@ -81,6 +81,17 @@
8181
<groupId>io.jenkins.plugins</groupId>
8282
<artifactId>apache-httpcomponents-client-5-api</artifactId>
8383
</dependency>
84+
<dependency>
85+
<groupId>io.github.hakky54</groupId>
86+
<artifactId>sslcontext-kickstart-for-apache5</artifactId>
87+
<version>9.1.0</version>
88+
<exclusions>
89+
<exclusion>
90+
<groupId>org.slf4j</groupId>
91+
<artifactId>slf4j-api</artifactId>
92+
</exclusion>
93+
</exclusions>
94+
</dependency>
8495
<dependency>
8596
<groupId>io.jenkins.plugins</groupId>
8697
<artifactId>commons-lang3-api</artifactId>

‎src/main/java/com/cloudbees/jenkins/plugins/bitbucket/impl/credentials/BitbucketClientCertificateAuthenticator.java

Lines changed: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,34 @@
2525
package com.cloudbees.jenkins.plugins.bitbucket.impl.credentials;
2626

2727
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketAuthenticator;
28-
import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketException;
29-
import com.cloudbees.jenkins.plugins.bitbucket.impl.client.BitbucketTlsSocketStrategy;
3028
import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials;
3129
import hudson.util.Secret;
32-
import java.security.KeyManagementException;
30+
import java.net.URI;
31+
import java.net.URISyntaxException;
3332
import java.security.KeyStore;
34-
import java.security.KeyStoreException;
35-
import java.security.NoSuchAlgorithmException;
36-
import java.security.UnrecoverableKeyException;
37-
import javax.net.ssl.SSLContext;
38-
import org.apache.hc.client5.http.protocol.HttpClientContext;
33+
import java.util.ArrayList;
34+
import java.util.List;
35+
import java.util.Map;
36+
import java.util.logging.Logger;
37+
import javax.net.ssl.X509ExtendedKeyManager;
38+
import nl.altindag.ssl.SSLFactory;
39+
import nl.altindag.ssl.keymanager.AggregatedX509ExtendedKeyManager;
40+
import nl.altindag.ssl.keymanager.DummyX509ExtendedKeyManager;
41+
import nl.altindag.ssl.keymanager.HotSwappableX509ExtendedKeyManager;
42+
import nl.altindag.ssl.keymanager.LoggingX509ExtendedKeyManager;
43+
import nl.altindag.ssl.keymanager.X509KeyManagerWrapper;
44+
import nl.altindag.ssl.util.KeyManagerUtils;
45+
import nl.altindag.ssl.util.KeyStoreUtils;
3946
import org.apache.hc.core5.http.HttpHost;
40-
import org.apache.hc.core5.ssl.SSLContextBuilder;
41-
import org.apache.hc.core5.ssl.SSLContexts;
47+
import org.kohsuke.accmod.Restricted;
48+
import org.kohsuke.accmod.restrictions.NoExternalUse;
4249

4350
/**
4451
* Authenticates against Bitbucket using a TLS client certificate
4552
*/
4653
public class BitbucketClientCertificateAuthenticator implements BitbucketAuthenticator {
54+
private static final Logger logger = Logger.getLogger(BitbucketClientCertificateAuthenticator.class.getName());
55+
4756
private final String credentialsId;
4857
private final KeyStore keyStore;
4958
private final Secret password;
@@ -55,23 +64,64 @@ public BitbucketClientCertificateAuthenticator(StandardCertificateCredentials cr
5564
}
5665

5766
/**
58-
* Sets the SSLContext for the builder to one that will connect with the selected certificate.
59-
* @param context The client builder context
67+
* Sets the SSLContext for the builder to one that will connect with the
68+
* selected certificate.
69+
*
70+
* @param sslFactory The client SSL context configured in the connection
71+
* pool
6072
* @param host the target host name
6173
*/
62-
@Override
63-
public void configureContext(HttpClientContext context, HttpHost host) {
64-
try {
65-
context.setAttribute(BitbucketTlsSocketStrategy.SOCKET_FACTORY_REGISTRY, buildSSLContext()); // override SSL registry for this context
66-
} catch (NoSuchAlgorithmException | UnrecoverableKeyException | KeyStoreException | KeyManagementException e) {
67-
throw new BitbucketException("Failed to set up SSL context from provided client certificate", e);
68-
}
74+
@Restricted(NoExternalUse.class)
75+
public synchronized /*required to avoid combine the same identity material twice */ void configureContext(SSLFactory sslFactory, HttpHost host) {
76+
sslFactory.getKeyManager().ifPresent(baseKeyManager -> {
77+
List<X509ExtendedKeyManager> keyManagers = new ArrayList<>();
78+
79+
AggregatedX509ExtendedKeyManager aggregate = unwrap(baseKeyManager);
80+
Map<String, List<URI>> routes = aggregate.getIdentityRoute();
81+
82+
for (String alias : KeyStoreUtils.getAliases(keyStore)) {
83+
// check if given route has been already added to the SSL context
84+
if (!routes.containsKey(alias)) {
85+
try {
86+
URI hostURI = new URI(host.toURI());
87+
routes.put(alias, new ArrayList<>(List.of(hostURI)));
88+
} catch (URISyntaxException e) {
89+
logger.severe("Invalid host " + host);
90+
}
91+
keyManagers.add(KeyManagerUtils.createKeyManager(keyStore, Secret.toString(password).toCharArray()));
92+
logger.info(() -> "Add new identity material (keyStore) " + alias + " to the SSLContext.");
93+
}
94+
}
95+
96+
if (!keyManagers.isEmpty()) {
97+
// create an aggregate keyManager with new identity material plus existing contributed
98+
// from other configured client certificate client
99+
X509ExtendedKeyManager combined = KeyManagerUtils.keyManagerBuilder()
100+
.withKeyManagers(aggregate.getInnerKeyManagers())
101+
.withKeyManagers(keyManagers)
102+
.withIdentityRoute(routes)
103+
.build();
104+
// swap identity materials and reuse existing http client
105+
KeyManagerUtils.swapKeyManager(baseKeyManager, combined);
106+
}
107+
});
69108
}
70109

71-
private SSLContext buildSSLContext() throws NoSuchAlgorithmException, KeyStoreException, UnrecoverableKeyException, KeyManagementException {
72-
SSLContextBuilder contextBuilder = SSLContexts.custom();
73-
contextBuilder.loadKeyMaterial(keyStore, Secret.toString(password).toCharArray());
74-
return contextBuilder.build();
110+
111+
private AggregatedX509ExtendedKeyManager unwrap(X509ExtendedKeyManager keyManager) {
112+
if (keyManager instanceof AggregatedX509ExtendedKeyManager aggregate) {
113+
return aggregate;
114+
} else if (keyManager instanceof HotSwappableX509ExtendedKeyManager swappable) {
115+
return unwrap(swappable.getInnerKeyManager());
116+
} else if (keyManager instanceof LoggingX509ExtendedKeyManager logger) {
117+
return unwrap(logger.getInnerKeyManager());
118+
} else if (keyManager instanceof X509KeyManagerWrapper wrapper) {
119+
return unwrap((X509ExtendedKeyManager) wrapper.getInnerKeyManager());
120+
} else if (keyManager instanceof DummyX509ExtendedKeyManager) {
121+
return new AggregatedX509ExtendedKeyManager(new ArrayList<>());
122+
} else {
123+
return new AggregatedX509ExtendedKeyManager(List.of(keyManager));
124+
}
75125
}
76126

77127
@Override

‎src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,15 @@
8888
import jenkins.scm.api.SCMFile;
8989
import jenkins.scm.api.SCMFile.Type;
9090
import jenkins.scm.impl.avatars.AvatarImage;
91+
import nl.altindag.ssl.SSLFactory;
92+
import nl.altindag.ssl.SSLFactory.Builder;
9193
import org.apache.commons.codec.digest.DigestUtils;
9294
import org.apache.commons.io.IOUtils;
9395
import org.apache.commons.lang3.StringUtils;
9496
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
97+
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
9598
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
99+
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
96100
import org.apache.hc.core5.http.HttpHost;
97101
import org.apache.hc.core5.http.HttpStatus;
98102
import org.apache.hc.core5.http.message.BasicNameValuePair;
@@ -140,10 +144,24 @@ public class BitbucketServerAPIClient extends AbstractBitbucketApi implements Bi
140144
private static final String API_MIRRORS_PATH = "/rest/mirroring/1.0/mirrorServers";
141145
private static final Integer DEFAULT_PAGE_LIMIT = 200;
142146

147+
private static Builder sslFactoryBuilder() {
148+
Builder builder = SSLFactory.builder()
149+
.withSystemTrustMaterial()
150+
.withDefaultTrustMaterial()
151+
// TODO remove .withTrustMaterial(Paths.get("D:\\Download\\JENKINS-75676\\jenkins\\trustStore.jks"), "JENKINS-75676".toCharArray())
152+
.withDummyIdentityMaterial()
153+
.withSwappableIdentityMaterial();
154+
if (System.getProperty("javax.net.ssl.trustStore") != null) {
155+
builder.withSystemPropertyDerivedTrustMaterial();
156+
}
157+
return builder;
158+
}
159+
160+
private static final SSLFactory sslFactory = sslFactoryBuilder().build();
143161
private static final HttpClientConnectionManager connectionManager = connectionManagerBuilder()
144-
.setMaxConnPerRoute(20)
145-
.setMaxConnTotal(40 /* should be 20 * number of server instances */)
146-
.setTlsSocketStrategy(new BitbucketTlsSocketStrategy())
162+
.setMaxConnPerRoute(1) // FIXME restore to 20
163+
.setMaxConnTotal(1 /* should be 20 * number of server instances */) // FIXME restore to 40
164+
.setTlsSocketStrategy(new DefaultClientTlsStrategy(sslFactory.getSslContext()))
147165
.build();
148166

149167
/**
@@ -162,6 +180,7 @@ public class BitbucketServerAPIClient extends AbstractBitbucketApi implements Bi
162180
private final BitbucketServerWebhookImplementation webhookImplementation;
163181
private final CloseableHttpClient client;
164182

183+
165184
public BitbucketServerAPIClient(@NonNull String baseURL, @NonNull String owner, @CheckForNull String repositoryName,
166185
@CheckForNull BitbucketAuthenticator authenticator, boolean userCentric) {
167186
this(baseURL, owner, repositoryName, authenticator, userCentric, BitbucketServerEndpoint.findWebhookImplementation(baseURL));
@@ -182,6 +201,14 @@ public BitbucketServerAPIClient(@NonNull String baseURL, @NonNull String owner,
182201
this.client = setupClientBuilder().build();
183202
}
184203

204+
@Override
205+
protected HttpClientBuilder setupClientBuilder() {
206+
if (getAuthenticator() instanceof BitbucketClientCertificateAuthenticator certificateAuthenticator) {
207+
certificateAuthenticator.configureContext(sslFactory, getHost());
208+
}
209+
return super.setupClientBuilder();
210+
}
211+
185212
@Override
186213
protected boolean isSupportedAuthenticator(@CheckForNull BitbucketAuthenticator authenticator) {
187214
return authenticator == null

0 commit comments

Comments
 (0)