Skip to content

Commit be9fa5d

Browse files
weizhouapacheweizhouapache
committed
PR11468: address Daan's comments
1 parent 5a0ec98 commit be9fa5d

File tree

6 files changed

+93
-66
lines changed

6 files changed

+93
-66
lines changed

‎core/src/main/java/com/cloud/network/HAProxyConfigurator.java‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -734,20 +734,24 @@ public SslCertEntry[] generateSslCertEntries(LoadBalancerConfigCommand lbCmd) {
734734
final Set<SslCertEntry> sslCertEntries = new HashSet<SslCertEntry>();
735735
for (final LoadBalancerTO lbTO : lbCmd.getLoadBalancers()) {
736736
if (lbTO.getSslCert() != null) {
737-
final LbSslCert cert = lbTO.getSslCert();
738-
if (cert.isRevoked()) {
739-
continue;
740-
}
741-
if (lbTO.getLbProtocol() == null || ! lbTO.getLbProtocol().equals(NetUtils.SSL_PROTO)) {
742-
continue;
743-
}
744-
StringBuilder sb = new StringBuilder();
745-
final String name = sb.append(lbTO.getSrcIp().replace(".", "_")).append('-').append(lbTO.getSrcPort()).toString();
746-
final SslCertEntry sslCertEntry = new SslCertEntry(name, cert.getCert(), cert.getKey(), cert.getChain(), cert.getPassword());
747-
sslCertEntries.add(sslCertEntry);
737+
addSslCertEntry(sslCertEntries, lbTO);
748738
}
749739
}
750740
final SslCertEntry[] result = sslCertEntries.toArray(new SslCertEntry[sslCertEntries.size()]);
751741
return result;
752742
}
743+
744+
private void addSslCertEntry(Set<SslCertEntry> sslCertEntries, LoadBalancerTO lbTO) {
745+
final LbSslCert cert = lbTO.getSslCert();
746+
if (cert.isRevoked()) {
747+
return;
748+
}
749+
if (lbTO.getLbProtocol() == null || ! lbTO.getLbProtocol().equals(NetUtils.SSL_PROTO)) {
750+
return;
751+
}
752+
StringBuilder sb = new StringBuilder();
753+
final String name = sb.append(lbTO.getSrcIp().replace(".", "_")).append('-').append(lbTO.getSrcPort()).toString();
754+
final SslCertEntry sslCertEntry = new SslCertEntry(name, cert.getCert(), cert.getKey(), cert.getChain(), cert.getPassword());
755+
sslCertEntries.add(sslCertEntry);
756+
}
753757
}

