search

dogtagpki / jss

Network Security Services for Java is a Java interface to NSS
https://dogtagpki.github.io/jss
19 stars 30 forks source link

Support ECGenParameterSpec in PK11KeyPairGenerator #226

Closed ZuluForce closed 5 years ago

ZuluForce commented 5 years ago

JSS, jss4.jar, is still built with JDK 1.4.2. While JDK 1.4.2 is EOL'd and all new product development should be using the latest JavaSE, legacy business products that must use JDK 1.4 or 1.5 can continue to add NSS/JSS security fixes/enhancements.

That quote is from https://www.dogtagpki.org/wiki/JSS and the warning on that page says it is sorely out of date. I am here questioning if Java 1.4 support is really still necessary.

Here's the problem I am running into right now. I am attempting to use JSS as the JCE provider for JSSE TLS 1.2 connections. I would much rather use an SSLEngine implementation backed by JSS which I know you're working on but I don't expect to be ready in a timeframe I need.

I have TLS connections functioning when the key agreement is DH[E] but not ECDH[E]. In ECDH the key agreement attempts to create an EC key using the ECGenParameterSpec class. I have no control to change this but it does throw an exception in JSS' PK11KeyPairGenerator because this parameter spec only exists in Java 1.5+.

Caused by: java.security.InvalidAlgorithmParameterException
    at org.mozilla.jss.pkcs11.PK11KeyPairGenerator.initialize(PK11KeyPairGenerator.java:439)
    at org.mozilla.jss.crypto.KeyPairGenerator.initialize(KeyPairGenerator.java:75)
    at org.mozilla.jss.provider.java.security.JSSKeyPairGeneratorSpi.initialize(JSSKeyPairGeneratorSpi.java:46)
    at org.mozilla.jss.provider.java.security.JSSKeyPairGeneratorSpi$EC.initialize(JSSKeyPairGeneratorSpi.java:71)
    at java.base/java.security.KeyPairGenerator$Delegate.initialize(KeyPairGenerator.java:699)
    at java.base/sun.security.ssl.ECDHKeyExchange$ECDHEPossession.<init>(ECDHKeyExchange.java:112)
    ... 39 more

The creation of JSS specific configuration classes may work for anyone using JSS directly but it's a terrible pattern if you want adoption in projects using standard frameworks or 3rd party code that expects a typical JCE contract. Is supporting Java 1.4 more important than supporting the framework users?

cipherboy commented 5 years ago

@ZuluForce \o welcome from a former Minnesota native.

That quote is really out of date, I've updated that: we're actual JDK 1.8+ compatible and actively test on JDK 1.8 and 11. I don't know if you're able to compile with previous JDKs (it sounds like you might've been able to do so), but you could try and force a later JDK to be compatible with earlier JDKs. See -target and -source options to the JDK. I'm not sure what version of the JDK you're trying to target in your product though.

I've had a couple of efforts to try and modernize JSS's interfaces; you're welcome to help out. I don't think we'll be able to wrangle org.mozilla.jss.ssl.SSLSocket / org.mozilla.jss.ssl.SSLServerSocket into the javax equivalent interfaces, but with a working SSLEngine implementation, this should be easy for someone to go and implement inside the org.mozilla.jss.ssl.javax namespace. Our main focus with the SSLEngine work is enabling TomcatJSS for Tomcat v8.5+ so I don't think we'll add SSLSocket support any time soon. We want a FIPS-certified SSL adapter for Tomcat that can use NSS DBs and HSMs as the backing certificate stores.

I've also been working on modernizing some of the PKCS#11-based parameters and specs. One common pattern I dislike about JSS is that there is significant overlap in functionality without actually implementing (or being compatible with) the javax.* or java.* interfaces. This is because JSS actually precedes Java 4's release. A lot of the javax.* interfaces we're used to were introduced in the Java 5's release.

Do you have sample code for triggering the exception above? If I have time, I'll try and take a look and at least provide pointers of what needs to be updated here.

I'd appreciate any testing you're willing to give on the SSLEngine work to know if that works for your use case. PR #196 should be used as it has one commit not yet in #150. This at least passes the tests I've written for it, but it isn't yet fully compatible with the Java expectations.

ZuluForce commented 5 years ago

@cipherboy Thanks for the quick reply. Yay Minnesota! :)

