From 52832e470f5fe8c222249ae5b539aeb3c74cdb25 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Wed, 18 May 2016 14:32:16 +0100 Subject: [PATCH] OID code tidy up. Tidy up and simplify OBJ_dup() and OBJ_create(). Sanity check added OIDs: don't allow duplicates. Reviewed-by: Richard Levitte --- crypto/objects/obj_dat.c | 46 +++++++++++++++++------------- crypto/objects/obj_err.c | 1 + crypto/objects/obj_lib.c | 59 ++++++++++++++------------------------- include/openssl/objects.h | 1 + 4 files changed, 49 insertions(+), 58 deletions(-) diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c index 9b8e93b054..820c275fd0 100644 --- a/crypto/objects/obj_dat.c +++ b/crypto/objects/obj_dat.c @@ -679,30 +679,36 @@ int OBJ_create_objects(BIO *in) int OBJ_create(const char *oid, const char *sn, const char *ln) { - int ok = 0; - ASN1_OBJECT *op = NULL; - unsigned char *buf; - int i; - - i = a2d_ASN1_OBJECT(NULL, 0, oid, -1); - if (i <= 0) - return (0); + ASN1_OBJECT *tmpoid = NULL; + int ok; - if ((buf = OPENSSL_malloc(i)) == NULL) { - OBJerr(OBJ_F_OBJ_CREATE, ERR_R_MALLOC_FAILURE); - return (0); + /* Check to see if short or long name already present */ + if (OBJ_sn2nid(sn) != NID_undef || OBJ_ln2nid(ln) != NID_undef) { + OBJerr(OBJ_F_OBJ_CREATE, OBJ_R_OID_EXISTS); + return 0; } - i = a2d_ASN1_OBJECT(buf, i, oid, -1); - if (i == 0) - goto err; - op = (ASN1_OBJECT *)ASN1_OBJECT_create(OBJ_new_nid(1), buf, i, sn, ln); - if (op == NULL) + + /* Convert numerical OID string to an ASN1_OBJECT structure */ + tmpoid = OBJ_txt2obj(oid, 1); + + /* If NID is not NID_undef then object already exists */ + if (OBJ_obj2nid(tmpoid) != NID_undef) { + OBJerr(OBJ_F_OBJ_CREATE, OBJ_R_OID_EXISTS); goto err; - ok = OBJ_add_object(op); + } + + tmpoid->nid = OBJ_new_nid(1); + tmpoid->sn = (char *)sn; + tmpoid->ln = (char *)ln; + + ok = OBJ_add_object(tmpoid); + + tmpoid->sn = NULL; + tmpoid->ln = NULL; + err: - ASN1_OBJECT_free(op); - OPENSSL_free(buf); - return (ok); + ASN1_OBJECT_free(tmpoid); + return ok; } size_t OBJ_length(const ASN1_OBJECT *obj) diff --git a/crypto/objects/obj_err.c b/crypto/objects/obj_err.c index cadfe81fbc..b8a90573ad 100644 --- a/crypto/objects/obj_err.c +++ b/crypto/objects/obj_err.c @@ -36,6 +36,7 @@ static ERR_STRING_DATA OBJ_str_functs[] = { static ERR_STRING_DATA OBJ_str_reasons[] = { {ERR_REASON(OBJ_R_MALLOC_FAILURE), "malloc failure"}, + {ERR_REASON(OBJ_R_OID_EXISTS), "oid exists"}, {ERR_REASON(OBJ_R_UNKNOWN_NID), "unknown nid"}, {0, NULL} }; diff --git a/crypto/objects/obj_lib.c b/crypto/objects/obj_lib.c index da977fedc8..33075e6451 100644 --- a/crypto/objects/obj_lib.c +++ b/crypto/objects/obj_lib.c @@ -17,59 +17,42 @@ ASN1_OBJECT *OBJ_dup(const ASN1_OBJECT *o) { ASN1_OBJECT *r; - int i; - char *ln = NULL, *sn = NULL; - unsigned char *data = NULL; if (o == NULL) - return (NULL); + return NULL; + /* If object isn't dynamic it's an internal OID which is never freed */ if (!(o->flags & ASN1_OBJECT_FLAG_DYNAMIC)) - return ((ASN1_OBJECT *)o); /* XXX: ugh! Why? What kind of duplication - * is this??? */ + return ((ASN1_OBJECT *)o); r = ASN1_OBJECT_new(); if (r == NULL) { OBJerr(OBJ_F_OBJ_DUP, ERR_R_ASN1_LIB); return (NULL); } - data = OPENSSL_malloc(o->length); - if (data == NULL) - goto err; - if (o->data != NULL) - memcpy(data, o->data, o->length); - /* once data attached to object it remains const */ - r->data = data; - r->length = o->length; - r->nid = o->nid; - r->ln = r->sn = NULL; - if (o->ln != NULL) { - i = strlen(o->ln) + 1; - ln = OPENSSL_malloc(i); - if (ln == NULL) - goto err; - memcpy(ln, o->ln, i); - r->ln = ln; - } - if (o->sn != NULL) { - i = strlen(o->sn) + 1; - sn = OPENSSL_malloc(i); - if (sn == NULL) - goto err; - memcpy(sn, o->sn, i); - r->sn = sn; - } + /* Set dynamic flags so everything gets freed up on error */ + r->flags = o->flags | (ASN1_OBJECT_FLAG_DYNAMIC | ASN1_OBJECT_FLAG_DYNAMIC_STRINGS | ASN1_OBJECT_FLAG_DYNAMIC_DATA); - return (r); + + if (o->length > 0 && (r->data = OPENSSL_memdup(o->data, o->length)) == NULL) + goto err; + + r->length = o->length; + r->nid = o->nid; + + if (o->ln != NULL && (r->ln = OPENSSL_strdup(o->ln)) == NULL) + goto err; + + if (o->sn != NULL && (r->sn = OPENSSL_strdup(o->sn)) == NULL) + goto err; + + return r; err: + ASN1_OBJECT_free(r); OBJerr(OBJ_F_OBJ_DUP, ERR_R_MALLOC_FAILURE); - OPENSSL_free(ln); - OPENSSL_free(sn); - OPENSSL_free(data); - OPENSSL_free(r); - return (NULL); + return NULL; } int OBJ_cmp(const ASN1_OBJECT *a, const ASN1_OBJECT *b) diff --git a/include/openssl/objects.h b/include/openssl/objects.h index 8b31e3dcfe..647a8ac81e 100644 --- a/include/openssl/objects.h +++ b/include/openssl/objects.h @@ -1088,6 +1088,7 @@ void ERR_load_OBJ_strings(void); /* Reason codes. */ # define OBJ_R_MALLOC_FAILURE 100 +# define OBJ_R_OID_EXISTS 102 # define OBJ_R_UNKNOWN_NID 101 #ifdef __cplusplus -- 2.34.1