[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pki-devel] [PATCH] pki-cfu-0084-Ticket-1459-Dogtag-clients-cannot-connect-when-CS-is.patch



Thank you jack for the review.
Also, thanks to Matt for helping out with the console dependency issue.

Please see the attached revision that addressed the comments.
It has been tested to work on all three types of clients.

thanks,
Christina

On 07/10/2015 03:59 PM, John Magne wrote:
Functionality looks good,
just a few minor suggestions:


1. This code:

+
+    static final Integer[] clientECCciphers = {
+        SSLSocket.TLS_ECDH_RSA_WITH_AES_128_CBC_SHA,
+        SSLSocket.TLS_ECDH_RSA_WITH_AES_256_CBC_SHA,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
+        SSLSocket.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
+    };
+    ArrayList<Integer> eccCiphers = new ArrayList(Arrays.asList(clientECCciphers));



For the ArrayList declaration:

Eclipse is complaining about unsafe conversions, it suggests something like:

ArrayList<Integer> eccCiphers = new ArrayList<Integer>(Arrays.asList(clientECCciphers));

Also, I think we can declare this with a more general Collection such as "List" and leave the ArrayList implementation, since
that appears to be the convention.

Also, due to the final suggestion, we can make  List static, since nothing changes and one would do.

2. I see the similar code to instantiate all this data and ensure the ciphers are legit is copied around 3 times.
I think we can move this stuff to a common class and have everyone use it, so if it needs to change it will only change in one place.





----- Original Message -----
From: "Christina Fu" <cfu redhat com>
To: pki-devel redhat com
Sent: Friday, July 10, 2015 11:51:08 AM
Subject: [Pki-devel] [PATCH]	pki-cfu-0084-Ticket-1459-Dogtag-clients-cannot-connect-when-CS-is.patch

These patches address the following ticket:
https://fedorahosted.org/pki/ticket/1459 Dogtag clients cannot connect
when CS is configured with ECC

the first patch is just to clean up the tabs in the constructor of the
file JSSConnection in preparation for code changes :
pki-cfu-0083-ecc-Console-1.-clean-up-the-tabs-in-the-JSSConnectio.patch

The second patch addresses the ECC ssl connection issue from the
- java console
- cli clients
- HttpClient

They have been tested to work with ECC ca.

thanks,
Christina

_______________________________________________
Pki-devel mailing list
Pki-devel redhat com
https://www.redhat.com/mailman/listinfo/pki-devel

>From f24d453ea962add1766725462741f417c14fc4d6 Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu redhat com>
Date: Fri, 10 Jul 2015 11:41:22 -0700
Subject: [PATCH] Ticket 1459 Dogtag clients cannot connect when CS is
 configured with ECC

---
 .../com/netscape/certsrv/client/PKIConnection.java | 34 ++++++++++++++++++++++
 base/console/src/CMakeLists.txt                    | 10 ++++++-
 .../admin/certsrv/connection/JSSConnection.java    | 28 ++++++++++++++++++
 base/console/templates/pki_console_wrapper         |  2 ++
 .../src/com/netscape/cmstools/HttpClient.java      | 30 +++++++++++++++++++
 .../com/netscape/cmsutil/crypto/CryptoUtil.java    | 15 ++++++++++
 6 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/base/common/src/com/netscape/certsrv/client/PKIConnection.java b/base/common/src/com/netscape/certsrv/client/PKIConnection.java
index 1f9b6dff16e88bd3746b2d9627fa14b1d1cd1cd5..a06f7f665bfe998048e70f19160cd90621723b9f 100644
--- a/base/common/src/com/netscape/certsrv/client/PKIConnection.java
+++ b/base/common/src/com/netscape/certsrv/client/PKIConnection.java
@@ -24,6 +24,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.Integer;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Socket;
@@ -82,6 +83,7 @@ import org.mozilla.jss.ssl.SSLCertificateApprovalCallback;
 import org.mozilla.jss.ssl.SSLSocket;
 
 import com.netscape.certsrv.base.PKIException;