I'm glad to hear the project has moved on from strictly 1.4 support. We are compiling with JDK 11 and that works totally fine. My problem with 1.4 came from reading this in the PK11KeyGenerator initialize method:

        } else {
            Assert._assert( algorithm == KeyPairAlgorithm.EC);
            // requires JAVA 1.5
            // if(! (params instanceof ECParameterSpec) ) {
            //   throw new InvalidAlgorithmParameterException();
            //}
            // requires JAVA 1.5
            if(! (params instanceof PK11ParameterSpec) ) {
               throw new InvalidAlgorithmParameterException();
            }

Further down in the generateKeyPair method there's this:

            Assert._assert( algorithm == KeyPairAlgorithm.EC );
            // requires JAVA 1.5 for ECParameters.
            //
            //AlgorithmParameters ecParams =
            //          AlgorithmParameters.getInstance("ECParameters");
            // ecParams.init(params);
            PK11ParameterSpec ecParams = (PK11ParameterSpec) params;

These code comments along with the wiki notes made me think the only reason this hadn't changed was due to 1.4 support. I felt I was left with two options, build our own fork of JSS that supports either PK11ParameterSpec or ECParameterSpec, or fallback to a different JCE provider for key generation. I am working in a FIPS environment and that other provider in our case would be Bouncy Castle which is annoying for a few reasons. If JSS could be updated I would prefer that.

The project I'm using to play around with JSS uses Netty as the networking library and JSSE for the SSLEngine implementation. Right now the project has grown large enough that it's difficult for me to share all of it here. If time permits I will try to make a smaller reproduction. I think my current problem is simple with those two commented out parts above. The improvement may be as simple as commenting out those lines and making sure the code works with either type of param.

Regarding SSLSocket and SSLServerSocket. The application I'm working on does not use those classes but rather the transport agnostic SSLEngine with the Netty and Jetty libraries. I can try to help you test the SSLEngine you're working on, time permitting. My timeline is very tight and, no offense to the work you're doing, I don't know that the SSLEngine will be ready for prime time. I've been focusing on using JSS strictly as a JCE provider. I want to circle back and try the SSLEngine after this deadline (which is out of my control) because it would simplify our application.

cipherboy commented 5 years ago

Right, so for context on some of this.

The org.mozilla.jss.pkcs11 code is an interface over NSS's PKCS#11 library. (Roughly, anything that begins with a PK11_ prefix in the NSS headers -- it is definitely a C style library). To use any of these NSS function calls, your key needs to be owned by NSS (i.e., it needs to be of the right struct type and you now have a pointer to it).

This happens one of three ways in JSS: generate it (like you're trying to do), have a reference to it from elsewhere (e.g., from the NSS DB via the CryptoManager) or import it (what I'm working on in my bytes-symkey branch). The historical reason why these org.mozilla.jss.pkcs11.* classes are a bit.. obtuse is because JSS needs to own the pointer of the correct type, and that typically involves a *Proxy class (due to the JNI component here).

This isolation from the standard Java classes has, IMO, gotten a little out of hand. In places where we could "upcast" and import (especially symmetric keys that aren't typically stored in a NSS DB, such as a HMAC key or generator specifications), we don't and prefer to error out, because nobody implemented the import method yet.

That's what happened in the first section you posted: ECParameterSpec is a standard Java interface introduced in Java 1.5 (hence, the commented out section when Java 1.4 support was a priority). Rather than understanding the standard ECParameterSpec and converting that into a PK11ParameterSpec, historically the answer was to error out because we still needed to support Java 1.4. I think the "upcast" here is handled by the second area. If you want to open a PR fixing that, I'd be happy to review and merge it (if that's all we need to do). Else I'll get to it tomorrow.

I hope supporting some ECParameterSpecs in PK11KeyGenerator will go better than supporting it in ECPublicKey and ECPrivateKey interfaces. I'll take a look at what I can do. One of the benefits of the SSLEngine was that we could avoid the issues of the ECPublicKey/ECPrivateKey interfaces because we could mandate use of the standard JSS keys classes. However, I wouldn't be surprised if you can't easily use the keys that you generate without fixing that issues as well. ~sigh~

And yes, SSLEngine probably has its own set of problems and likely won't make your deadline. I'm working on NSS to fix another, higher-priority issue, so I might look to see if there is some work I can do to make my life easier implementing the SSLEngine. I've learned a few things about NSS that will definitely help me, but I'm not sure if that's enough to make the SSLEngine compliant with the JDK contract though.

(If you have a RHEL subscription somewhere and really want to see either SSLEngine and/or the ECPublicKey / ECPrivateKey issue fixed soon, I'd encourage you to file a customer support case so I can get this prioritized internally. However I too have a few deadlines upcoming for other, higher priority work so we'll have to see how it pans out as-is.)

ZuluForce commented 5 years ago

I also hope to test changes to the PK11KeyGenerator to support either parameter spec. If you can't do it today that's ok.

To use any of these NSS function calls, your key needs to be owned by NSS (i.e., it needs to be of the right struct type and you now have a pointer to it).

I've become very aware of that. The application being supported definitely does not play well with PKCS11 because it's traditionally always had access to key material. Without going into all the details on startup I am creating a session object for the private key (created by a SUN* JCE provider) by unwrapping it into NSS. The key used for [un]wrapping was generated within NSS. I then created my own KeyManagerFactory so that Jetty/Netty can be supplied this PK11*PrivateKey without it actually being stored on the PKCS11 token. I originally tried using the PKCS11 KeyStore but realized I would need to store the keys on the token which we're not doing.

When JSSE goes to create an X509Possession I've seen it acquire a signature algorithm from the JSSProvider and successfully use the previously "imported" PK11RsaPrivateKey. Since that test I went farther and tried to have JSS also do the key generation for the key agreement and that's where I am now.

I know what I'm doing is really a hodgepodge. My hope was that if all TLS key generation (either ephemeral DH or symmetric keys) were generated within NSS then any future ciphers/signatures/hmacs served by the JSSProvider would work. The one caveat being the private key for the server's own identity which I addressed by "importing" it as a session object.

I foresee one question is why I'm not using the SunPKCS11 provider. That's what I originally did but I could not get the unwrapping of a key into a session object to work, primarily because their cipher implementations don't support unwrap mode. The second reason is that I can't initialize NSS into a nodb (yet still FIPS level 1) mode with that provider as I've been able to with JSS. I got the idea from http://mozilla.6506.n7.nabble.com/NSS-NoDB-Init-quot-quot-and-FIPS-mode-td353030.html . JSS does call SECMOD_DeleteInternalModule() when enabling fips mode which has allowed me to get away without having a secmod.db or the key databases. I basically want to use NSS as a pure in-memory cryptographic provider in FIPS 140-2 level 1 mode while still having our application be the holder of key material.

ZuluForce commented 5 years ago

Here are the changes I made:

diff --git a/org/mozilla/jss/pkcs11/PK11KeyPairGenerator.java b/org/mozilla/jss/pkcs11/PK11KeyPairGenerator.java
index 6da0ad57..9932a52e 100644
--- a/org/mozilla/jss/pkcs11/PK11KeyPairGenerator.java
+++ b/org/mozilla/jss/pkcs11/PK11KeyPairGenerator.java
@@ -5,6 +5,7 @@
 package org.mozilla.jss.pkcs11;

 import java.math.BigInteger;
+import java.security.AlgorithmParameters;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidParameterException;
 import java.security.KeyPair;
@@ -12,6 +13,7 @@ import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.security.spec.AlgorithmParameterSpec;
 import java.security.spec.DSAParameterSpec;
+import java.security.spec.ECGenParameterSpec;
 import java.util.Hashtable;

 import org.mozilla.jss.asn1.ASN1Util;
@@ -430,15 +432,11 @@ public final class PK11KeyPairGenerator
             }
         } else {
             Assert._assert( algorithm == KeyPairAlgorithm.EC);
-            // requires JAVA 1.5
-            // if(! (params instanceof ECParameterSpec) ) {
-            //   throw new InvalidAlgorithmParameterException();
-            //}
-            // requires JAVA 1.5
-            if(! (params instanceof PK11ParameterSpec) ) {
-               throw new InvalidAlgorithmParameterException();
+
+            if (!(params instanceof PK11ParameterSpec || params instanceof ECGenParameterSpec)) {
+               throw new InvalidAlgorithmParameterException("Invalid EC parameters: " + params.getClass());
             }
-       } // future add support for X509EncodedSpec
+           } // future add support for X509EncodedSpec

         this.params = params;
     }
