1637488 - API incompatibility in 3.52

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[Mike Hommey [:glandium]]

AIUI 3.52 added PKCS#11 3.0 support while purposely breaking API compatibility, although there's an opt-out in the form of NSS_PKCS_2_0_COMPAT.

It feels to me like the API should preserve compatibility, or, if not, break building against the API in a visible way (as in, fail the build and make users choose between the old or the new explicitly). Here, we're in the middle, and the result is that rebuilding an application against a newer version of NSS suddenly yields a broken application. See comments in bug 1624128 and others bugs that were filed about the issue in Firefox, but technically speaking, this can presumably affect any other application that uses the PKCS#11 API. Even for Firefox, it also affects Firefox ESR for downstreams that build it against NSS.

I guess there might be a discussion to have whether Firefox should still support building against system NSS, but Firefox is only one user of NSS.

Comment 1

[Kevin Jacobs [:kjacobs]]

Without adding another define to acknowledge the changes (which, if missing, somehow breaks compilation), I'm not sure how we'd make this a build failure. It's a choice of the application to warn or error on uninitialized fields.

Bob: any ideas on either 1) how we might get the word out to Linux distros, or 2) some other programmatic solution?

Comment 2

[Robert Relyea]

First, there is not binary incompatibilty, applications compiled with the old headers work correctly.

The problem is that the PKCS #11 2.4 spec had an issue the definition of the GCM structure is different between the OASIS supplied header files and the text in the main spec. Under the OASIS rules the header files control. NSS took the structure from the spec. In PKCS #11 3.0 they reconciled in favor of the main spec.

To deal with this, I created 2 definitions of that header file: and NSS specific and the CK_NSS_GCM_PARAMS, and the PKCS #11 v3 definition: CK_GCM_PARAMS_V3. A #define (NSS_PKCS11_2_0_COMPAT) controls which definition CK_GCM_PARAMS takes on.

The new version has an additional field, which has to be set correctly, which is where applications that are compiled with the new header files, but have not been modified, run into issues. I really needed to include this in the release notes. I have a small patch that changes the sense of the #define (creating NSS_PKCS11_3_0_STRICT, which you set to get the new behavior).

PKCS #11 also creates a new better interface for using AES GCM which removes the need to access the PKCS #11 specific headers, and automatically handles access to tokens that may have been caught up in this change as well.

Upshot, we do need to move to the new structure (or the new api), very stable things can use the patch to reverse the sense, eventually everyone should move the the PK11_AEADOp interface which doesn't need the data structure.

bob

Comment 3

[Robert Relyea]

Here's what should have been in the release notes:
PKCS #11 2.40 had a mismatch between the SPEC and the released header file for CK_GCM_PARAMS. The latter is controlling. We created or header based on the former. In PKCS #11 v3.0 the reconciled this, but it left us with. The new (to NSS) definition has a new field ulIvBits, which must be set correctly.

To solve this, the NSS 3.52 headers has both definitions: CK_NSS_GCM_PARAMS is the original NSS definition and CK_GCM_PARAMS_V3 is the new (to NSS) definition. CK_GCM_PARAMS takes on one or the other based on the definition of NSS_PKCS11_2_0_COMPAT.

Upstream NSS_PKCS11_2_0_COMPAT is not defined. In RHEL we will definitely make NSS_PKCS11_2_0_COMPAT the default. We need to decide what to do in Fedora by default.

Applications can fix this the following ways:

option 1

#define NSS_PKCS11_2_0_COMPAT 1

or compile with -DNSS_PKCS11_2_0_COMPAT

your app will compile and run using current and older versions of NSS, but may break on newer tokens that use the new definition.

---

option 2

rename CK_GCM_PARAMS to CK_NSS_GCM_PARAMS (this will now require nss >= 3.52 to compile, but won't change based on the setting of NSS_PKCS11_2_0_COMPAT).

---

option 3

rename CK_GCM_PARAMS to CK_GCM_PARAMS_V3 and set ulIvBits to ulIvLen*8.

This will require nss >= 3.52 to compile and to run.

---

option 4

Move to PK11_AEADOp interface, which all requires nss >= 3.52 to compile and run, but it's less surprising and the dependency will be picked up automatically because you are using a new for 3.52 interface.

---

( also something for distros that are on stable platforms and what to keep the old behavior for the life of the release).

Comment 4

[Robert Relyea]

Attachment: nss-gcm-param-default-pkcs11v2.patch

Patch to reverse the sense of the #define. This is not for applying to the NSS respository, but to distribute to distros to allow phased in updates to their nss package in their stable branches.

Comment 5

[Robert Relyea]

( also something for distros that are on stable platforms and what to keep the old behavior for the life of the release):

For distributions that are picking up NSS into stable platforms, or wish to include a transition period, the following patch will reverse the sense of the NSS_PKCS11_2_0_COMPAT define by creating an NSS_PKCS11_3_0_STRICT define. All the options above are agnostic about what the sense of this define is, so distributions can create a landing space with the patch in which applications can safely change.

Comment 6

[Mike Hommey [:glandium]]

I, for one, think this should actually land. This doesn't preclude any of the options 2 to 4, which as I understand is what NSS users should be doing in the long run. Not doing any of the four options currently leads to something broken, which is not a great state to be in.