Skip to content

Commit f2464e4

Browse files
SakshamsSakshams
committed
CLOUDSTACK-6864: UploadSSlCert API requires double encoding of URL params
1 parent 60638c1 commit f2464e4

2 files changed

Lines changed: 41 additions & 46 deletions

File tree

‎server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818

1919
import java.io.IOException;
2020
import java.io.StringReader;
21-
import java.io.UnsupportedEncodingException;
22-
import java.net.URLDecoder;
2321
import java.security.InvalidAlgorithmParameterException;
2422
import java.security.InvalidKeyException;
2523
import java.security.KeyPair;
@@ -53,18 +51,17 @@
5351
import javax.ejb.Local;
5452
import javax.inject.Inject;
5553

56-
import org.apache.commons.io.IOUtils;
57-
import org.apache.log4j.Logger;
58-
import org.bouncycastle.jce.provider.BouncyCastleProvider;
59-
import org.bouncycastle.openssl.PEMReader;
60-
import org.bouncycastle.openssl.PasswordFinder;
61-
6254
import org.apache.cloudstack.acl.SecurityChecker;
6355
import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd;
6456
import org.apache.cloudstack.api.command.user.loadbalancer.ListSslCertsCmd;
6557
import org.apache.cloudstack.api.command.user.loadbalancer.UploadSslCertCmd;
6658
import org.apache.cloudstack.api.response.SslCertResponse;
6759
import org.apache.cloudstack.context.CallContext;
60+
import org.apache.commons.io.IOUtils;
61+
import org.apache.log4j.Logger;
62+
import org.bouncycastle.jce.provider.BouncyCastleProvider;
63+
import org.bouncycastle.openssl.PEMReader;
64+
import org.bouncycastle.openssl.PasswordFinder;
6865