‎server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,7 +1270,7 @@ public LbSslCert getLbSslCert(long lbRuleId) {
12701270
public boolean assignCertToLoadBalancer(long lbRuleId, Long certId, boolean forced) {
12711271
CallContext caller = CallContext.current();
12721272

1273-
LoadBalancerVO loadBalancer = _lbDao.findById(Long.valueOf(lbRuleId));
1273+
LoadBalancerVO loadBalancer = _lbDao.findById(lbRuleId);
12741274
if (loadBalancer == null) {
12751275
throw new InvalidParameterValueException("Invalid load balancer id: " + lbRuleId);
12761276
}
@@ -1292,15 +1292,7 @@ public boolean assignCertToLoadBalancer(long lbRuleId, Long certId, boolean forc
12921292
throw new InvalidParameterValueException("Ssl termination not supported by the loadbalancer");
12931293
}
12941294

1295-
//check if the lb is already bound
1296-
LoadBalancerCertMapVO certMapRule = _lbCertMapDao.findByLbRuleId(loadBalancer.getId());
1297-
if (certMapRule != null) {
1298-
if (!forced) {
1299-
throw new InvalidParameterValueException("Another certificate is already bound to the LB");
1300-
}
1301-
logger.debug("Another certificate is already bound to the LB, removing it");
1302-
removeCertFromLoadBalancer(lbRuleId);
1303-
}
1295+
validateCertMapRule(lbRuleId, forced);
13041296

13051297
//check for correct port
13061298
if (loadBalancer.getLbProtocol() == null || !(loadBalancer.getLbProtocol().equals(NetUtils.SSL_PROTO)))
@@ -1331,6 +1323,18 @@ public boolean assignCertToLoadBalancer(long lbRuleId, Long certId, boolean forc
13311323
return success;
13321324
}
13331325

1326+
private void validateCertMapRule(long lbRuleId, boolean forced) {
1327+
//check if the lb is already bound
1328+
LoadBalancerCertMapVO certMapRule = _lbCertMapDao.findByLbRuleId(lbRuleId);
1329+
if (certMapRule != null) {
1330+
if (!forced) {
1331+
throw new InvalidParameterValueException("Another certificate is already bound to the LB");
1332+
}
1333+
logger.debug("Another certificate is already bound to the LB, removing it");
1334+
removeCertFromLoadBalancer(lbRuleId);
1335+
}
1336+
}
1337+
13341338
@Override
13351339
@DB
13361340
@ActionEvent(eventType = EventTypes.EVENT_LB_CERT_REMOVE, eventDescription = "removing certificate from load balancer", async = true)
@@ -2271,11 +2275,7 @@ public LoadBalancer updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd) {
22712275
_lbDao.persist(lb);
22722276
applyLoadBalancerConfig(lbRuleId);
22732277
if (!lb.getLbProtocol().equals(NetUtils.SSL_PROTO)) {
2274-
LoadBalancerCertMapVO loadBalancerCertMapVO = _lbCertMapDao.findByLbRuleId(lbRuleId);
2275-
if (loadBalancerCertMapVO != null) {
2276-
logger.debug("Removing SSL cert for load balancer %s as the new protocol is not ssl but %s", lbRuleId, lb.getLbProtocol());
2277-
_lbCertMapDao.remove(loadBalancerCertMapVO.getId());
2278-
}
2278+
removeCertMapIfExists(lb);
22792279
}
22802280
} catch (ResourceUnavailableException e) {
22812281
if (isRollBackAllowedForProvider(lb)) {
@@ -2323,6 +2323,13 @@ private void validateInputsForExternalNetworkProvider(LoadBalancerVO lb, String
23232323
if (Objects.nonNull(protocol) && "tcp-proxy".equalsIgnoreCase(protocol)) {
23242324
throw new InvalidParameterValueException("TCP Proxy protocol is not supported for Netris Provider.");
23252325
}
2326+
}
2327+
2328+
private void removeCertMapIfExists(LoadBalancerVO lb) {
2329+
LoadBalancerCertMapVO loadBalancerCertMapVO = _lbCertMapDao.findByLbRuleId(lb.getId());
2330+
if (loadBalancerCertMapVO != null) {
2331+
logger.debug("Removing SSL cert for load balancer %s as the new protocol is not ssl but %s", lb, lb.getLbProtocol());
2332+
_lbCertMapDao.remove(loadBalancerCertMapVO.getId());
23262333
}
23272334
}
23282335

‎server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -929,10 +929,7 @@ public boolean validateHAProxyLBRule(final LoadBalancingRule rule) {
929929
return false;
930930
}
931931

932-
List<String> lbProtocols = Arrays.asList("tcp", "udp", "tcp-proxy", "ssl");
933-
if (rule.getLbProtocol() != null && !lbProtocols.contains(rule.getLbProtocol())) {
934-
throw new InvalidParameterValueException("protocol " + rule.getLbProtocol() + " is not in valid protocols " + lbProtocols);
935-
}
932+
validateHAproxyLbProtocol(rule.getLbProtocol());
936933

937934
for (final LoadBalancingRule.LbStickinessPolicy stickinessPolicy : rule.getStickinessPolicies()) {
938935
final List<Pair<String, String>> paramsList = stickinessPolicy.getParams();
@@ -987,6 +984,13 @@ public boolean validateHAProxyLBRule(final LoadBalancingRule rule) {
987984
return true;
988985
}
989986

987+
private void validateHAproxyLbProtocol(String lbProtocol) {
988+
List<String> lbProtocols = Arrays.asList("tcp", "udp", "tcp-proxy", "ssl");
989+
if (lbProtocol != null && !lbProtocols.contains(lbProtocol)) {
990+
throw new InvalidParameterValueException(String.format("protocol %s is not in valid protocols %s", lbProtocol, lbProtocols));
991+
}
992+
}
993+
990994
/*
991995
* This function detects numbers like 12 ,32h ,42m .. etc,. 1) plain number
992996
* like 12 2) time or tablesize like 12h, 34m, 45k, 54m , here last

‎server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1743,13 +1743,7 @@ private void updateWithLbRules(final DomainRouterJoinVO routerJoinVO, final Stri
17431743
.append(",destPortEnd=").append(loadBalancerVO.getDefaultPortEnd())
17441744
.append(",algorithm=").append(loadBalancerVO.getAlgorithm())
17451745
.append(",protocol=").append(loadBalancerVO.getLbProtocol());
1746-
if (loadBalancerVO.getLbProtocol() != null && loadBalancerVO.getLbProtocol().equals(NetUtils.SSL_PROTO)) {
1747-
final LbSslCert sslCert = _lbMgr.getLbSslCert(firewallRuleVO.getId());
1748-
if (sslCert != null && ! sslCert.isRevoked()) {
1749-
loadBalancingData.append(",sslcert=").append(sourceIp.replace(".", "_")).append('-')
1750-
.append(loadBalancerVO.getSourcePortStart()).append(".pem");
1751-
}
1752-
}
1746+
updateWithLbRuleSslCertificates(loadBalancingData, loadBalancerVO, sourceIp);
17531747
} else if (firewallRuleVO instanceof ApplicationLoadBalancerRuleVO) {
17541748
ApplicationLoadBalancerRuleVO appLoadBalancerVO = (ApplicationLoadBalancerRuleVO) firewallRuleVO;
17551749
loadBalancingData.append(",sourceIp=").append(appLoadBalancerVO.getSourceIp())
@@ -1768,6 +1762,16 @@ private void updateWithLbRules(final DomainRouterJoinVO routerJoinVO, final Stri
17681762
}
17691763
}
17701764

1765+
private void updateWithLbRuleSslCertificates(final StringBuilder loadBalancingData, LoadBalancerVO loadBalancerVO, String sourceIp) {
1766+
if (NetUtils.SSL_PROTO.equals(loadBalancerVO.getLbProtocol())) {
1767+
final LbSslCert sslCert = _lbMgr.getLbSslCert(loadBalancerVO.getId());
1768+
if (sslCert != null && ! sslCert.isRevoked()) {
1769+
loadBalancingData.append(",sslcert=").append(sourceIp.replace(".", "_")).append('-')
1770+
.append(loadBalancerVO.getSourcePortStart()).append(".pem");
1771+
}
1772+
}
1773+
}
1774+
17711775
protected Map<String, String> getRouterHealthChecksConfig(final DomainRouterVO router) {
17721776
Map<String, String> data = new HashMap<>();
17731777
List<DomainRouterJoinVO> routerJoinVOs = domainRouterJoinDao.searchByIds(router.getId());

‎server/src/main/java/org/apache/cloudstack/network/ssl/CertServiceImpl.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -441,25 +441,9 @@ public PrivateKey parsePrivateKey(final String key, String password) throws IOEx
441441
}
442442
PrivateKey privateKey;
443443
if (privateKeyObj instanceof PKCS8EncryptedPrivateKeyInfo) {
444-
if (password == null) {
445-
throw new CloudRuntimeException("Key is encrypted by PKCS#8 but password is null");
446-
}
447-
PKCS8EncryptedPrivateKeyInfo encryptedPrivateKeyInfo = (PKCS8EncryptedPrivateKeyInfo)privateKeyObj;
448-
JceOpenSSLPKCS8DecryptorProviderBuilder builder = new JceOpenSSLPKCS8DecryptorProviderBuilder();
449-
InputDecryptorProvider decryptor = builder.build(password.toCharArray());
450-
451-
PrivateKeyInfo privateKeyInfo = encryptedPrivateKeyInfo.decryptPrivateKeyInfo(decryptor);
452-
String algorithm = privateKeyInfo.getPrivateKeyAlgorithm().getAlgorithm().getId();
453-
KeyFactory keyFactory = KeyFactory.getInstance(algorithm);
454-
PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(privateKeyInfo.getEncoded());
455-
return keyFactory.generatePrivate(keySpec);
444+
privateKey = parsePKCS8EncryptedPrivateKeyInfo((PKCS8EncryptedPrivateKeyInfo)privateKeyObj, password);
456445
} else if (privateKeyObj instanceof PEMEncryptedKeyPair) {
457-
if (password == null) {
458-
throw new CloudRuntimeException("Key is encrypted but password is null");
459-
}
460-
PEMEncryptedKeyPair encryptedKeyPair = (PEMEncryptedKeyPair)privateKeyObj;
461-
privateKey = new JcaPEMKeyConverter().getKeyPair(
462-
encryptedKeyPair.decryptKeyPair(new JcePEMDecryptorProviderBuilder().build(password.toCharArray()))).getPrivate();
446+
privateKey = parsePEMEncryptedKeyPair((PEMEncryptedKeyPair)privateKeyObj, password);
463447
} else if (privateKeyObj instanceof PEMKeyPair) {
464448
// Key pair
465449
PEMKeyPair pemKeyPair = (PEMKeyPair) privateKeyObj;
@@ -475,6 +459,30 @@ public PrivateKey parsePrivateKey(final String key, String password) throws IOEx
475459
return privateKey;
476460
}
477461

462+
private PrivateKey parsePKCS8EncryptedPrivateKeyInfo(PKCS8EncryptedPrivateKeyInfo privateKeyObj, String password)
463+
throws IOException, OperatorCreationException, PKCSException, NoSuchAlgorithmException, InvalidKeySpecException {
464+
if (password == null) {
465+
throw new CloudRuntimeException("Key is encrypted by PKCS#8 but password is null");
466+
}
467+
PKCS8EncryptedPrivateKeyInfo encryptedPrivateKeyInfo = (PKCS8EncryptedPrivateKeyInfo)privateKeyObj;
468+
JceOpenSSLPKCS8DecryptorProviderBuilder builder = new JceOpenSSLPKCS8DecryptorProviderBuilder();
469+
InputDecryptorProvider decryptor = builder.build(password.toCharArray());
470+
471+
PrivateKeyInfo privateKeyInfo = encryptedPrivateKeyInfo.decryptPrivateKeyInfo(decryptor);
472+
String algorithm = privateKeyInfo.getPrivateKeyAlgorithm().getAlgorithm().getId();
473+
KeyFactory keyFactory = KeyFactory.getInstance(algorithm);
474+
PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(privateKeyInfo.getEncoded());
475+
return keyFactory.generatePrivate(keySpec);
476+
}
477+
478+
private PrivateKey parsePEMEncryptedKeyPair(PEMEncryptedKeyPair encryptedKeyPair, String password) throws IOException {
479+
if (password == null) {
480+
throw new CloudRuntimeException("Key is encrypted but password is null");
481+
}
482+
return new JcaPEMKeyConverter().getKeyPair(
483+
encryptedKeyPair.decryptKeyPair(new JcePEMDecryptorProviderBuilder().build(password.toCharArray()))).getPrivate();
484+
}
485+
478486
@Override
479487
public Certificate parseCertificate(final String cert) {
480488
Preconditions.checkArgument(StringUtils.isNotEmpty(cert));

‎test/integration/smoke/test_ssl_offloading.py‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -193,16 +193,6 @@ def setUpClass(cls):
193193

194194
cls.services["virtual_machine"]["zoneid"] = cls.zone.id
195195

196-
#Create an account, network, VM and IP addresses
197-
cls.account = Account.create(
198-
cls.apiclient,
199-
cls.services["account"],
200-
admin=True,
201-
domainid=cls.domain.id
202-
)
203-
cls.user = cls.account.user[0]
204-
cls.userapiclient = cls.testClient.getUserApiClient(cls.user.username, cls.domain.name)
205-
206196
# Save full chain as a file
207197
with open(FULL_CHAIN, "w", encoding="utf-8") as f:
208198
f.write(CERT["certchain"])
@@ -231,6 +221,7 @@ def setUpClass(cls):
231221
cls.apiclient,
232222
cls.services["service_offerings"]["big"] # 512MB memory
233223
)
224+
cls._cleanup.append(cls.service_offering)
234225

235226
# Create network offering
236227
cls.services["isolated_network_offering"]["egress_policy"] = "true"
@@ -240,8 +231,17 @@ def setUpClass(cls):
240231
cls.network_offering.update(cls.apiclient, state='Enabled')
241232

242233
cls._cleanup.append(cls.network_offering)
243-
cls._cleanup.append(cls.service_offering)
234+
235+
#Create an account, network, VM and IP addresses
236+
cls.account = Account.create(
237+
cls.apiclient,
238+
cls.services["account"],
239+
admin=True,
240+
domainid=cls.domain.id
241+
)
244242
cls._cleanup.append(cls.account)
243+
cls.user = cls.account.user[0]
244+
cls.userapiclient = cls.testClient.getUserApiClient(cls.user.username, cls.domain.name)
245245

246246
def setUp(self):
247247
self.apiclient = self.testClient.getApiClient()

0 commit comments

Comments
 (0)