validate_issuer(X509 *cert, bool is_ta)
{
X509_NAME *issuer;
+ struct rfc5280_name name;
int error;
issuer = X509_get_issuer_name(cert);
if (!is_ta)
return validate_issuer_name("Certificate", issuer);
- error = x509_name_decode(issuer, NID_commonName, NULL);
- if (error == -ESRCH)
- pr_err("The 'issuer' name lacks a commonName attribute.");
+ error = x509_name_decode(issuer, "issuer", &name);
+ if (!error)
+ x509_name_cleanup(&name);
return error;
}
validate_subject(X509 *cert)
{
struct validation *state;
- char *subject;
+ struct rfc5280_name name;
int error;
state = state_retrieve();
if (state == NULL)
return -EINVAL;
- error = x509_name_decode(X509_get_subject_name(cert), NID_commonName,
- &subject);
- if (error == -ESRCH)
- pr_err("Certificate's subject lacks the CommonName atribute.");
+ error = x509_name_decode(X509_get_subject_name(cert), "subject", &name);
if (error)
return error;
- error = validation_store_subject(state, subject);
+ error = validation_store_subject(state, &name);
+ if (error)
+ x509_name_cleanup(&name);
- free(subject);
return error;
}
#include "log.h"
#include "thread_var.h"
-/**
- * Only prints error message if the result is not 0 nor -ESRCH.
- */
-int
-x509_name_decode(X509_NAME *name, int nid, char **_result)
+static int
+name2string(X509_NAME_ENTRY *name, char **_result)
{
+ const ASN1_STRING *data;
char *result;
- int len1, len2;
- len1 = X509_NAME_get_text_by_NID(name, nid, NULL, 0);
- if (len1 < 0)
- return -ESRCH;
+ data = X509_NAME_ENTRY_get_data(name);
+ if (data == NULL)
+ return crypto_err("X509_NAME_ENTRY_get_data() returned NULL");
- if (_result == NULL)
- return 0;
-
- result = calloc(len1 + 1, sizeof(char));
+ result = malloc(data->length + 1);
if (result == NULL)
return pr_enomem();
- len2 = X509_NAME_get_text_by_NID(name, nid, result, len1 + 1);
- if (len1 != len2) {
- free(result);
- return pr_err("Likely programming error: X509_NAME_get_text_by_NID() returned inconsistent lengths: %d,%d",
- len1, len2);
- }
+ memcpy(result, data->data, data->length);
+ result[data->length] = '\0';
*_result = result;
return 0;
}
-struct rfc5280_names {
- char *commonName;
- char *serialNumber;
-};
-
-static int
-get_names(X509_NAME *name, char const *what, struct rfc5280_names *result)
+int
+x509_name_decode(X509_NAME *name, char const *what,
+ struct rfc5280_name *result)
{
+ int i;
+ X509_NAME_ENTRY *entry;
+ int nid;
int error;
- error = x509_name_decode(name, NID_commonName, &result->commonName);
- if (error == -ESRCH) {
- return pr_err("The '%s' name lacks a commonName attribute.",
- what);
+ result->commonName = NULL;
+ result->serialNumber = NULL;
+
+ for (i = 0; i < X509_NAME_entry_count(name); i++) {
+ entry = X509_NAME_get_entry(name, i);
+ nid = OBJ_obj2nid(X509_NAME_ENTRY_get_object(entry));
+ switch (nid) {
+ case NID_commonName:
+ error = name2string(entry, &result->commonName);
+ break;
+ case NID_serialNumber:
+ error = name2string(entry, &result->serialNumber);
+ break;
+ default:
+ error = pr_err("The '%s' name has an unknown attribute. (NID: %d)",
+ what, nid);
+ break;
+ }
+
+ if (error)
+ goto fail;
}
- if (error)
- return error;
- error = x509_name_decode(name, NID_serialNumber, &result->serialNumber);
- if (error == -ESRCH) {
- result->serialNumber = NULL;
- return 0;
- }
- if (error) {
- free(result->commonName);
- return error;
+ if (result->commonName == NULL) {
+ error = pr_err("The '%s' name lacks a commonName attribute.",
+ what);
+ goto fail;
}
return 0;
+
+fail:
+ x509_name_cleanup(result);
+ return error;
+}
+
+void
+x509_name_cleanup(struct rfc5280_name *name)
+{
+ free(name->commonName);
+ free(name->serialNumber);
}
/**
return strcmp(str1, str2) == 0;
}
+bool
+x509_name_equals(struct rfc5280_name *a, struct rfc5280_name *b)
+{
+ return (strcmp(a->commonName, b->commonName) == 0)
+ && str_equals(a->serialNumber, b->serialNumber);
+}
+
int
validate_issuer_name(char const *container, X509_NAME *issuer)
{
struct validation *state;
X509 *parent;
- struct rfc5280_names parent_subject = { 0 };
- struct rfc5280_names child_issuer = { 0 };
+ struct rfc5280_name parent_subject;
+ struct rfc5280_name child_issuer;
int error;
/*
container);
}
- error = get_names(X509_get_subject_name(parent), "subject",
+ error = x509_name_decode(X509_get_subject_name(parent), "subject",
&parent_subject);
if (error)
return error;
- error = get_names(issuer, "issuer", &child_issuer);
+ error = x509_name_decode(issuer, "issuer", &child_issuer);
if (error)
- goto end2;
-
- if (strcmp(parent_subject.commonName, child_issuer.commonName) != 0) {
- error = pr_err("%s's issuer commonName ('%s') does not equal issuer certificate's commonName ('%s').",
- container, parent_subject.commonName,
- child_issuer.commonName);
- goto end1;
- }
-
- if (!str_equals(parent_subject.serialNumber,
- child_issuer.serialNumber)) {
- error = pr_err("%s's issuer serialNumber ('%s') does not equal issuer certificate's serialNumber ('%s').",
- container, parent_subject.serialNumber,
- child_issuer.serialNumber);
- goto end1;
+ goto end;
+
+ if (!x509_name_equals(&parent_subject, &child_issuer)) {
+ error = pr_err("%s's issuer name ('%s%s%s') does not equal issuer certificate's name ('%s%s%s').",
+ container,
+ parent_subject.commonName,
+ (parent_subject.serialNumber != NULL) ? "/" : "",
+ (parent_subject.serialNumber != NULL)
+ ? parent_subject.serialNumber
+ : "",
+ child_issuer.commonName,
+ (child_issuer.serialNumber != NULL) ? "/" : "",
+ (child_issuer.serialNumber != NULL)
+ ? child_issuer.serialNumber
+ : "");
}
-end1:
- free(child_issuer.commonName);
- free(child_issuer.serialNumber);
-end2:
- free(parent_subject.commonName);
- free(parent_subject.serialNumber);
+ x509_name_cleanup(&child_issuer);
+end: x509_name_cleanup(&parent_subject);
return error;
}
#ifndef SRC_OBJECT_NAME_H_
#define SRC_OBJECT_NAME_H_
+#include <stdbool.h>
#include <openssl/x509.h>
-int x509_name_decode(X509_NAME *, int, char **);
+/**
+ * It's an RFC5280 name, but from RFC 6487's perspective.
+ * Meaning, only commonName and serialNumbers are allowed, and the latter is
+ * optional.
+ */
+struct rfc5280_name {
+ char *commonName;
+ char *serialNumber;
+};
+
+int x509_name_decode(X509_NAME *, char const *, struct rfc5280_name *);
+void x509_name_cleanup(struct rfc5280_name *);
+
+bool x509_name_equals(struct rfc5280_name *, struct rfc5280_name *);
+
int validate_issuer_name(char const *, X509_NAME *);
#endif /* SRC_OBJECT_NAME_H_ */
#include "log.h"
+/*
+ * TODO (next iteration) Implement RFC 6350.
+ * (The current code implements RFC 6493 *only*.)
+ */
+
/**
* Reminder: UTF-8 strings are **not** C strings.
* They can contain null characters, which do not terminate them.
- * DO NOT use standard string operations on them. (Except for the `n` ones).
+ * DO NOT use standard string operations on them. (Unless you know what you're
+ * doing).
*/
struct utf8_string {
uint8_t *val;
#include <string.h>
#include "array_list.h"
#include "log.h"
+#include "str.h"
#include "thread_var.h"
#include "object/certificate.h"
-ARRAY_LIST(serial_numbers, BIGNUM *)
-ARRAY_LIST(subjects, char *)
+struct serial_number {
+ BIGNUM *number;
+ char *file; /* File where this serial number was found. */
+};
+
+struct subject_name {
+ struct rfc5280_name name;
+ char *file; /* File where this subject name was found. */
+};
+
+ARRAY_LIST(serial_numbers, struct serial_number)
+ARRAY_LIST(subjects, struct subject_name)
/**
* Cached certificate data.
}
static void
-serial_cleanup(BIGNUM **serial)
+serial_cleanup(struct serial_number *serial)
{
- BN_free(*serial);
+ BN_free(serial->number);
+ free(serial->file);
}
static void
-subject_cleanup(char **subject)
+subject_cleanup(struct subject_name *subject)
{
- free(*subject);
+ x509_name_cleanup(&subject->name);
+ free(subject->file);
}
void
return (cert != NULL) ? cert->resources : NULL;
}
+static int
+get_current_file_name(char **_result)
+{
+ char const *tmp;
+ char *result;
+
+ tmp = fnstack_peek();
+ if (tmp == NULL)
+ return pr_crit("The file name stack is empty.");
+
+ result = strdup(tmp);
+ if (result == NULL)
+ return pr_enomem();
+
+ *_result = result;
+ return 0;
+}
+
+/**
+ * This function will steal ownership of @number on success.
+ */
int
validation_store_serial_number(struct validation *state, BIGNUM *number)
{
struct certificate *cert;
- BIGNUM **cursor;
- BIGNUM *duplicate;
+ struct serial_number *cursor;
+ struct serial_number duplicate;
+ char *string;
int error;
+ /* Remember to free @number if you return 0 but don't store it. */
+
cert = SLIST_FIRST(&state->certs);
- if (cert == NULL)
+ if (cert == NULL) {
+ BN_free(number);
return 0; /* The TA lacks siblings, so serial is unique. */
+ }
- ARRAYLIST_FOREACH(&cert->serials, cursor)
- if (BN_cmp(*cursor, number) == 0)
- return pr_err("Serial number is not unique.");
+ /*
+ * Note: This is is reported as a warning, even though duplicate serial
+ * numbers are clearly a violation of the RFC and common sense.
+ *
+ * But it cannot be simply upgraded into an error because we are
+ * realizing the problem too late; our traversal is depth-first, so we
+ * already approved the other bogus certificate and its children.
+ * (I don't think changing to a breath-first traversal would be a good
+ * idea; the RAM usage would skyrocket because, since we need the entire
+ * certificate path to the root to validate any certificate, we would
+ * end up having the entire tree loaded in memory by the time we're done
+ * traversing.)
+ *
+ * So instead of arbitrarily approving one certificate but not the
+ * other, we will accept both but report a warning.
+ *
+ * Also: It's pretty odd; a significant amount of certificates seem to
+ * be breaking this rule. Maybe we're the only ones catching it?
+ */
+ ARRAYLIST_FOREACH(&cert->serials, cursor) {
+ if (BN_cmp(cursor->number, number) == 0) {
+ BN2string(number, &string);
+ pr_warn("Serial number '%s' is not unique. (Also found in '%s'.)",
+ string, cursor->file);
+ BN_free(number);
+ free(string);
+ return 0;
+ }
+ }
- duplicate = BN_dup(number);
- if (duplicate == NULL)
- return crypto_err("Could not duplicate a BIGNUM");
+ duplicate.number = number;
+ error = get_current_file_name(&duplicate.file);
+ if (error)
+ return error;
error = serial_numbers_add(&cert->serials, &duplicate);
if (error)
- BN_free(duplicate);
+ free(duplicate.file);
return error;
}
+/**
+ * This function will steal ownership of @subject on success.
+ */
int
-validation_store_subject(struct validation *state, char *subject)
+validation_store_subject(struct validation *state, struct rfc5280_name *subject)
{
struct certificate *cert;
- char **cursor;
- char *duplicate;
+ struct subject_name *cursor;
+ struct subject_name duplicate;
int error;
+ /*
+ * There's something that's not clicking with me:
+ *
+ * "Each distinct subordinate CA and
+ * EE certified by the issuer MUST be identified using a subject name
+ * that is unique per issuer. In this context, 'distinct' is defined as
+ * an entity and a given public key."
+ *
+ * Does the last sentence have any significance to us? I don't even
+ * understand why the requirement exists. 5280 and 6487 don't even
+ * define "entity." I guess it's the same meaning from "End-Entity",
+ * and in this case it's supposed to function as a synonym for "subject
+ * name."
+ *
+ * "An issuer SHOULD use a different
+ * subject name if the subject's key pair has changed (i.e., when the CA
+ * issues a certificate as part of re-keying the subject.)"
+ *
+ * It's really weird that it seems to be rewording the same requirement
+ * except the first version is defined as MUST and the second one is
+ * defined as SHOULD.
+ *
+ * Ugh. Okay. Let's use some common sense. There are four possible
+ * situations:
+ *
+ * - Certificates do not share name nor public key. We should accept
+ * this.
+ * - Certificates share name, but not public key. We should reject this.
+ * - Certificates share public key, but not name. This is basically
+ * impossible, but fine nonetheless. Accept.
+ * (But maybe issue a warning. It sounds like the two children can
+ * impersonate each other.)
+ * - Certificates share name and public key. This likely means that we
+ * are looking at two different versions of the same certificate, but
+ * we can't tell for sure right now, and so we should accept it.
+ *
+ * This is all conjecture. We should probably mail the IETF.
+ *
+ * TODO (next iteration) The code below complains when certificates
+ * share names, and ignores public keys. I've decided to defer the
+ * fixing.
+ */
+
+ /* Remember to free @subject if you return 0 but don't store it. */
+
cert = SLIST_FIRST(&state->certs);
- if (cert == NULL)
+ if (cert == NULL) {
+ x509_name_cleanup(subject);
return 0; /* The TA lacks siblings, so subject is unique. */
+ }
+ /* See the large comment in validation_store_serial_number(). */
ARRAYLIST_FOREACH(&cert->subjects, cursor) {
- if (strcmp(*cursor, subject) == 0) {
- /*
- * I had to downgrade this because lots of people are
- * breaking this rule.
- * TODO (next iteration) make a framework so the user
- * can choose the severity of each error.
- */
- return pr_warn("Subject name '%s' is not unique.",
- subject);
+ if (x509_name_equals(&cursor->name, subject)) {
+ pr_warn("Subject name '%s%s%s' is not unique. (Also found in '%s'.)",
+ subject->commonName,
+ (subject->serialNumber != NULL) ? "/" : "",
+ (subject->serialNumber != NULL)
+ ? subject->serialNumber
+ : "",
+ cursor->file);
+ x509_name_cleanup(subject);
+ return 0;
}
}
- duplicate = strdup(subject);
- if (duplicate == NULL)
- return pr_err("Could not duplicate a String");
+ duplicate.name = *subject;
+ error = get_current_file_name(&duplicate.file);
+ if (error)
+ return error;
error = subjects_add(&cert->subjects, &duplicate);
if (error)
- free(duplicate);
+ free(duplicate.file);
return error;
}
#include <openssl/x509.h>
#include "resource.h"
+#include "object/name.h"
#include "object/tal.h"
struct validation;
struct resources *validation_peek_resource(struct validation *);
int validation_store_serial_number(struct validation *, BIGNUM *);
-int validation_store_subject(struct validation *, char *);
+int validation_store_subject(struct validation *, struct rfc5280_name *);
char *validation_get_ip_buffer1(struct validation *);
char *validation_get_ip_buffer2(struct validation *);
#include "str.h"
+#include <errno.h>
#include <string.h>
#include "log.h"
: string_clone(ia5->data, ia5->length, result);
}
+int
+BN2string(BIGNUM *bn, char **_result)
+{
+ BIO *bio;
+ uint64_t written;
+ char *result;
+
+ /* Callers can call free() whether this function fails or not. */
+ *_result = NULL;
+
+ bio = BIO_new(BIO_s_mem());
+ if (bio == NULL)
+ return -ENOMEM;
+
+ if (BN_print(bio, bn) == 0) {
+ BIO_free(bio);
+ return crypto_err("Unable to print the BIGNUM into a BIO");
+ }
+
+ written = BIO_number_written(bio);
+ result = malloc(written + 1);
+ if (result == NULL) {
+ BIO_free(bio);
+ return pr_enomem();
+ }
+
+ BIO_read(bio, result, written);
+ result[written] = '\0';
+
+ BIO_free(bio);
+ *_result = result;
+ return 0;
+}
+
void
string_tokenizer_init(struct string_tokenizer *tokenizer, char const *str,
size_t str_len, unsigned char separator)
int string_clone(void const *, size_t, char **);
int ia5s2string(ASN1_IA5STRING *, char **);
+int BN2string(BIGNUM *, char **);
/* This file is named "str.h" because "string.h" collides with <string.h>. */
static char const *
get_filename(char const *file_path)
{
- char *slash = strrchr(file_path, '/');
- return (slash != NULL) ? (slash + 1) : file_path;
+ /* char *slash = strrchr(file_path, '/'); */
+ return /* (slash != NULL) ? (slash + 1) : */ file_path;
}
/**
projects / thirdparty / FORT-validator.git / commitdiff