+import com.netscape.cmsutil.crypto.CryptoUtil;
 
 
 public class PKIConnection {
@@ -346,6 +348,38 @@ public class PKIConnection {
             SSLSocket.setSSLVersionRangeDefault(
                     org.mozilla.jss.ssl.SSLSocket.SSLProtocolVariant.DATA_GRAM,
                     datagram_range);
+
+            int ciphers[] = SSLSocket.getImplementedCipherSuites();
+            for (int j = 0; ciphers != null && j < ciphers.length; j++) {
+                boolean enabled = SSLSocket.getCipherPreferenceDefault(ciphers[j]);
+                if (verbose) {
+                    System.out.println("Debug: cipher '0x" +
+                        Integer.toHexString(ciphers[j]) + "'" + " enabled? " +
+                        enabled);
+                }
+                // make sure SSLv2 ciphers are not enabled
+                if ((ciphers[j] & 0xfff0) ==0xff00) {
+                    if (enabled) {
+                        if (verbose) {
+                          System.out.println("Debug: disabling SSL2 NSS Cipher '0x" +
+                          Integer.toHexString(ciphers[j]) + "'");
+                        }
+                        SSLSocket.setCipherPreferenceDefault(ciphers[j], false);
+                    }
+                } else {
+                    /*
+                     * unlike RSA ciphers, ECC ciphers are not enabled by default
+                     */
+                    if ((!enabled) && CryptoUtil.clientECCipherList.contains(ciphers[j])) {
+                        if (verbose) {
+                          System.out.println("Debug: enabling ECC NSS Cipher '0x" +
+                          Integer.toHexString(ciphers[j]) + "'");
+                        }
+                        SSLSocket.setCipherPreferenceDefault(ciphers[j], true);
+                    }
+                }
+            }
+
             SSLSocket socket;
             if (sock == null) {
                 socket = new SSLSocket(InetAddress.getByName(hostName),
diff --git a/base/console/src/CMakeLists.txt b/base/console/src/CMakeLists.txt
index 3dc0f5d4152544ecade008362c2fb01a93e97d5c..c1a86b7c4c0e6245a3d58675cc63c6287a573ecc 100644
--- a/base/console/src/CMakeLists.txt
+++ b/base/console/src/CMakeLists.txt
@@ -17,6 +17,14 @@ find_file(PKI_CERTSRV_JAR
         /usr/share/java/pki
 )
 
+find_file(PKI_CMSUTIL_JAR
+    NAMES
+        pki-cmsutil.jar
+    PATHS
+        ${JAVA_LIB_INSTALL_DIR}
+        /usr/share/java/pki
+)
+
 
 # '/usr/share/java' jars
 find_file(BASE_JAR
@@ -92,7 +100,7 @@ javac(pki-console-classes
         ${CMAKE_BINARY_DIR}/classes
         ${BASE_JAR} ${LDAPJDK_JAR} ${MMC_JAR}
         ${MMC_EN_JAR} ${NMCLF_JAR} ${NMCLF_EN_JAR}
-        ${PKI_NSUTIL_JAR} ${PKI_CERTSRV_JAR}
+        ${PKI_CMSUTIL_JAR} ${PKI_NSUTIL_JAR} ${PKI_CERTSRV_JAR}
         ${JSS_JAR} ${COMMONS_CODEC_JAR}
     OUTPUT_DIR
         ${CMAKE_BINARY_DIR}/classes
diff --git a/base/console/src/com/netscape/admin/certsrv/connection/JSSConnection.java b/base/console/src/com/netscape/admin/certsrv/connection/JSSConnection.java
index 43d1c234b39df8e4c133cb6b6368e3f1a9c0d9f9..5c135cf86e5884bed635244181530acc4ec04ed1 100644
--- a/base/console/src/com/netscape/admin/certsrv/connection/JSSConnection.java
+++ b/base/console/src/com/netscape/admin/certsrv/connection/JSSConnection.java
@@ -34,6 +34,8 @@ import org.mozilla.jss.pkcs11.*;
 import javax.swing.*;
 import java.awt.*;
 
+import com.netscape.cmsutil.crypto.CryptoUtil;
+
 /**
  * JSSConnection deals with establishing a connection to
  * a server, sending requests and reading responses.
@@ -113,6 +115,32 @@ public class JSSConnection implements IConnection, SSLCertificateApprovalCallbac
         SSLSocket.setSSLVersionRangeDefault(
             org.mozilla.jss.ssl.SSLSocket.SSLProtocolVariant.DATA_GRAM,
             datagram_range);
+
+        int ciphers[] = SSLSocket.getImplementedCipherSuites();
+        for (int i = 0; ciphers != null && i < ciphers.length; i++) {
+            boolean enabled = SSLSocket.getCipherPreferenceDefault(ciphers[i]);
+            Debug.println("JSSConnection Debug: cipher '0x" +
+                Integer.toHexString(ciphers[i]) + "'" + " enabled? " +
+                enabled);
+            // make sure SSLv2 ciphers are not enabled
+            if ((ciphers[i] & 0xfff0) ==0xff00) {
+                if (enabled) {
+                    Debug.println("JSSConnection Debug: disabling SSL2 NSS Cipher '0x" +
+                    Integer.toHexString(ciphers[i]) + "'");
+                    SSLSocket.setCipherPreferenceDefault(ciphers[i], false);
+                }
+            } else {
+                /*
+                 * unlike RSA ciphers, ECC ciphers are not enabled by default
+                 */
+                if ((!enabled) && CryptoUtil.clientECCipherList.contains(ciphers[i])) {
+                    Debug.println("JSSConnection Debug: enabling ECC NSS Cipher '0x" +
+                        Integer.toHexString(ciphers[i]) + "'");
+                    SSLSocket.setCipherPreferenceDefault(ciphers[i], true);
+                }
+            }
+        }
+
         s = new SSLSocket(host, port, null, 0, this, this);
 
         // Initialze Http Input and Output Streams
diff --git a/base/console/templates/pki_console_wrapper b/base/console/templates/pki_console_wrapper
index 2f110ed8501696a33139585022ffa4c7cea13c80..296eba24d4934303197a49230e33613a92265ece 100755
--- a/base/console/templates/pki_console_wrapper
+++ b/base/console/templates/pki_console_wrapper
@@ -138,6 +138,8 @@ CP=/usr/share/java/idm-console-mcc.jar:${CP}
 CP=/usr/share/java/idm-console-mcc_en.jar:${CP}
 CP=/usr/share/java/idm-console-base.jar:${CP}
 CP=/usr/share/java/389-console_en.jar:${CP}
+CP=/usr/share/java/${PRODUCT}/pki-nsutil.jar:${CP}
+CP=/usr/share/java/${PRODUCT}/pki-cmsutil.jar:${CP}
 CP=/usr/share/java/${PRODUCT}/pki-certsrv.jar:${CP}
 CP=/usr/share/java/${PRODUCT}/pki-console-theme.jar:${CP}
 CP=/usr/share/java/${PRODUCT}/pki-console.jar:${CP}
diff --git a/base/java-tools/src/com/netscape/cmstools/HttpClient.java b/base/java-tools/src/com/netscape/cmstools/HttpClient.java
index f0603a4bd10f17240b5f3957d8d91da5af7b07a1..f6575feefa689ad28b1ab054c9a0945be778b165 100644
--- a/base/java-tools/src/com/netscape/cmstools/HttpClient.java
+++ b/base/java-tools/src/com/netscape/cmstools/HttpClient.java
@@ -31,6 +31,8 @@ import java.io.InputStreamReader;
 import java.io.PrintStream;
 import java.net.Socket;
 import java.util.StringTokenizer;
+import java.util.Arrays;
+import java.util.ArrayList;
 
 import org.mozilla.jss.CryptoManager;
 import org.mozilla.jss.crypto.CryptoToken;
@@ -40,6 +42,7 @@ import org.mozilla.jss.ssl.SSLHandshakeCompletedListener;
 import org.mozilla.jss.ssl.SSLSocket;
 import org.mozilla.jss.util.Password;
 
+import com.netscape.cmsutil.crypto.CryptoUtil;
 import com.netscape.cmsutil.util.Utils;
 
 /**
@@ -49,6 +52,7 @@ import com.netscape.cmsutil.util.Utils;
  */
 public class HttpClient {
     public static final String PR_INTERNAL_TOKEN_NAME = "internal";
+
     private String _host = null;
     private int _port = 0;
     private boolean _secure = false;
@@ -144,6 +148,32 @@ public class HttpClient {
                 SSLSocket.setSSLVersionRangeDefault(
                     org.mozilla.jss.ssl.SSLSocket.SSLProtocolVariant.DATA_GRAM,
                     datagram_range);
+
+                int ciphers[] = SSLSocket.getImplementedCipherSuites();
+                for (int j = 0; ciphers != null && j < ciphers.length; j++) {
+                    boolean enabled = SSLSocket.getCipherPreferenceDefault(ciphers[j]);
+                    //System.out.println("HttpClient Debug: cipher '0x" +
+                    //    Integer.toHexString(ciphers[j]) + "'" + " enabled? " +
+                    //    enabled);
+                    // make sure SSLv2 ciphers are not enabled
+                    if ((ciphers[j] & 0xfff0) ==0xff00) {
+                        if (enabled) {
+                            //System.out.println("HttpClient Debug: disabling SSL2 NSS Cipher '0x" +
+                            //    Integer.toHexString(ciphers[j]) + "'");
+                            SSLSocket.setCipherPreferenceDefault(ciphers[j], false);
+                        }
+                    } else {
+                        /*
+                         * unlike RSA ciphers, ECC ciphers are not enabled by default
+                         */
+                        if ((!enabled) && CryptoUtil.clientECCipherList.contains(ciphers[j])) {
+                          //System.out.println("Debug: enabling ECC NSS Cipher '0x" +
+                          //    Integer.toHexString(ciphers[j]) + "'");
+                          SSLSocket.setCipherPreferenceDefault(ciphers[j], true);
+                        }
+                    }
+                }
+
                 sslSocket = new SSLSocket(_host, _port);
                 // setSSLVersionRange needs to be exposed in jss
                 // sslSocket.setSSLVersionRange(org.mozilla.jss.ssl.SSLSocket.SSLVersionRange.tls1_0, org.mozilla.jss.ssl.SSLSocket.SSLVersionRange.tls1_2);
diff --git a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
index 3b1041a74bb4b663dd9c5f4c9fa983da133f04a3..f07922ceb459d9db02e7b72faedcd40a43b01f99 100644
--- a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
+++ b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
@@ -36,9 +36,12 @@ import java.security.cert.CertificateException;
 import java.security.interfaces.DSAParams;
 import java.security.interfaces.DSAPublicKey;
 import java.security.interfaces.RSAPublicKey;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Random;
 import java.util.StringTokenizer;
 import java.util.Vector;
@@ -122,6 +125,7 @@ import org.mozilla.jss.pkix.crmf.PKIArchiveOptions;
 import org.mozilla.jss.pkix.primitive.AlgorithmIdentifier;
 import org.mozilla.jss.pkix.primitive.Name;
 import org.mozilla.jss.pkix.primitive.SubjectPublicKeyInfo;
+import org.mozilla.jss.ssl.SSLSocket;
 import org.mozilla.jss.util.Base64OutputStream;
 import org.mozilla.jss.util.Password;
 
@@ -136,6 +140,17 @@ public class CryptoUtil {
     public static final String CERT_BEGIN_HEADING = "-----BEGIN CERTIFICATE-----";
     public static final String CERT_END_HEADING = "-----END CERTIFICATE-----";
 
+    static public final Integer[] clientECCiphers = {
+        SSLSocket.TLS_ECDH_RSA_WITH_AES_128_CBC_SHA,
+        SSLSocket.TLS_ECDH_RSA_WITH_AES_256_CBC_SHA,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
+        SSLSocket.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
+        SSLSocket.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
+    };
+    static public List<Integer> clientECCipherList = new ArrayList<Integer>(Arrays.asList(clientECCiphers));
+
     private static final String[] ecCurves = {
             "nistp256", "nistp384", "nistp521", "sect163k1", "nistk163", "sect163r1", "sect163r2",
             "nistb163", "sect193r1", "sect193r2", "sect233k1", "nistk233", "sect233r1", "nistb233", "sect239k1",
-- 
1.8.4.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]