1714909 - [python-nss] Bugfix for NSS 3.58 class naming

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Alexander Scheel]

Attachment: 0001-Update-version-number-to-match-current-version.patch

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:88.0) Gecko/20100101 Firefox/88.0

Steps to reproduce:

This patch is for python-nss, not NSS. Per email discussion with Martin Thomson, filing this bug against NSS so the correct code owner for python-nss can be identified and these changes committed.

Note that John Dennis (former code owner) has retired and is no longer employed at Red Hat. At the time of my leaving, Red Hat no replacement active maintainer for python-nss (other than myself). I never asked for commit permissions as I wasn't making active changes. These patches were pushed into Fedora by myself (as Fedora & RHEL maintainer of python-nss). As of this time, (from Dogtag v10.10+ IIUC), Dogtag PKI has dropped its dependency on python-nss and no longer requires it. It is likely this package will be orphaned in Fedora (if it hasn't already -- need to check).

So, I'm curious if anyone else (especially at Mozilla) is using this package and wishes to maintain it.

When compiling against newer NSS versions (as of 3.58+), NSS renamed some classes, causing conflicts with classes in python-nss. This is understandable as the Python class names were fairly generic.

Two patches:

1. Fix up a version number that seemed incorrect to align with version number elsewhere.
2. Fix up class names preventing compilation on newer NSS versions.

Both of these should be fairly easy. There shouldn't be an impact form the Python change as this was in the C FFI layer and shouldn't be exposed to users of the library IIRC.

Comment 1

[Alexander Scheel]

Attachment: 0002-Rename-DSA-RSA-PublicKey-to-Py-DSA-RSA-PublicKey.patch

Second patch; squash of 51343db2e865b05054f0204dc6116a5633c671c7 and d8151c7929f604f66ef1342e2aa5758df1c9f8d9 from https://github.com/cipherboy/python-nss/tree/dev.

Comment 2

[Martin Thomson [:mt:]]

Both of these are r+ from me. I don't know whether this is something I can just land. I guess that I could just try.

Comment 3

[Martin Thomson [:mt:]]

Attachment: updated.patch

These are the changes I've made and am preparing to (try to) push. It's got both patches in the one file. Alex, I've changed your email address, is that OK? I had to resolve conflicts with the first one for some unknown reason, so I'd appreciate a sanity check.

Comment 4

[Marcin Cieślak (:saper)]

I haven't seen those patches and I have re-done them myself, incidentally came up with the same name for the classes :)

But I have further fixes, especially in the PKCS#12 area, including fix for a crash:

https://ylog.eu/tmp/python-nss.patch

I have asked about the current state of play on the dev-crypo-tech newsgroup https://groups.google.com/a/mozilla.org/g/dev-tech-crypto/c/L0eP6vHFJYY

Comment 5

[Marcin Cieślak (:saper)]

Comment on attachment 9234351 [details] [diff] [review]
updated.patch

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

In line 7274 probably a copy+paste error sneaked in

::: src/py_nss.c
@@ +7270,5 @@
>      0,						/* tp_getattro */
>      0,						/* tp_setattro */
>      0,						/* tp_as_buffer */
>      Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,	/* tp_flags */
> +    PyRSAPublicKey_d+    PyRSAPublicKey_doc,				/* tp_doc */

Just "PyRSAPublicKey_doc,				/* tp_doc */" there. 

Will not compile:

src/py_nss.c:7274:5: error: use of undeclared identifier 'PyRSAPublicKey_d'; did you mean 'PyRSAPublicKey_doc'?
    PyRSAPublicKey_d+    PyRSAPublicKey_doc,                            /* tp_doc */
    ^~~~~~~~~~~~~~~~
    PyRSAPublicKey_doc
src/py_nss.c:7242:14: note: 'PyRSAPublicKey_doc' declared here

Comment 6

[Alexander Scheel]

I agree with Marcin above; looks like a copy/paste error not present in the original patch. Otherwise, LGTM.

Comment 7

[Martin Thomson [:mt:]]

Seems like I have the power to push to that repo, so...

https://hg.mozilla.org/projects/python-nss/rev/4dcf09e100f77d326c61e1a5b1ce1873ab014193
https://hg.mozilla.org/projects/python-nss/rev/9de14a6f77e2586269e91f770ca7f7b95282945d

Thanks for catching the error.

I don't have any ability to publish anything, so I'll rely on others to guide me as to what happens next. But as far as this bug is concerned, we're good.

Comment 8

[Marcin Cieślak (:saper)]

Thank you Martin for taking care of this!

I think we should cut a quick release. Do you know how could I get access to the repository? There was no one in the last 4 years who could commit there and Red Hat people I have emailed recently back in December only promise to "talk back to their manager" about it and I never heard anything since.

Old maintainer was kind enough to give me access to PyPI so we could publish a new release there.

Should I just file a bug with Mozilla to ask for a commit access?

Comment 9

[Martin Thomson [:mt:]]

Regarding the changes in comment 4, I would start by opening bugs for the individual changes. I'd prefer to work through those before we talk about commit access. What I'm seeing in the patch there looks fine, but we should try to follow some sort of process for managing changes.