1624128 - Update CK_GCM_PARAMS uses for PKCS11 v3

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement)

Description

[Kevin Jacobs [:kjacobs]]

PKCS11 pre-v3 has a spec bug where CK_GCM_PARAMS contains a ulIvBits field in the header file, but not in the specification text. NSS has not included this field in the past, but PKCS11 v3 now requires it.

Bug 1603628 adds and validates this field*, so we need to update external uses of this struct to initialize appropriately.

*For third-party dependencies, we can define NSS_PKCS11_2_0_COMPAT prior to pulling in the includes. This gives the old definition, and NSS internally uses the size of the param to tell which struct definition was used by the caller.

This impacts some non-PSM code, but this seems like a logical place for the bug.

Comment 1

[Kevin Jacobs [:kjacobs]]

Attachment: Bug 1624128 - Update CK_GCM_PARAMS uses for PKCS11 v3.0 definition

This patch initializes the ulIvBits member of CK_GCM_PARAMS, which is new in PKCS11 v3.

For libprio, we instead define NSS_PKCS11_2_0_COMPAT, which yields the old struct definition.

Depends on D67229

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ed30e6b6de1
Update CK_GCM_PARAMS uses for PKCS11 v3.0 definition r=keeler

Comment 3

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/3ed30e6b6de1

Comment 4

[Kevin Jacobs [:kjacobs]]

Attachment: Bug 1624128 - Backed out changeset 3ed30e6b6de1 - Updating Fx76 to NSS 3.51.1

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/665137da70ee
Backed out changeset 3ed30e6b6de1 - Updating Fx76 to NSS 3.51.1 r=keeler

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/665137da70ee

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/463069687b3d
Update CK_GCM_PARAMS uses for PKCS11 v3.0 definition r=keeler

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/463069687b3d

Comment 9

[tt_1]

hey there, your patch caused some problems with webrtc I believe:

5:08.33 /usr/bin/armv7a-unknown-linux-gnueabihf-gcc -std=gnu99 -o Unified_c_netwerk_srtp_src0.o -c -flto -flifetime-dse=1 -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ff/dist/system_wrappers -include /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 '-DPACKAGE_STRING="libsrtp2 2.2.0-pre"' '-DPACKAGE_VERSION="2.2.0-pre"' -DHAVE_CONFIG_H=1 -DHAVE_STDLIB_H=1 -DHAVE_UINT8_T=1 -DHAVE_UINT16_T=1 -DHAVE_INT32_T=1 -DHAVE_UINT32_T=1 -DHAVE_UINT64_T=1 -DGCM=1 -DNSS=1 -DCPU_CISC=1 -DHAVE_NETINET_IN_H=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/netwerk/srtp/src -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ff/netwerk/srtp/src -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ff/ipc/ipdl/_ipdlheaders -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ipc/chromium/src -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ipc/glue -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/netwerk/srtp/src/crypto/include -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/netwerk/srtp/src/include -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ff/dist/include -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/nspr -I/usr/include/nss -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ff/dist/include/nss -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/pixman-1 -fPIC -include /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/ff/mozilla-config.h -DMOZILLA_CLIENT -pipe -mthumb -mno-thumb-interwork -mfpu=neon -mfloat-abi=hard -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -fPIC -pipe -O2 -fomit-frame-pointer -funwind-tables -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-security -Wformat-overflow=2 -std=gnu99 -MD -MP -MF .deps/Unified_c_netwerk_srtp_src0.o.pp -fdiagnostics-color Unified_c_netwerk_srtp_src0.c
5:08.33 In file included from Unified_c_netwerk_srtp_src0.c:2:
5:08.33 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/netwerk/srtp/src/crypto/cipher/aes_gcm_nss.c: In function 'srtp_aes_gcm_nss_do_crypto':
5:08.33 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-76.0.1/work/firefox-76.0.1/netwerk/srtp/src/crypto/cipher/aes_gcm_nss.c:274:15: error: 'CK_GCM_PARAMS' {aka 'struct CK_GCM_PARAMS'} has no member named 'ulIvBits'; did you mean 'ulTagBits'?
5:08.33 274 | c->params.ulIvBits = GCM_IV_LEN * 8;
5:08.33 | ^~~~~~~~
5:08.33 | ulTagBits

would you like to discuss this issue here, or in a new bug?

Comment 10

[Kevin Jacobs [:kjacobs]]

Please see the note added in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.52_release_notes (in Notable Changes). If you still think there's an issue, a new bug would be appropriate.

This is more likely a build/configuration issue causing Firefox to pull in a pre-3.52 version of NSS.

Comment 11

[tt_1]

I'm not sure what the underlying problem is, but I will talk with my distro's maintainer of nss to see what his opinion is.

Comment 12

[Kevin Jacobs [:kjacobs]]

I just noticed that your build log mentions Fx 76. This patch was backed out of 76 after the corresponding NSS change was pushed to 3.52 (Fx76 shipped with NSS 3.51.1). It re-landed in 77 with NSS 3.52.

I wonder if you're on an old commit (and possibly building with system NSS), since this patch is not in 76: https://hg.mozilla.org/mozilla-unified/file/e2de5f11bc0afd9a3024d32b83cb9f0ada95717a/netwerk/srtp/src/crypto/cipher

Comment 13

[tt_1]

You're correct, after half a day of chasing this I'm almost certain that my problems were caused by the maintainer of the firefox package, who backported the patchset without taking care of the additional nss dependency at compile time.

Comment 14

[Kevin Jacobs [:kjacobs]]

Thanks. Sorry you had to chase that down!

Comment 15

[Mike Hommey [:glandium]]

(In reply to Kevin Jacobs [:kjacobs] from comment #10)

Please see the note added in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.52_release_notes (in Notable Changes). If you still think there's an issue, a new bug would be appropriate.

This is more likely a build/configuration issue causing Firefox to pull in a pre-3.52 version of NSS.

There is nothing in Firefox that prevents building against a newer version of NSS. I suppose this means building any version of Firefox, including ESR, against NSS 3.52 breaks Firefox? What about non-Firefox? I suppose anything that builds against NSS 3.52 might be broken depending on their use of PKCS? It seems to me what needs adaptation here is NSS, not everything that uses it.

Comment 16

[Kevin Jacobs [:kjacobs]]

(In reply to Mike Hommey [:glandium] from comment #15)

(In reply to Kevin Jacobs [:kjacobs] from comment #10)

Please see the note added in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.52_release_notes (in Notable Changes). If you still think there's an issue, a new bug would be appropriate.

This is more likely a build/configuration issue causing Firefox to pull in a pre-3.52 version of NSS.

There is nothing in Firefox that prevents building against a newer version of NSS. I suppose this means building any version of Firefox, including ESR, against NSS 3.52 breaks Firefox? What about non-Firefox? I suppose anything that builds against NSS 3.52 might be broken depending on their use of PKCS? It seems to me what needs adaptation here is NSS, not everything that uses it.

Old binaries will continue to work with 3.52 (loaded dynamically) since 3.52 uses the size of the struct to tell which definition was used. The change impacts recompiling with newer versions of NSS, where NSS_PKCS11_2_0_COMPAT can be used to avoid any code changes.

Regardless, this bug is for the associated updates to Firefox. The NSS change happened in bug 1603628, and there is further discussion of this issue in D63241. Please file a new bug in NSS if you feel this needs to be revisited.