1321350 - Remove all JSS bypassPKCS11 functions as latest NSS has removed PKCS#11 bypass support

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect, P1)

Description

[Christina Fu]

This bug spinned off 1313122
See comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1313122#c28
In suggestion "1"
We should remove all JSS bypassPKCS11 functions as latest NSS has removed PKCS#11 bypass support; it will not be made a blocker

Comment 1

[Christina Fu]

Note: this bug depends on 1313122 as it contains caller code that would not compile if these functions are removed.

Comment 2

[Christina Fu]

Attachment: RemoveBypassPK11Funcs.patch

JSS library patch to remove reference calls to now-obsolete PKCS#11 bypass functions.

Comment 3

[jmagne]

Comment on attachment 8817301 [details] [diff] [review]
RemoveBypassPK11Funcs.patch

Review of attachment 8817301 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 4

[Elio Maldonado]

Comment on attachment 8817301 [details] [diff] [review]
RemoveBypassPK11Funcs.patch

Review of attachment 8817301 [details] [diff] [review]:
-----------------------------------------------------------------

r+ from me also, checked it on Rawhide and macOS.

Comment 5

[Carsten Book [:Tomcat]]

needs rebasing at least for mozilla-inbound 

applying RemoveBypassPK11Funcs.patch
unable to find 'org/mozilla/jss/ssl/SSLServerSocket.java' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file org/mozilla/jss/ssl/SSLServerSocket.java.rej
unable to find 'org/mozilla/jss/ssl/SSLSocket.java' for patching
(use '--prefix' to apply patch relative to the current directory)
2 out of 2 hunks FAILED -- saving rejects to file org/mozilla/jss/ssl/SSLSocket.java.rej
unable to find 'org/mozilla/jss/ssl/SocketBase.java' for patching
(use '--prefix' to apply patch relative to the current directory)
3 out of 3 hunks FAILED -- saving rejects to file org/mozilla/jss/ssl/SocketBase.java.rej
unable to find 'org/mozilla/jss/ssl/common.c' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file org/mozilla/jss/ssl/common.c.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh RemoveBypassPK11Funcs.patch

Comment 6

[Elio Maldonado]

