Skip to content

Commit 845ee9b

Browse files
ketanzabil
ketan
authored and
zabil
committed
Tweak the agent registration protocol a bit (#2464) (#2558)
* Tweak the agent registration protocol a bit (#2464) * The server responds with a 202 (Accepted) and an empty JSON object `{}` as long as the registration is pending. * The server responds with a 200 (OK) and the old JSON object containing the private key and the cert chain. * Move serializaiton/deserialization of Registration into another class This prevents having to add GSON as a dependency in go.jar/lib
1 parent ff0bfdc commit 845ee9b

File tree

5 files changed

+164
-100
lines changed

5 files changed

+164
-100
lines changed

‎agent/src/com/thoughtworks/go/agent/service/SslInfrastructureService.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.thoughtworks.go.config.GuidService;
2424
import com.thoughtworks.go.security.KeyStoreManager;
2525
import com.thoughtworks.go.security.Registration;
26+
import com.thoughtworks.go.security.RegistrationJSONizer;
2627
import com.thoughtworks.go.server.service.AgentRuntimeInfo;
2728
import com.thoughtworks.go.util.SystemEnvironment;
2829
import com.thoughtworks.go.util.SystemUtil;
@@ -96,8 +97,8 @@ public boolean isRegistered() {
9697

9798
private void register(AgentAutoRegistrationProperties agentAutoRegistrationProperties) throws Exception {
9899
String hostName = SystemUtil.getLocalhostNameOrRandomNameIfNotFound();
99-
Registration keyEntry = null;
100-
while (keyEntry == null || keyEntry.getChain().length == 0) {
100+
Registration keyEntry = Registration.createNullPrivateKeyEntry();
101+
while (!keyEntry.isValid()) {
101102
try {
102103
keyEntry = remoteRegistrationRequester.requestRegistration(hostName, agentAutoRegistrationProperties);
103104
} catch (Exception e) {
@@ -183,7 +184,7 @@ protected Registration requestRegistration(String agentHostName, AgentAutoRegist
183184
}
184185

185186
protected Registration readResponse(InputStream is) throws IOException, ClassNotFoundException {
186-
return Registration.fromJson(IOUtils.toString(is));
187+
return RegistrationJSONizer.fromJson(IOUtils.toString(is));
187188
}
188189
}
189190
}

‎common/src/com/thoughtworks/go/security/Registration.java‎

Lines changed: 9 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -16,52 +16,21 @@
1616

1717
package com.thoughtworks.go.security;
1818

19-
import com.google.gson.Gson;
20-
import org.apache.commons.io.IOUtils;
21-
import org.bouncycastle.util.io.pem.PemObject;
22-
import org.bouncycastle.util.io.pem.PemReader;
23-
import org.bouncycastle.util.io.pem.PemWriter;
24-
25-
import java.io.*;
26-
import java.security.*;
19+
import java.io.Serializable;
20+
import java.security.KeyStore;
21+
import java.security.PrivateKey;
22+
import java.security.PublicKey;
2723
import java.security.cert.Certificate;
28-
import java.security.cert.*;
29-
import java.security.spec.InvalidKeySpecException;
30-
import java.security.spec.PKCS8EncodedKeySpec;
31-
import java.util.*;
32-
33-
import static com.thoughtworks.go.util.ExceptionUtils.bomb;
24+
import java.security.cert.X509Certificate;
25+
import java.util.Date;
3426

3527
public class Registration implements Serializable {
3628

37-
public static Registration fromJson(String json) {
38-
Map map = new Gson().fromJson(json, Map.class);
39-
List<Certificate> chain = new ArrayList<>();
40-
try {
41-
PemReader reader = new PemReader(new StringReader((String) map.get("agentPrivateKey")));
42-
KeyFactory kf = KeyFactory.getInstance("RSA");
43-
PKCS8EncodedKeySpec spec = new PKCS8EncodedKeySpec(reader.readPemObject().getContent());
44-
PrivateKey privateKey = kf.generatePrivate(spec);
45-
String agentCertificate = (String) map.get("agentCertificate");
46-
PemReader certReader = new PemReader(new StringReader(agentCertificate));
47-
while (true) {
48-
PemObject obj = certReader.readPemObject();
49-
if (obj == null) {
50-
break;
51-
}
52-
chain.add(CertificateFactory.getInstance("X.509").generateCertificate(new ByteArrayInputStream(obj.getContent())));
53-
}
54-
return new Registration(privateKey, chain.toArray(new Certificate[chain.size()]));
55-
} catch (IOException | NoSuchAlgorithmException | CertificateException | InvalidKeySpecException e) {
56-
throw bomb(e);
57-
}
58-
}
59-
6029
private final PrivateKey privateKey;
6130
private final Certificate[] chain;
6231

6332
public static Registration createNullPrivateKeyEntry() {
64-
return new Registration(emptyPrivateKey());
33+
return new Registration(null);
6534
}
6635

6736
public Registration(PrivateKey privateKey, Certificate... chain) {
@@ -89,53 +58,12 @@ public Date getCertificateNotBeforeDate() {
8958
return getFirstCertificate().getNotBefore();
9059
}
9160

92-
private static PrivateKey emptyPrivateKey() {
93-
return new PrivateKey() {
94-
public String getAlgorithm() {
95-
return null;
96-
}
97-
98-
public String getFormat() {
99-
return null;
100-
}
101-
102-
public byte[] getEncoded() {
103-
return new byte[0];
104-
}
105-
};
106-
}
107-
10861
public KeyStore.PrivateKeyEntry asKeyStoreEntry() {
10962
return new KeyStore.PrivateKeyEntry(privateKey, chain);
11063
}
11164

112-
public String toJson() {
113-
Map<String, Object> ret = new HashMap<>();
114-
ret.put("agentPrivateKey", serialize("RSA PRIVATE KEY", privateKey.getEncoded()));
115-
StringBuilder builder = new StringBuilder();
116-
for (Certificate c : chain) {
117-
try {
118-
builder.append(serialize("CERTIFICATE", c.getEncoded()));
119-
} catch (CertificateEncodingException e) {
120-
throw bomb(e);
121-
}
122-
}
123-
ret.put("agentCertificate", builder.toString());
124-
return new Gson().toJson(ret);
125-
}
126-
127-
private String serialize(String type, byte[] data) {
128-
PemObject obj = new PemObject(type, data);
129-
StringWriter out = new StringWriter();
130-
PemWriter writer = new PemWriter(out);
131-
try {
132-
writer.writeObject(obj);
133-
} catch (IOException e) {
134-
throw bomb(e);
135-
} finally {
136-
IOUtils.closeQuietly(writer);
137-
}
138-
return out.toString();
65+
public boolean isValid() {
66+
return privateKey != null && chain != null && chain.length > 0;
13967
}
14068

14169
}

‎common/src/com/thoughtworks/go/security/RegistrationJSONizer.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Copyright 2016 ThoughtWorks, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.thoughtworks.go.security;
18+
19+
import com.google.gson.Gson;
20+
import org.apache.commons.io.IOUtils;
21+
import org.bouncycastle.util.io.pem.PemObject;
22+
import org.bouncycastle.util.io.pem.PemReader;
23+
import org.bouncycastle.util.io.pem.PemWriter;
24+
25+
import java.io.ByteArrayInputStream;
26+
import java.io.IOException;
27+
import java.io.StringReader;
28+
import java.io.StringWriter;
29+
import java.security.KeyFactory;
30+
import java.security.NoSuchAlgorithmException;
31+
import java.security.PrivateKey;
32+
import java.security.cert.Certificate;
33+
import java.security.cert.CertificateEncodingException;
34+
import java.security.cert.CertificateException;
35+
import java.security.cert.CertificateFactory;
36+
import java.security.spec.InvalidKeySpecException;
37+
import java.security.spec.PKCS8EncodedKeySpec;
38+
import java.util.ArrayList;
39+
import java.util.HashMap;
40+
import java.util.List;
41+
import java.util.Map;
42+
43+
import static com.thoughtworks.go.util.ExceptionUtils.bomb;
44+
45+
public class RegistrationJSONizer {
46+
private static final Gson GSON = new Gson();
47+
48+
public static Registration fromJson(String json) {
49+
Map map = GSON.fromJson(json, Map.class);
50+
51+
if (map.isEmpty()) {
52+
return Registration.createNullPrivateKeyEntry();
53+
}
54+
55+
List<Certificate> chain = new ArrayList<>();
56+
try {
57+
PemReader reader = new PemReader(new StringReader((String) map.get("agentPrivateKey")));
58+
KeyFactory kf = KeyFactory.getInstance("RSA");
59+
PKCS8EncodedKeySpec spec = new PKCS8EncodedKeySpec(reader.readPemObject().getContent());
60+
PrivateKey privateKey = kf.generatePrivate(spec);
61+
String agentCertificate = (String) map.get("agentCertificate");
62+
PemReader certReader = new PemReader(new StringReader(agentCertificate));
63+
while (true) {
64+
PemObject obj = certReader.readPemObject();
65+
if (obj == null) {
66+
break;
67+
}
68+
chain.add(CertificateFactory.getInstance("X.509").generateCertificate(new ByteArrayInputStream(obj.getContent())));
69+
}
70+
return new Registration(privateKey, chain.toArray(new Certificate[chain.size()]));
71+
} catch (IOException | NoSuchAlgorithmException | CertificateException | InvalidKeySpecException e) {
72+
throw bomb(e);
73+
}
74+
}
75+
76+
77+
public static String toJson(Registration registration) {
78+
Map<String, Object> ret = new HashMap<>();
79+
80+
if (registration.isValid()) {
81+
ret.put("agentPrivateKey", serialize("RSA PRIVATE KEY", registration.getPrivateKey().getEncoded()));
82+
StringBuilder builder = new StringBuilder();
83+
for (Certificate c : registration.getChain()) {
84+
try {
85+
builder.append(serialize("CERTIFICATE", c.getEncoded()));
86+
} catch (CertificateEncodingException e) {
87+
throw bomb(e);
88+
}
89+
}
90+
ret.put("agentCertificate", builder.toString());
91+
}
92+
93+
return GSON.toJson(ret);
94+
}
95+
96+
private static String serialize(String type, byte[] data) {
97+
PemObject obj = new PemObject(type, data);
98+
StringWriter out = new StringWriter();
99+
PemWriter writer = new PemWriter(out);
100+
try {
101+
writer.writeObject(obj);
102+
} catch (IOException e) {
103+
throw bomb(e);
104+
} finally {
105+
IOUtils.closeQuietly(writer);
106+
}
107+
return out.toString();
108+
}
109+
}

‎common/test/unit/com/thoughtworks/go/security/RegistrationTest.java‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,25 @@
1616

1717
package com.thoughtworks.go.security;
1818

19-
import com.thoughtworks.go.util.TestFileUtil;
19+
import org.apache.commons.lang.builder.EqualsBuilder;
20+
import org.junit.Rule;
2021
import org.junit.Test;
22+
import org.junit.rules.TemporaryFolder;
2123

2224
import java.io.File;
25+
import java.io.IOException;
2326

2427
import static org.hamcrest.Matchers.is;
25-
import static org.junit.Assert.assertThat;
28+
import static org.junit.Assert.*;
2629

2730
public class RegistrationTest {
28-
private static String authorityKeystorePath = "tempAuthorityKeystore";
31+
@Rule
32+
public TemporaryFolder tmpFolder = new TemporaryFolder();
2933

3034
@Test
31-
public void decodeFromJson() {
35+
public void decodeFromJson() throws IOException {
3236
Registration origin = createRegistration();
33-
Registration reg = Registration.fromJson(origin.toJson());
37+
Registration reg = RegistrationJSONizer.fromJson(RegistrationJSONizer.toJson(origin));
3438
assertThat(reg.getPrivateKey(), is(origin.getPrivateKey()));
3539
assertThat(reg.getPublicKey(), is(origin.getPublicKey()));
3640
assertThat(reg.getChain(), is(origin.getChain()));
@@ -39,8 +43,31 @@ public void decodeFromJson() {
3943
assertThat(reg.getChain().length, is(3));
4044
}
4145

42-
public static Registration createRegistration() {
43-
File tempKeystoreFile = TestFileUtil.createUniqueTempFile(authorityKeystorePath);
46+
47+
@Test
48+
public void shouldBeValidWhenPrivateKeyAndChainIsPresent() throws Exception {
49+
assertFalse(Registration.createNullPrivateKeyEntry().isValid());
50+
assertFalse(new Registration(null, null).isValid());
51+
assertFalse(new Registration(null).isValid());
52+
53+
assertTrue(createRegistration().isValid());
54+
}
55+
56+
@Test
57+
public void registrationFromEmptyJSONShouldBeInvalid() throws Exception {
58+
assertFalse(RegistrationJSONizer.fromJson("{}").isValid());
59+
}
60+
61+
@Test
62+
public void shouldEncodeDecodeEmptyRegistration() throws Exception {
63+
Registration toSerialize = Registration.createNullPrivateKeyEntry();
64+
Registration deserialized = RegistrationJSONizer.fromJson(RegistrationJSONizer.toJson(toSerialize));
65+
66+
assertTrue(EqualsBuilder.reflectionEquals(toSerialize, deserialized));
67+
}
68+
69+
private Registration createRegistration() throws IOException {
70+
File tempKeystoreFile = tmpFolder.newFile();
4471
X509CertificateGenerator certificateGenerator = new X509CertificateGenerator();
4572
certificateGenerator.createAndStoreCACertificates(tempKeystoreFile);
4673
return certificateGenerator.createAgentCertificate(tempKeystoreFile, "blah");

‎server/src/com/thoughtworks/go/server/controller/AgentRegistrationController.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import com.thoughtworks.go.domain.ConfigErrors;
2626
import com.thoughtworks.go.plugin.infra.commons.PluginsZip;
2727
import com.thoughtworks.go.security.Registration;
28-
import com.thoughtworks.go.server.domain.Username;
28+
import com.thoughtworks.go.security.RegistrationJSONizer;
2929
import com.thoughtworks.go.server.service.*;
3030
import com.thoughtworks.go.server.service.result.HttpOperationResult;
3131
import com.thoughtworks.go.util.SystemEnvironment;
@@ -40,7 +40,6 @@
4040
import org.springframework.web.servlet.ModelAndView;
4141
import org.springframework.web.servlet.View;
4242

43-
import javax.servlet.ServletOutputStream;
4443
import javax.servlet.http.HttpServletRequest;
4544
import javax.servlet.http.HttpServletResponse;
4645
import java.io.*;
@@ -251,20 +250,20 @@ public ModelAndView agentRequest(@RequestParam("hostname") String hostname,
251250
LOG.error("Error occured during agent registration process: ", e);
252251
}
253252

254-
final Registration anotherCopy = keyEntry;
253+
return render(keyEntry);
254+
}
255+
256+
private ModelAndView render(final Registration registration) {
255257
return new ModelAndView(new View() {
256258
public String getContentType() {
257259
return "application/json";
258260
}
259261

260262
public void render(Map model, HttpServletRequest request, HttpServletResponse response) throws IOException {
261-
ServletOutputStream servletOutputStream = null;
262-
try {
263-
servletOutputStream = response.getOutputStream();
264-
servletOutputStream.print(anotherCopy.toJson());
265-
} finally {
266-
IOUtils.closeQuietly(servletOutputStream);
263+
if (!registration.isValid()) {
264+
response.setStatus(HttpServletResponse.SC_ACCEPTED);
267265
}
266+
response.getWriter().print(RegistrationJSONizer.toJson(registration));
268267
}
269268
});
270269
}

0 commit comments

Comments
 (0)