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.

ticket: 7889

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index 5732064bf6d3bf2191809e0b80fc42a95b3719d2..02378134f08068bd0853643239f9c6d577544ed6 100644 (file)
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -423,8 +423,6 @@ unsigned char pkinit_4096_dhprime[4096/8] = {
     0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
 };
 
-static int pkinit_oids_refs = 0;
-
 krb5_error_code
 pkinit_init_plg_crypto(pkinit_plg_crypto_context *cryptoctx)
 {
@@ -561,57 +559,43 @@ pkinit_fini_req_crypto(pkinit_req_crypto_context req_cryptoctx)
 static krb5_error_code
 pkinit_init_pkinit_oids(pkinit_plg_crypto_context ctx)
 {
-    krb5_error_code retval = ENOMEM;
-    int nid = 0;
-
-    /*
-     * If OpenSSL already knows about the OID, use the
-     * existing definition. Otherwise, create an OID object.
-     */
-#define CREATE_OBJ_IF_NEEDED(oid, vn, sn, ln)                           \
-    nid = OBJ_txt2nid(oid);                                             \
-    if (nid == NID_undef) {                                             \
-        nid = OBJ_create(oid, sn, ln);                                  \
-        if (nid == NID_undef) {                                         \
-            pkiDebug("Error creating oid object for '%s'\n", oid);      \
-            goto out;                                                   \
-        }                                                               \
-    }                                                                   \
-    ctx->vn = OBJ_nid2obj(nid);
-
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.2.2", id_pkinit_san,
-                         "id-pkinit-san", "KRB5PrincipalName");
-
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.2.3.1", id_pkinit_authData,
-                         "id-pkinit-authdata", "PKINIT signedAuthPack");
+    ctx->id_pkinit_san = OBJ_txt2obj("1.3.6.1.5.2.2", 1);
+    if (ctx->id_pkinit_san == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.2.3.2", id_pkinit_DHKeyData,
-                         "id-pkinit-DHKeyData", "PKINIT dhSignedData");
+    ctx->id_pkinit_authData = OBJ_txt2obj("1.3.6.1.5.2.3.1", 1);
+    if (ctx->id_pkinit_authData == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.2.3.3", id_pkinit_rkeyData,
-                         "id-pkinit-rkeyData", "PKINIT encKeyPack");
+    ctx->id_pkinit_DHKeyData = OBJ_txt2obj("1.3.6.1.5.2.3.2", 1);
+    if (ctx->id_pkinit_DHKeyData == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.2.3.4", id_pkinit_KPClientAuth,
-                         "id-pkinit-KPClientAuth", "PKINIT Client EKU");
+    ctx->id_pkinit_rkeyData = OBJ_txt2obj("1.3.6.1.5.2.3.3", 1);
+    if (ctx->id_pkinit_rkeyData == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.2.3.5", id_pkinit_KPKdc,
-                         "id-pkinit-KPKdc", "KDC EKU");
+    ctx->id_pkinit_KPClientAuth = OBJ_txt2obj("1.3.6.1.5.2.3.4", 1);
+    if (ctx->id_pkinit_KPClientAuth == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.4.1.311.20.2.2", id_ms_kp_sc_logon,
-                         "id-ms-kp-sc-logon EKU", "Microsoft SmartCard Login EKU");
+    ctx->id_pkinit_KPKdc = OBJ_txt2obj("1.3.6.1.5.2.3.5", 1);
+    if (ctx->id_pkinit_KPKdc == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.4.1.311.20.2.3", id_ms_san_upn,
-                         "id-ms-san-upn", "Microsoft Universal Principal Name");
+    ctx->id_ms_kp_sc_logon = OBJ_txt2obj("1.3.6.1.4.1.311.20.2.2", 1);
+    if (ctx->id_ms_kp_sc_logon == NULL)
+        return ENOMEM;
 
-    CREATE_OBJ_IF_NEEDED("1.3.6.1.5.5.7.3.1", id_kp_serverAuth,
-                         "id-kp-serverAuth EKU", "Server Authentication EKU");
+    ctx->id_ms_san_upn = OBJ_txt2obj("1.3.6.1.4.1.311.20.2.3", 1);
+    if (ctx->id_ms_san_upn == NULL)
+        return ENOMEM;
 
-    /* Success */
-    retval = 0;
-    pkinit_oids_refs++;
+    ctx->id_kp_serverAuth = OBJ_txt2obj("1.3.6.1.5.5.7.3.1", 1);
+    if (ctx->id_kp_serverAuth == NULL)
+        return ENOMEM;
 
-out:
-    return retval;
+    return 0;
 }
 
 static krb5_error_code
@@ -757,10 +741,15 @@ pkinit_fini_pkinit_oids(pkinit_plg_crypto_context ctx)
 {
     if (ctx == NULL)
         return;
-
-    /* Only call OBJ_cleanup once! */
-    if (--pkinit_oids_refs == 0)
-        OBJ_cleanup();
+    ASN1_OBJECT_free(ctx->id_pkinit_san);
+    ASN1_OBJECT_free(ctx->id_pkinit_authData);
+    ASN1_OBJECT_free(ctx->id_pkinit_DHKeyData);
+    ASN1_OBJECT_free(ctx->id_pkinit_rkeyData);
+    ASN1_OBJECT_free(ctx->id_pkinit_KPClientAuth);
+    ASN1_OBJECT_free(ctx->id_pkinit_KPKdc);
+    ASN1_OBJECT_free(ctx->id_ms_kp_sc_logon);
+    ASN1_OBJECT_free(ctx->id_ms_san_upn);
+    ASN1_OBJECT_free(ctx->id_kp_serverAuth);
 }
 
 static krb5_error_code
@@ -979,7 +968,7 @@ cms_contentinfo_create(krb5_context context,                          /* IN */
                        unsigned char **out_data, unsigned int *out_data_len)
 {
     krb5_error_code retval = ENOMEM;
-    ASN1_OBJECT *oid = NULL;
+    ASN1_OBJECT *oid;
     PKCS7 *p7 = NULL;
     unsigned char *p;
 
@@ -1017,8 +1006,6 @@ cms_contentinfo_create(krb5_context context,                          /* IN */
 cleanup:
     if (p7)
         PKCS7_free(p7);
-    if (oid)
-        ASN1_OBJECT_free(oid);
     return retval;
 }
 
@@ -1056,7 +1043,7 @@ cms_signeddata_create(krb5_context context,
     unsigned int alg_len = 0, digest_len = 0;
     unsigned char *y = NULL, *alg_buf = NULL, *digest_buf = NULL;
     X509 *cert = NULL;
-    ASN1_OBJECT *oid = NULL;
+    ASN1_OBJECT *oid = NULL, *oid_copy;
 
     /* Start creating PKCS7 data. */
     if ((p7 = PKCS7_new()) == NULL)
@@ -1178,8 +1165,11 @@ cms_signeddata_create(krb5_context context,
                                        V_ASN1_OCTET_STRING, (char *) digest_attr);
 
             /* create a content-type attr */
+            oid_copy = OBJ_dup(oid);
+            if (oid_copy == NULL)
+                goto cleanup2;
             PKCS7_add_signed_attribute(p7si, NID_pkcs9_contentType,
-                                       V_ASN1_OBJECT, oid);
+                                       V_ASN1_OBJECT, oid_copy);
 
             /* create the signature over signed attributes. get DER encoded value */
             /* This is the place where smartcard signature needs to be calculated */