(In reply to Carsten Book [:Tomcat] from comment #5)
> needs rebasing at least for mozilla-inbound 

Apologies for my ignorance what what's "rebasing at least for mozilla-inbound"?

As far I know Mozilla products do not use JSS.

> 
> applying RemoveBypassPK11Funcs.patch
> unable to find 'org/mozilla/jss/ssl/SSLServerSocket.java' for patching
> (use '--prefix' to apply patch relative to the current directory)
> 1 out of 1 hunks FAILED -- saving rejects to file
> org/mozilla/jss/ssl/SSLServerSocket.java.rej
> unable to find 'org/mozilla/jss/ssl/SSLSocket.java' for patching
> (use '--prefix' to apply patch relative to the current directory)
> 2 out of 2 hunks FAILED -- saving rejects to file
> org/mozilla/jss/ssl/SSLSocket.java.rej
> unable to find 'org/mozilla/jss/ssl/SocketBase.java' for patching
> (use '--prefix' to apply patch relative to the current directory)
> 3 out of 3 hunks FAILED -- saving rejects to file
> org/mozilla/jss/ssl/SocketBase.java.rej
> unable to find 'org/mozilla/jss/ssl/common.c' for patching
> (use '--prefix' to apply patch relative to the current directory)
> 1 out of 1 hunks FAILED -- saving rejects to file
> org/mozilla/jss/ssl/common.c.rej
> patch failed, unable to continue (try -v)
> patch failed, rejects left in working directory
> errors during apply, please fix and qrefresh RemoveBypassPK11Funcs.patch

Did you follow the the steps in the README? They worked for Jack and me and we tried them on several platforms.

Comment 7

[Kai Engert (:KaiE:)]

Hello Carsten, I think you can ignore this patch, it isn't intended for Firefox/Mozilla at all, but only for a separate project/repository.

Comment 8

[Elio Maldonado]

pushed: https://hg.mozilla.org/projects/jss/rev/1a96a08e6f3d7553fda15bfa1069eb6a740a1272

Comment 9

[mmcclain]

BTW, because this change changes constants (which Javac inlines), code that
is compiled against 2097:1a96a08e6f3d will throw an exception if run against
code running JSS 4.3.2 and
sslSocket.enableRenegotiation(SSLSocket.SSL_RENEGOTIATE_NEVER) is issued.

New value is 23.  23 used to be SSL_ENABLE_RENEGOTIATION.

Alternatively, code that was compiled against version 4.3.2, but run against
2097:1a96a08e6f3d will set SSL_RENEGOTIATE_UNRESTRICTED when
SSL_RENEGOTIATE_NEVER was intended.

Caused by: java.net.SocketException: Incorrect input value.
	at org.mozilla.jss.ssl.SSLServerSocket.enableRenegotiation(SSLServerSocket.java:428)

Caused by: java.net.SocketException: Incorrect input value.
	at org.mozilla.jss.ssl.SSLSocket.enableRenegotiation(SSLSocket.java:617)

Comment 10

[mmcclain]

Attachment: backward-compatible-constants.patch

Patch to keep constants backward compatible with previous versions of JSS.

Comment 11

[Elio Maldonado]

(In reply to mmcclain from comment #10)
> Created attachment 8830729 [details] [diff] [review]
> backward-compatible-constants.patch
> 
> Patch to keep constants backward compatible with previous versions of JSS.

Thank you Mathew for the patch. Notice this comment in common.c.
/*
 * These must match up with the constants defined in SocketBase.java.
 * Note to developer these constants are not all related! i.e. you cannot
 * pass in PR_SHUTDOWN_RCV to setSSLOption etc! Check their usage
 * in NSS and NSPR before using.
 */
PRInt32 JSSL_enums[] = {
and on SocketBase.java there are similar ones
    /**
     * Enums. These must match the enums table in common.c. This is
     * safer than copying the values of the C constants, which are subject
     * to change, into Java code.
     * Note to developer these constants are not all related! i.e. you cannot
     * pass in PR_SHUTDOWN_RCV to setSSLOption etc! Check their usage
     * in NSS and NSPR before using.
     */
Given that then along with common.c the SocketBase.java would need to be patched. This is something that we have discussed in the past and opted not to change it. We will revisit this topic. 

I will review and test your patch(es) adapted to to latest sources in that NSS_EXPERIMENTAL BRANCH so I can take advantage of the preferred way of building and testing from Bug 1331765. If needed I will modify the patches for bugs listed in the tracking bug 1307859 as needed.  I'll get back to you in a few days.  

Thanks again.

Elio

Comment 12

[mmcclain]

The patch I attached modified both common.c and SocketBase.java.

The purpose was so Java code compiled against the old source (JSS 4.3.2) would continue to work without recompilation with the new source as long as it didn't use the removed option (SSL_BYPASS_PKCS11) or methods (bypassPKCS11, bypassPKCS11Default).

Comment 13

[Elio Maldonado]

My bad, I didn't read carefully when looking at it via Splinter Review.

Comment 14

[Christina Fu]

Comment on attachment 8830729 [details] [diff] [review]
backward-compatible-constants.patch

Review of attachment 8830729 [details] [diff] [review]:
-----------------------------------------------------------------

looks good.  Thank you!

Comment 15

[Elio Maldonado]

Attachment: tests results using this patch

Applied this patch, trivially modified on account of latest sources, and following the steps documented in Bug 1331765 and all tests passed.

Comment 16

[Elio Maldonado]

Pushed: https://hg.mozilla.org/projects/jss/rev/e5e2f877029d5c7f4beaa99f43b269f986593fad