[krbdev.mit.edu #7889] PKINIT use of OpenSSL OID table is not thread-safe or application-friendly

[Greg Hudson via RT]

OpenSSL contains two global internal tables which map between object 
identifiers and integer identifiers (called NIDs) for application 
convenience.  (These tables also map OIDs to "short names" and "long 
names" but that is unimportant to this bug.)  The first table is 
immutable and maps frequently used OIDs to to API constants such as 
NID_sha1.  The second table is a mutable and can be populated by callers 
using OBJ_create or OBJ_add_object.  Query functions such as OBJ_nid2obj 
and OBJ_sn2nid consult the mutable table first, then the immutable 
table.  OBJ_cleanup frees and resets the mutable table.  As of this 
writing, the mutable table does not appear to be protected by any mutex 
calls, even if mutex callbacks are registered by the OpenSSL caller.

On initialization, PKINIT adds nine PKINIT-related OIDs to the mutable 
table if they don't have pre-existing mappings.  On cleanup, PKINIT 
calls OBJ_cleanup.  A global counter in the PKINIT code (not protected 
by any mutex) tries to prevent PKINIT from removing its own OIDs from 
the mutable table when another context has the PKINIT module loaded.

In addition, pkinit_pkcs7type2oid registers 1.2.840.113549.1.7.1, aka 
id-data from RFC 3369.  Comments indicate that that the goal is to 
intentionally prevent OpenSSL from recognizing the CMS id-data OID.

There are three big problems here:

1. There are obvious thread-safety issues.  PKINIT does not protect its 
counter and OpenSSL does not protect its mutable table.

2. The OBJ_cleanup call will throw away any mutable table entries 
created by other callers of OpenSSL.

3. The shadow entry for id-data could interfere with other callers of 
OpenSSL.  In fact, the registration is explicitly deferred to avoid 
breaking "things like the use of pkcs12" within PKINIT itself.

To solve these issues, we should avoid using the mutable table.  The 
work for this breaks down as follows:

* Six of the OIDs are only used to compare against OIDs from received 
messages or certificates.  These can be created as anonymous OIDs with 
OBJ_txt2oid and freed with ASN1_OBJECT_free.

* The other four OIDs (including the shadowed one) are converted to NID 
and passed to PKCS7_set0_type_other, which converts the NID back to an 
ASN1_OBJECT pointer and sets the PKCS7 handle's "type" field to that.  
Converting to NID and back won't work with an anonymous OID.  Since the 
PKCS7 structure is public, we could avoid using PKCS7_set0_type_other 
and just do it ourselves, using a duplicate of an anonymous OID for the 
type.

* We have to stop shadowing the CMS id-data OID.  More analysis is 
required here.  Unfortunately, the need to prevent OpenSSL from 
recognizing id-data relates to draft9 compatibility, which is not 
covered by automated tests.