[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15710067#comment-15710067
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9632:
--------------------------------------------

Github user jburwell commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1799#discussion_r90341463
  
    --- Diff: server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java 
---
    @@ -339,190 +348,182 @@ public SslCertResponse createCertResponse(SslCertVO 
cert, List<LoadBalancerCertM
             return response;
         }
     
    -    private void validateCert(Certificate cert, boolean chainPresent) {
    +    private void validateCert(final Certificate cert, final boolean 
chainPresent) {
     
    -        if (!(cert instanceof X509Certificate))
    +        if (!(cert instanceof X509Certificate)) {
                 throw new IllegalArgumentException("Invalid certificate 
format. Expected X509 certificate");
    +        }
     
             try {
                 ((X509Certificate)cert).checkValidity();
    -        } catch (Exception e) {
    +        } catch (final Exception e) {
                 throw new IllegalArgumentException("Certificate expired or not 
valid", e);
             }
         }
     
    -    private void validateKeys(PublicKey pubKey, PrivateKey privKey) {
    +    private void validateKeys(final PublicKey pubKey, final PrivateKey 
privKey) {
     
    -        if (pubKey.getAlgorithm() != privKey.getAlgorithm())
    +        if (pubKey.getAlgorithm() != privKey.getAlgorithm()) {
                 throw new IllegalArgumentException("Public and private key 
have different algorithms");
    +        }
     
             // No encryption for DSA
    -        if (pubKey.getAlgorithm() != "RSA")
    +        if (pubKey.getAlgorithm() != "RSA") {
                 return;
    +        }
     
             try {
     
    -            String data = "ENCRYPT_DATA";
    -            SecureRandom random = new SecureRandom();
    -            Cipher cipher = Cipher.getInstance(pubKey.getAlgorithm());
    +            final String data = "ENCRYPT_DATA";
    +            final SecureRandom random = new SecureRandom();
    +            final Cipher cipher = 
Cipher.getInstance(pubKey.getAlgorithm());
                 cipher.init(Cipher.ENCRYPT_MODE, privKey, random);
    -            byte[] encryptedData = cipher.doFinal(data.getBytes());
    +            final byte[] encryptedData = cipher.doFinal(data.getBytes());
     
                 cipher.init(Cipher.DECRYPT_MODE, pubKey, random);
    -            String decreptedData = new 
String(cipher.doFinal(encryptedData));
    -            if (!decreptedData.equals(data))
    +            final String decreptedData = new 
String(cipher.doFinal(encryptedData));
    +            if (!decreptedData.equals(data)) {
                     throw new IllegalArgumentException("Bad public-private 
key");
    +            }
     
    -        } catch (BadPaddingException e) {
    +        } catch (final BadPaddingException e) {
                 throw new IllegalArgumentException("Bad public-private key", 
e);
    -        } catch (IllegalBlockSizeException e) {
    +        } catch (final IllegalBlockSizeException e) {
                 throw new IllegalArgumentException("Bad public-private key", 
e);
    -        } catch (NoSuchPaddingException e) {
    +        } catch (final NoSuchPaddingException e) {
                 throw new IllegalArgumentException("Bad public-private key", 
e);
    -        } catch (InvalidKeyException e) {
    +        } catch (final InvalidKeyException e) {
                 throw new IllegalArgumentException("Invalid public-private 
key", e);
    -        } catch (NoSuchAlgorithmException e) {
    +        } catch (final NoSuchAlgorithmException e) {
                 throw new IllegalArgumentException("Invalid algorithm for 
public-private key", e);
             }
         }
     
    -    private void validateChain(List<Certificate> chain, Certificate cert) {
    +    private void validateChain(final List<Certificate> chain, final 
Certificate cert) {
     
    -        List<Certificate> certs = new ArrayList<Certificate>();
    -        Set<TrustAnchor> anchors = new HashSet<TrustAnchor>();
    +        final List<Certificate> certs = new ArrayList<Certificate>();
    +        final Set<TrustAnchor> anchors = new HashSet<TrustAnchor>();
     
             certs.add(cert); // adding for self signed certs
             certs.addAll(chain);
     
    -        for (Certificate c : certs) {
    -            if (!(c instanceof X509Certificate))
    +        for (final Certificate c : certs) {
    +            if (!(c instanceof X509Certificate)) {
                     throw new IllegalArgumentException("Invalid chain format. 
Expected X509 certificate");
    +            }
     
    -            X509Certificate xCert = (X509Certificate)c;
    +            final X509Certificate xCert = (X509Certificate)c;
     
    -            Principal subject = xCert.getSubjectDN();
    -            Principal issuer = xCert.getIssuerDN();
    +            xCert.getSubjectDN();
    +            xCert.getIssuerDN();
     
    -           anchors.add(new TrustAnchor(xCert, null));
    +            anchors.add(new TrustAnchor(xCert, null));
             }
     
    -        X509CertSelector target = new X509CertSelector();
    +        final X509CertSelector target = new X509CertSelector();
             target.setCertificate((X509Certificate)cert);
     
             PKIXBuilderParameters params = null;
             try {
                 params = new PKIXBuilderParameters(anchors, target);
                 params.setRevocationEnabled(false);
                 params.addCertStore(CertStore.getInstance("Collection", new 
CollectionCertStoreParameters(certs)));
    -            CertPathBuilder builder = CertPathBuilder.getInstance("PKIX", 
"BC");
    +            final CertPathBuilder builder = 
CertPathBuilder.getInstance("PKIX", "BC");
                 builder.build(params);
     
    -        } catch (InvalidAlgorithmParameterException e) {
    +        } catch (final InvalidAlgorithmParameterException e) {
                 throw new IllegalArgumentException("Invalid certificate 
chain", e);
    -        } catch (CertPathBuilderException e) {
    +        } catch (final CertPathBuilderException e) {
                 throw new IllegalArgumentException("Invalid certificate 
chain", e);
    -        } catch (NoSuchAlgorithmException e) {
    +        } catch (final NoSuchAlgorithmException e) {
                 throw new IllegalArgumentException("Invalid certificate 
chain", e);
    -        } catch (NoSuchProviderException e) {
    +        } catch (final NoSuchProviderException e) {
                 throw new CloudRuntimeException("No provider for certificate 
validation", e);
             }
     
         }
     
    -    public PrivateKey parsePrivateKey(String key, String password) throws 
IOException {
    -
    -        PasswordFinder pGet = null;
    -
    -        if (password != null)
    -            pGet = new KeyPassword(password.toCharArray());
    -
    -        PEMReader privateKey = new PEMReader(new StringReader(key), pGet);
    -        Object obj = null;
    -        try {
    -            obj = privateKey.readObject();
    -        } finally {
    -            IOUtils.closeQuietly(privateKey);
    -        }
    -
    -        try {
    -
    -            if (obj instanceof KeyPair)
    -                return ((KeyPair)obj).getPrivate();
    -
    -            return (PrivateKey)obj;
    -
    -        } catch (Exception e) {
    -            throw new IOException("Invalid Key format or invalid 
password.", e);
    +    public PrivateKey parsePrivateKey(final String key) throws IOException 
{
    +        try (final PemReader pemReader = new PemReader(new 
StringReader(key));) {
    +            final PemObject pemObject = pemReader.readPemObject();
    +            final byte[] content = pemObject.getContent();
    +            final PKCS8EncodedKeySpec privKeySpec = new 
PKCS8EncodedKeySpec(content);
    +            final KeyFactory factory = KeyFactory.getInstance("RSA", "BC");
    +            return factory.generatePrivate(privKeySpec);
    +        } catch (NoSuchAlgorithmException | NoSuchProviderException e) {
    +            throw new IOException("No encryption provider available.", e);
    +        } catch (final InvalidKeySpecException e) {
    +            throw new IOException("Invalid Key format.", e);
             }
         }
     
    -    public Certificate parseCertificate(String cert) {
    -        PEMReader certPem = new PEMReader(new StringReader(cert));
    +    public Certificate parseCertificate(final String cert) {
    --- End diff --
    
    Please consider adding a `Precondition.checkArgument` to check that `cert` 
is not blank.


> Upgrade bountycastle to 1.55+
> -----------------------------
>
>                 Key: CLOUDSTACK-9632
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9632
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>            Reporter: Rohit Yadav
>            Assignee: Rohit Yadav
>             Fix For: Future, 4.10.0.0
>
>
> Upgrade bountycastle library to latest versions.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to