@@ -490,16 +488,24 @@ public final class PK11KeyPairGenerator
                 opFlags, opFlagsMask);
         } else {
             Assert._assert( algorithm == KeyPairAlgorithm.EC );
-            // requires JAVA 1.5 for ECParameters.
-            //
-            //AlgorithmParameters ecParams =
-               //                      AlgorithmParameters.getInstance("ECParameters");
-               // ecParams.init(params);
-            PK11ParameterSpec ecParams = (PK11ParameterSpec) params;
+            final byte[] encodedCurve;
+            if (params instanceof PK11ParameterSpec) {
+                encodedCurve = ((PK11ParameterSpec) params).getEncoded();
+            } else {
+                // Assumed to be ECParameterSpec based on cosntraints from initialize
+                try {
+                AlgorithmParameters ecParams = AlgorithmParameters.getInstance("EC");
+                ecParams.init(params);
+
+                encodedCurve = ecParams.getEncoded();
+                } catch (Exception e) {
+                    throw new RuntimeException(e);
+                }
+            }

             return generateECKeyPairWithOpFlags(
                 token,
-                       ecParams.getEncoded(), /* curve */
+                       encodedCurve,
                 temporaryPairMode,
                 sensitivePairMode,
                 extractablePairMode,

Excuse the blanket catch(Exception e). There's a lot of exceptions and I was just experimenting. This set of changes did allow an EC key pair to be created when called from JSSE. I did have to allow NSS to use the key databases since the key pair is generated as a token object and without the DB files I got this error.

Caused by: org.mozilla.jss.crypto.TokenRuntimeException: Keypair Generation failed on token with error: -8126 : 
    at org.mozilla.jss.provider.java.security.JSSKeyPairGeneratorSpi.generateKeyPair(JSSKeyPairGeneratorSpi.java:57)
    at org.mozilla.jss.provider.java.security.JSSKeyPairGeneratorSpi$EC.generateKeyPair(JSSKeyPairGeneratorSpi.java:71)
    at java.base/java.security.KeyPairGenerator$Delegate.generateKeyPair(KeyPairGenerator.java:728)
    at java.base/sun.security.ssl.ECDHKeyExchange$ECDHEPossession.<init>(ECDHKeyExchange.java:113)
    at java.base/sun.security.ssl.ECDHKeyExchange$ECDHEPossessionGenerator.createPossession(ECDHKeyExchange.java:231)
    at java.base/sun.security.ssl.SSLKeyExchange$T12KeyAgreement.createPossession(SSLKeyExchange.java:357)

That translates to SEC_ERROR_READ_ONLY from the NSS docs. Even after getting past all those issues I ran into the problem you predicted which is the lack of support for ECPublicKey and ECPrivateKey. Since https://pagure.io/jss/issue/32 does not seem like it's going to be resolved I will have to find another path.

I think you can close this ticket since the only reason I wanted to generate EC keys using standard JCE params was so it could be used by JSSE which doesn't work anyways. The patch for creating the keys worked so with some alteration to the exception handling you may want to take it. I don't have a proper JSS dev environment setup and I need to switch gears to find an alternative so I won't be providing a PR.

cipherboy commented 5 years ago

@ZuluForce I've actually been working on JSS pagure-issue#32 today. I have some progress towards the Public Key that I'll likely push soon. I don't know if I'll have it by EOD though.

What curves are you interested in using? The NIST p-{256,384,521} curves? The issue is I need to build a mapping across all representations of curves in order to.

I'm fine leaving this open. Its definitely something I want to work on and fix, hopefully before SSLEngine lands. It'll definitely help our existing TomcatJSS(E) adapter support EC keys.

ZuluForce commented 5 years ago

@cipherboy OK, awesome. The NIST p curves are enough for me right now.

cipherboy commented 5 years ago

Ok, so I stand by that pagure issue.

If that's workable, I'll implement PrivateKey that way tonight/tomorrow. Otherwise I'll probably start a discussion around NSS and getting the private values.

Ultimately, SSLEngine (and updating SSLSocket/SSLServerSocket) is the better way to go such that the data doesn't leave the NSS boundary.

ZuluForce commented 5 years ago

I do not know whether the private key is treated opaquely or if the the private values are accessed. I've only gotten to the point where I fail on public key problems. The only place I can find it being used for ECDHE is inside ECDHEPossession.getAgreedSecret:

        // called by ClientHandshaker with either the server's static or
        // ephemeral public key
        SecretKey getAgreedSecret(
                PublicKey peerPublicKey) throws SSLHandshakeException {

            try {
                KeyAgreement ka = JsseJce.getKeyAgreement("ECDH");
                ka.init(privateKey);
                ka.doPhase(peerPublicKey, true);
                return ka.generateSecret("TlsPremasterSecret");
            } catch (GeneralSecurityException e) {
                throw (SSLHandshakeException) new SSLHandshakeException(
                    "Could not generate secret").initCause(e);
            }
        }

So it treats it opaquely until the point it's passed to the key agreement implementation. This will not be a JSS/NSS implementation because the JSSProvider does not implement JCE key agreement. This is very likely to fail in other providers. For example with BouncyCastle I tracked what it will do with the private key above and it puts it into a ProvECPrivateKey whose constructor wants getS() to return a value.

    ProvECPrivateKey(
        Algorithm algorithm,
        ECPrivateKey key)
     {
         ECDomainParameters domainParameters = ECUtil.convertFromSpec(key.getParams());

         this.baseKey = new AsymmetricECPrivateKey(algorithm, domainParameters, key.getS());
     }

A working ECPublicKey implementation is definitely important as the key agreement requests a parameter spec from it, presumably to send to the client. Here's what that looks like from sun.security.ssl.ECDHServerKeyExchange in JSSE:

            publicKey = ecdhePossession.publicKey;
            ECParameterSpec params = publicKey.getParams();
            ECPoint point = publicKey.getW();
            publicPoint = JsseJce.encodePoint(point, params.getCurve());

            this.namedGroup = NamedGroup.valueOf(params)
cipherboy commented 5 years ago

By JCE Key Agreement, you mean KeyAgreement, correct? I'm not entirely sure if we have the parts to implement that easily, but I can take a look at that sometime.

Please take a look at PR #230 and PR #233 and see if that addresses the issues you have. If so, great! If not, it is likely due to either needing KeyAgreement out of the JSS Provider or an implementation of getS().

(I don't have anything yet to exercise the branches of #233 yet so I'd appreciate feedback on it; you might need to add more curves to ECCurve if you're not using the one I've added so far. You might find this, this, this, and/or this to be helpful when looking for information to add enum elements).

ZuluForce commented 5 years ago

By JCE Key Agreement, you mean KeyAgreement, correct?

Correct.

Please take a look at PR #230 and PR #233 and see if that addresses the issues you have. If so, great! If not, it is likely due to either needing KeyAgreement out of the JSS Provider or an implementation of getS().

@cipherboy Thanks for taking the time to put up those PRs. We have an internal fork (based off JSS 4.6) with changes to the build for our supported platforms. I will have to see if one of my colleagues can backport it so we can test it out.

I think there's a high likely hood it will fail within the KeyAgreement because of the need for private values. Because of that, last week I got TLS working using Bouncy Castle for the key agreement. The end of the key agreement ends up deriving a key that exists in NSS so a NSS AES cipher can be used for the encryption of the TLS connection. This is the part I was most concerned about because it's where the biggest slowdown could happen if we were forced to use BC.

My colleague who would backport it is busy so I don't know when we'd get around to testing it. I am moving forward since there's more issues to solve and we have something working.

cipherboy commented 5 years ago

ACK no problem. I'll see about KeyAgreement later then.

If I may ask, what changes to the build system? If they're reasonable to have upstream, I'm happy to review and merge. In v4.5.1 I replaced the build system with CMake. At the time I got a lot of "I don't knows" when asking about the old system -- this current one is at least better documented. :-)

c42-arthur commented 5 years ago

Hello chipherboy:

Here's what I needed to do to build on macOS:

From e03c0fc41955c150c3aab548c627899ad647e592 Mon Sep 17 00:00:00 2001
From: "arthur.ramsey" <arthur.ramsey@code42.com>
Date: Thu, 25 Jul 2019 23:59:44 -0500
Subject: [PATCH] Fix 1 of 2 for macOS and Windows CMake Error: Attempt to add
 a custom rule to output "build/lib/buffer.o.rule" which already has a custom
 rule.  This is caused by case insensitive file systems.

---
 org/mozilla/jss/ssl/javax/{buffer.c => buffer_.c} | 2 +-
 org/mozilla/jss/ssl/javax/{buffer.h => buffer_.h} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename org/mozilla/jss/ssl/javax/{buffer.c => buffer_.c} (99%)
 rename org/mozilla/jss/ssl/javax/{buffer.h => buffer_.h} (100%)

diff --git a/org/mozilla/jss/ssl/javax/buffer.c b/org/mozilla/jss/ssl/javax/buffer_.c
similarity index 99%
rename from org/mozilla/jss/ssl/javax/buffer.c
rename to org/mozilla/jss/ssl/javax/buffer_.c
index b7e6888f..1692df34 100644
--- a/org/mozilla/jss/ssl/javax/buffer.c
+++ b/org/mozilla/jss/ssl/javax/buffer_.c
@@ -1,4 +1,4 @@
-#include "buffer.h"
+#include "buffer_.h"

 #include <stdlib.h>
 #include <stdio.h>
diff --git a/org/mozilla/jss/ssl/javax/buffer.h b/org/mozilla/jss/ssl/javax/buffer_.h
similarity index 100%
rename from org/mozilla/jss/ssl/javax/buffer.h
rename to org/mozilla/jss/ssl/javax/buffer_.h
-- 
2.19.2

From 74d9cc9f29c6ebfb266a17a7a8d9c10a3c9e216a Mon Sep 17 00:00:00 2001
From: "arthur.ramsey" <arthur.ramsey@code42.com>
Date: Fri, 26 Jul 2019 00:00:43 -0500
Subject: [PATCH] Fix 2 of 2 for CMake Error: Attempt to add a custom rule to
 output "build/lib/buffer.o.rule" which already has a custom rule.

---
 org/mozilla/jss/nss/Buffer.c           | 2 +-
 org/mozilla/jss/nss/BufferProxy.h      | 2 +-
 org/mozilla/jss/ssl/javax/BufferPRFD.c | 2 +-
 org/mozilla/jss/ssl/javax/BufferPRFD.h | 2 +-
 org/mozilla/jss/tests/buffer_size_1.c  | 2 +-
 org/mozilla/jss/tests/buffer_size_4.c  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/org/mozilla/jss/nss/Buffer.c b/org/mozilla/jss/nss/Buffer.c
index 3229c572..f746086a 100644
--- a/org/mozilla/jss/nss/Buffer.c
+++ b/org/mozilla/jss/nss/Buffer.c
@@ -5,7 +5,7 @@

 #include "jssutil.h"
 #include "BufferProxy.h"
-#include "buffer.h"
+#include "buffer_.h"

 #include "_jni/org_mozilla_jss_nss_Buffer.h"

diff --git a/org/mozilla/jss/nss/BufferProxy.h b/org/mozilla/jss/nss/BufferProxy.h
index 989ab038..bad398d2 100644
--- a/org/mozilla/jss/nss/BufferProxy.h
+++ b/org/mozilla/jss/nss/BufferProxy.h
@@ -1,5 +1,5 @@
 #include <jni.h>
-#include "buffer.h"
+#include "buffer_.h"

 #pragma once

diff --git a/org/mozilla/jss/ssl/javax/BufferPRFD.c b/org/mozilla/jss/ssl/javax/BufferPRFD.c
index d6d224d5..4bdb606f 100644
--- a/org/mozilla/jss/ssl/javax/BufferPRFD.c
+++ b/org/mozilla/jss/ssl/javax/BufferPRFD.c
@@ -7,7 +7,7 @@
 #include <string.h>

 /* Ring buffer implementation to handle read/write calls. */
-#include "buffer.h"
+#include "buffer_.h"
 #include "BufferPRFD.h"

 /* This struct stores all the private data we need access to from inside our
diff --git a/org/mozilla/jss/ssl/javax/BufferPRFD.h b/org/mozilla/jss/ssl/javax/BufferPRFD.h
index 868ce31f..a35caf15 100644
--- a/org/mozilla/jss/ssl/javax/BufferPRFD.h
+++ b/org/mozilla/jss/ssl/javax/BufferPRFD.h
@@ -7,7 +7,7 @@
 #include <string.h>

 /* Ring buffer implementation to handle read/write calls. */
-#include "buffer.h"
+#include "buffer_.h"

 /* Use a modern pragma guard... */
 #pragma once
diff --git a/org/mozilla/jss/tests/buffer_size_1.c b/org/mozilla/jss/tests/buffer_size_1.c
index 3423d311..50c0144c 100644
--- a/org/mozilla/jss/tests/buffer_size_1.c
+++ b/org/mozilla/jss/tests/buffer_size_1.c
@@ -1,4 +1,4 @@
-#include "buffer.h"
+#include "buffer_.h"
 #include "assert.h"

 #include <stdio.h>
diff --git a/org/mozilla/jss/tests/buffer_size_4.c b/org/mozilla/jss/tests/buffer_size_4.c
index 6cdee74d..ae9176f3 100644
--- a/org/mozilla/jss/tests/buffer_size_4.c
+++ b/org/mozilla/jss/tests/buffer_size_4.c
@@ -1,4 +1,4 @@
-#include "buffer.h"
+#include "buffer_.h"
 #include "assert.h"

 #include <stdio.h>
-- 
2.19.2
From bc4c6315014c50f323d212bcb877937313a769fd Mon Sep 17 00:00:00 2001
From: "arthur.ramsey" <arthur.ramsey@code42.com>
Date: Fri, 26 Jul 2019 00:26:40 -0500
Subject: [PATCH] Disable build flags incompatible with macOS

---
 cmake/JSSConfig.cmake | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/cmake/JSSConfig.cmake b/cmake/JSSConfig.cmake
index f045850a..47c4ca84 100644
--- a/cmake/JSSConfig.cmake
+++ b/cmake/JSSConfig.cmake
@@ -175,12 +175,14 @@ macro(jss_config_ldflags)
     list(APPEND JSS_LD_FLAGS "-ldl")

     # This set of flags is specific to building the libjss library.
+    list(APPEND JSS_LD_FLAGS "-L/opt/nss/lib")
+
     list(APPEND JSS_LIBRARY_FLAGS "-shared")
-    list(APPEND JSS_LIBRARY_FLAGS "-Wl,-z,defs")
-    list(APPEND JSS_LIBRARY_FLAGS "-Wl,-soname")
-    list(APPEND JSS_LIBRARY_FLAGS "-Wl,${JSS_SO}")
+    # list(APPEND JSS_LIBRARY_FLAGS "-Wl,-z,defs")
+    # list(APPEND JSS_LIBRARY_FLAGS "-Wl,-soname")
+    # list(APPEND JSS_LIBRARY_FLAGS "-Wl,${JSS_SO}")

-    set(JSS_VERSION_SCRIPT "-Wl,--version-script,${PROJECT_SOURCE_DIR}/lib/jss.map")
+    # set(JSS_VERSION_SCRIPT "-Wl,--version-script,${PROJECT_SOURCE_DIR}/lib/jss.map")
 endmacro()

 macro(jss_config_java)
-- 
2.19.2

Thanks, Arthur

c42-arthur commented 5 years ago

For Windows I also needed the previous patches, the following patch and additionally modify generated VC project files:

diff --git a/cmake/JSSConfig.cmake b/cmake/JSSConfig.cmake index e51c4a8a..0529785c 100644 --- a/cmake/JSSConfig.cmake +++ b/cmake/JSSConfig.cmake @@ -164,20 +164,20 @@ macro(jss_config_ldflags)

This list of C linker flags was taken from the original build scripts

 # for debug and release builds. We lack a "check_c_linker_flag" macro,
 # so no effort is made to validate these flags.

+#if defined(_WIN32) || defined(_WIN64) +# if defined(_WIN64)

cipherboy commented 5 years ago

@c42-arthur Cool! I didn't know buffer.h was an issue on MacOS, so if you don't mind the noise, I can fix that upstream for you. That's code that's eventually needed for the open SSLEngine work, and since that isn't yet done, you shouldn't be actively affected by the move (outside of the rebase issues).

I probably wouldn't merge most of the Windows changes though as its really hard for me to support without CI (and I lack a windows development environment in case I break anything).

If you have any thoughts for MacOS CI, I'm happy to hear suggestions so I don't accidentally break your stuff in the future. Was my assertion correct in #72 that Windows on the old build system (e.g., what is still used on the v4.4.x branch) was broken? Or was that actually mostly fine?

c42-arthur commented 5 years ago

I can fix that upstream for you

I'd appreciate it.

Was my assertion correct in #72

I didn't try building v4.4.x on Windows.

I probably wouldn't merge most of the Windows changes though as its really hard for me to support without CI

No problem, if you could just merge the C fixes that'd be nice. It would take a fair bit more work to support Windows, Linux and macOS build using the same CMake build scripts.

As far as CI, unfortunately my employer doesn't have any public CI resources.

cipherboy commented 5 years ago

I'd appreciate it.

ACK, will open a PR for that and ping you on it.

I didn't try building v4.4.x on Windows.

I don't know when you started using JSS. :) If you used any version older than v4.5.1, you would've used the old build system (v4.5.0 for example, along with the betas while it transitioned from a Mozilla project to a Dogtag project. Or v4.4.x branch as I don't have plans to modify the build system there).

As far as CI, unfortunately my employer doesn't have any public CI resources.

Ah that's fine. I wasn't sure if you were familiar with anything free and public that did MacOS builds. No problem though. :)

c42-arthur commented 5 years ago

We just started a PoC using JSS. I can test building v4.4.x on Windows if it would be helpful.

cipherboy commented 5 years ago

OK, I'm marking this particular issue as closed since #233 is closed. Please refer to the comment if you have issues on older versions of NSS.

I'll open another issue to address adding KeyAgreement to the JSS Provider.

cipherboy commented 4 years ago

@c42-arthur I've merged PR #313 which might help with Java support on Windows. It lets you use a single javac command by passing the files using @argfile syntax so the command length stays short. It sounded like your patch used multiple calls to javac, which'd also work but might be a slower. YMMV.