Skip Menu |

Subject: PKINIT use of OpenSSL OID table is not thread-safe or application-friendly
Download (untitled) / with headers
text/plain 2.7KiB
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

* 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.
It appears that PKINIT is shadowing NID_pkcs7_data so that
create_contentinfo can call PKCS7_set0_type_other regardless of whether
the type is NID_pkcs7_data or an "other" type.

If we eliminate the shadowing and don't change anything else,
create_contentinfo creates an inconsistent PKCS7 object, with type set to
id-pkcs7-data but with the data in d.other instead of When this
inconsistent PKCS7 is ASN.1 encoded, it contains four bytes of garbage
instead of the intended data.

This is easy enough to fix; create_contentinfo just needs to special-case
the id-pkcs7-data OID and set up the PKCS7 object correctly. This can be
done using PKCS7_set_type or by filling in by hand.
Subject: git commit

Stop shadowing id-pkcs7-data OID

pkinit_crypto_openssl.c currently creates a shadow entry for
id-pkcs7-data so that OpenSSL will expect to see the corresponding
octet string in d.other instead than This shadowing is very
unfriendly to other uses of OpenSSL and we should stop. Eliminate the
shadowing and rewrite create_contentinfo so that it sets up the PKCS7
object correctly if the OID is id-pkcs7-data.
Author: Greg Hudson <>
Commit: 8ee1790ba6e3468d7ed53ed46123dc9545a4216f
Branch: master
src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 116 ++++++++------------
src/plugins/preauth/pkinit/pkinit_crypto_openssl.h | 1 -
2 files changed, 45 insertions(+), 72 deletions(-)
Subject: git commit

Use anonymous OIDs in pkinit_crypto_openssl.c

Stop adding OIDs to the global OpenSSL table. It isn't thread-safe
(even with locking callbacks registered), and calling OBJ_cleanup
could break other uses of OpenSSL. Instead, use anonymous OIDs
created with OBJ_txt2oid. Anonymous OIDs need to be managed more
careful to avoid double-freeing, so create a copy before calling
PKCS7_add_signed_attribute, and don't free the result of
pkinit_pkcs7type2oid in cms_contentinfo_create.
Author: Greg Hudson <>
Commit: 6b9e570a7e98470b806a26c5119e53b2145e2586
Branch: master
src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 96 +++++++++-----------
1 files changed, 43 insertions(+), 53 deletions(-)