6966
import com.cloud.event.ActionEvent;
7067
import com.cloud.event.EventTypes;
@@ -109,10 +106,10 @@ public CertServiceImpl() {
109106
public SslCertResponse uploadSslCert(UploadSslCertCmd certCmd) {
110107
try {
111108

112-
String cert = URLDecoder.decode(certCmd.getCert(), "UTF-8");
113-
String key = URLDecoder.decode(certCmd.getKey(), "UTF-8");
109+
String cert = certCmd.getCert();
110+
String key = certCmd.getKey();
114111
String password = certCmd.getPassword();
115-
String chain = certCmd.getChain() == null ? null : URLDecoder.decode(certCmd.getChain(), "UTF-8");
112+
String chain = certCmd.getChain();
116113

117114
validate(cert, key, password, chain);
118115
s_logger.debug("Certificate Validation succeeded");
@@ -127,8 +124,8 @@ public SslCertResponse uploadSslCert(UploadSslCertCmd certCmd) {
127124

128125
return createCertResponse(certVO, null);
129126

130-
} catch (UnsupportedEncodingException e) {
131-
throw new CloudRuntimeException("Error decoding certificate data");
127+
} catch (Exception e) {
128+
throw new CloudRuntimeException("Error parsing certificate data " + e.getMessage());
132129
}
133130

134131
}
@@ -429,7 +426,7 @@ public Certificate parseCertificate(String cert) {
429426
try {
430427
return (Certificate)certPem.readObject();
431428
} catch (Exception e) {
432-
throw new InvalidParameterValueException("Invalid Certificate format. Expected X509 certificate");
429+
throw new InvalidParameterValueException("Invalid Certificate format. Expected X509 certificate. Failed due to " + e.getMessage());
433430
} finally {
434431
IOUtils.closeQuietly(certPem);
435432
}

‎server/test/org/apache/cloudstack/network/lb/CertServiceTest.java‎

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,19 @@
2727
import java.io.File;
2828
import java.io.IOException;
2929
import java.lang.reflect.Field;
30-
import java.net.URLEncoder;
3130
import java.util.ArrayList;
3231
import java.util.List;
3332
import java.util.UUID;
3433

34+
import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd;
35+
import org.apache.cloudstack.api.command.user.loadbalancer.UploadSslCertCmd;
36+
import org.apache.cloudstack.context.CallContext;
3537
import org.junit.After;
3638
import org.junit.Assume;
3739
import org.junit.Before;
3840
import org.junit.Test;
3941
import org.mockito.Mockito;
4042

41-
import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd;
42-
import org.apache.cloudstack.api.command.user.loadbalancer.UploadSslCertCmd;
43-
import org.apache.cloudstack.context.CallContext;
44-
4543
import com.cloud.network.dao.LoadBalancerCertMapDao;
4644
import com.cloud.network.dao.LoadBalancerCertMapVO;
4745
import com.cloud.network.dao.LoadBalancerVO;
@@ -101,9 +99,9 @@ public void runUploadSslCertWithCAChain() throws Exception {
10199
String chainFile = getClass().getResource("/certs/root_chain.crt").getFile();
102100
String password = "user";
103101

104-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
105-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
106-
String chain = URLEncoder.encode(readFileToString(new File(chainFile)), "UTF-8");
102+
String cert = readFileToString(new File(certFile));
103+
String key = readFileToString(new File(keyFile));
104+
String chain = readFileToString(new File(chainFile));
107105

108106
CertServiceImpl certService = new CertServiceImpl();
109107

@@ -153,8 +151,8 @@ public void runUploadSslCertSelfSignedWithPassword() throws Exception {
153151
String keyFile = getClass().getResource("/certs/rsa_self_signed_with_pwd.key").getFile();
154152
String password = "test";
155153

156-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
157-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
154+
String cert = readFileToString(new File(certFile));
155+
String key = readFileToString(new File(keyFile));
158156

159157
CertServiceImpl certService = new CertServiceImpl();
160158

@@ -199,8 +197,8 @@ public void runUploadSslCertSelfSignedNoPassword() throws Exception {
199197
String certFile = getClass().getResource("/certs/rsa_self_signed.crt").getFile();
200198
String keyFile = getClass().getResource("/certs/rsa_self_signed.key").getFile();
201199

202-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
203-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
200+
String cert = readFileToString(new File(certFile));
201+
String key = readFileToString(new File(keyFile));
204202

205203
CertServiceImpl certService = new CertServiceImpl();
206204

@@ -239,9 +237,9 @@ public void runUploadSslCertBadChain() throws IOException, IllegalAccessExceptio
239237
String chainFile = getClass().getResource("/certs/rsa_self_signed.crt").getFile();
240238
String password = "user";
241239

242-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
243-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
244-
String chain = URLEncoder.encode(readFileToString(new File(chainFile)), "UTF-8");
240+
String cert = readFileToString(new File(certFile));
241+
String key = readFileToString(new File(keyFile));
242+
String chain = readFileToString(new File(chainFile));
245243

246244
CertServiceImpl certService = new CertServiceImpl();
247245

@@ -291,9 +289,9 @@ public void runUploadSslCertNoRootCert() throws IOException, IllegalAccessExcept
291289
String chainFile = getClass().getResource("/certs/rsa_ca_signed2.crt").getFile();
292290
String password = "user";
293291

294-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
295-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
296-
String chain = URLEncoder.encode(readFileToString(new File(chainFile)), "UTF-8");
292+
String cert = readFileToString(new File(certFile));
293+
String key = readFileToString(new File(keyFile));
294+
String chain = readFileToString(new File(chainFile));
297295

298296
CertServiceImpl certService = new CertServiceImpl();
299297

@@ -343,8 +341,8 @@ public void runUploadSslCertNoChain() throws IOException, IllegalAccessException
343341
String keyFile = getClass().getResource("/certs/rsa_ca_signed.key").getFile();
344342
String password = "user";
345343

346-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
347-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
344+
String cert = readFileToString(new File(certFile));
345+
String key = readFileToString(new File(keyFile));
348346

349347
CertServiceImpl certService = new CertServiceImpl();
350348

@@ -388,8 +386,8 @@ public void runUploadSslCertBadPassword() throws IOException, IllegalAccessExcep
388386
String keyFile = getClass().getResource("/certs/rsa_self_signed_with_pwd.key").getFile();
389387
String password = "bad_password";
390388

391-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
392-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
389+
String cert = readFileToString(new File(certFile));
390+
String key = readFileToString(new File(keyFile));
393391

394392
CertServiceImpl certService = new CertServiceImpl();
395393

@@ -432,8 +430,8 @@ public void runUploadSslCertBadkeyPair() throws IOException, IllegalAccessExcept
432430
String certFile = getClass().getResource("/certs/rsa_self_signed.crt").getFile();
433431
String keyFile = getClass().getResource("/certs/rsa_random_pkey.key").getFile();
434432

435-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
436-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
433+
String cert = readFileToString(new File(certFile));
434+
String key = readFileToString(new File(keyFile));
437435

438436
CertServiceImpl certService = new CertServiceImpl();
439437

@@ -471,8 +469,8 @@ public void runUploadSslCertBadkeyAlgo() throws IOException, IllegalAccessExcept
471469
String certFile = getClass().getResource("/certs/rsa_self_signed.crt").getFile();
472470
String keyFile = getClass().getResource("/certs/dsa_self_signed.key").getFile();
473471

474-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
475-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
472+
String cert = readFileToString(new File(certFile));
473+
String key = readFileToString(new File(keyFile));
476474

477475
CertServiceImpl certService = new CertServiceImpl();
478476

@@ -511,8 +509,8 @@ public void runUploadSslCertExpiredCert() throws IOException, IllegalAccessExcep
511509
String certFile = getClass().getResource("/certs/expired_cert.crt").getFile();
512510
String keyFile = getClass().getResource("/certs/rsa_self_signed.key").getFile();
513511

514-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
515-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
512+
String cert = readFileToString(new File(certFile));
513+
String key = readFileToString(new File(keyFile));
516514

517515
CertServiceImpl certService = new CertServiceImpl();
518516

@@ -550,8 +548,8 @@ public void runUploadSslCertNotX509() throws IOException, IllegalAccessException
550548
String certFile = getClass().getResource("/certs/non_x509_pem.crt").getFile();
551549
String keyFile = getClass().getResource("/certs/rsa_self_signed.key").getFile();
552550

553-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
554-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
551+
String cert = readFileToString(new File(certFile));
552+
String key = readFileToString(new File(keyFile));
555553

556554
CertServiceImpl certService = new CertServiceImpl();
557555

@@ -590,8 +588,8 @@ public void runUploadSslCertBadFormat() throws IOException, IllegalAccessExcepti
590588
String certFile = getClass().getResource("/certs/bad_format_cert.crt").getFile();
591589
String keyFile = getClass().getResource("/certs/rsa_self_signed.key").getFile();
592590

593-
String cert = URLEncoder.encode(readFileToString(new File(certFile)), "UTF-8");
594-
String key = URLEncoder.encode(readFileToString(new File(keyFile)), "UTF-8");
591+
String cert = readFileToString(new File(certFile));
592+
String key = readFileToString(new File(keyFile));
595593

596594
CertServiceImpl certService = new CertServiceImpl();
597595

0 commit comments

Comments
 (0)