Created attachment 1056040 [details] pkcs10 object with unrecognised request extension Description of problem: When working with custom/exotic X.509 extensions, if NSS does not know about the oid, oid_dotted_decimal(ext) throws an error Version-Release number of selected component (if applicable): 0.16.0-1.fc22 How reproducible: Always Steps to Reproduce: >>> import nss.nss as nss >>> nss.nss_init_nodb() >>> with open('iecUserRoles.der') as f: ... der = f.read() ... >>> req = nss.CertificateRequest(der) >>> ext = req.extensions[0] >>> ext OID.1.2.840.10070.8.1 >>> ext.oid_tag 0 >>> nss.oid_dotted_decimal(ext.oid) Traceback (most recent call last): File "<stdin>", line 1, in <module> SystemError: error return without exception set (I have attached iecUserRoles.der, which contains the `iecUserRoles' extension defined in IEC 62351-8. Actual results: SystemError raised (see transcript, above). Expected results: Applying oid_dotted_decimal to extension with unknown OID should return a string like "OID.1.2.840.10070.8.1". Additional info: Blocking of https://fedorahosted.org/freeipa/ticket/4752
The problem is with the internal function get_oid_tag_from_object() and how it's called. It can return -1 for an error condition or SEC_OID_UNKNOWN (e.g. zero) if NSS is unable to identify the OID. Many of the callers of get_oid_tag_from_object() test for a -1 or SEC_OID_UNKNOWN result and return as if an exception had been set. However an exception is only set if -1 is returned, get_oid_tag_from_object() does not set an excpetion for the case of SEC_OID_UNKNOWN. So the question is should SEC_OID_UNKNOWN raise an exception? I tend to think not, I think SEC_OID_UNKNONWN should be treated as a valid value in most cases (probably depends on the context in which get_oid_tag_from_object() is called). get_oid_tag_from_object() is called in about a dozen places, I think I need to carefully evaluate each place where get_oid_tag_from_object() could return SEC_OID_UNKNOWN and decide if it should raise an exception or continue with the SEC_OID_UNKNOWN value.
There is one inconsistency in get_oid_tag_from_object() that needs fixing. This code: /* Get the OID tag from the SECItem */ if ((oid_tag = SECOID_FindOIDTag(&item)) == SEC_OID_UNKNOWN) { Sets an exception if the value is SEC_OID_UNKNOWN, it shouldn't it should just return SEC_OID_UNKNOWN to be consistent with the other code blocks which return SEC_OID_UNKNOWN without an exception.
I think returning SEC_OID_UNKNOWN instead of raising only gets us halfway there - the method is still trying to convert to an oid tag, then back into an OID string, which will not work for unrecognised OIDs. Perhaps oid_dotted_decimal should detect when the argument is a SecItem and if it is, go straight to converting it to dotted decimal. Something like: --- a/src/py_nss.c Thu Jul 09 12:41:28 2015 -0400 +++ b/src/py_nss.c Sun Jul 26 14:08:26 2015 +1000 @@ -22022,6 +22022,9 @@ if (!PyArg_ParseTuple(args, "O:oid_dotted_decimal", &arg)) return NULL; + if (PySecItem_Check(arg)) + return oid_secitem_to_pystr_dotted_decimal(&((SecItem *)arg)->item); + oid_tag = get_oid_tag_from_object(arg); if (oid_tag == SEC_OID_UNKNOWN || oid_tag == -1) { return NULL;
-1 means error occurred, SEC_OID_UNKNONW means exactly that. What this bug brought to light is handling these two values has not been done consistently and each place they referenced need to be examined to determine the proper action. Sometimes returning SEC_OID_UNKNOWN is permissible, but as you point out sometimes the tag value is fed into another function, in which case SEC_OID_UNKNOWN needs to be treated as an error raising an exception. The very specific case you uncovered needs to treat SEC_OID_UNKNOWN as an exception. The internal API contract is that if -1 is returned an exception is already set, but not for SEC_OID_UNKNOWN. In this specific case a return of SEC_OID_UNKOWN needs to set an exception.
Created attachment 1056288 [details] oid string conversion patch
Patch LGTM.
Fedora 22 changed to end-of-life (EOL) status on 2016-07-19. Fedora 22 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. If you are unable to reopen this bug, please file a new report against the current release. If you experience problems, please add a comment to this bug. Thank you for reporting this bug and we are sorry it could not be fixed.
This was fixed, my bad for not updating the bug.