From: Alberto Leiva Popper Date: Mon, 17 Dec 2018 19:01:35 +0000 (-0600) Subject: Adds: X-Git-Tag: v0.0.2~117 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3eafa9b02ac35a2150c26baadc03307afcbfd4d9;p=thirdparty%2FFORT-validator.git Adds: - Check that the TAL's public key matches the root cert's public key - Validate EE certificates differently than CA certificates - Reorder tree traversal. (I noticed that I was doing it wrong.) - Polish many other validations by hunting TODOs --- diff --git a/src/Makefile.am b/src/Makefile.am index 23378d80..5bc740af 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -7,6 +7,7 @@ bin_PROGRAMS = rpki_validator rpki_validator_SOURCES = main.c rpki_validator_SOURCES += address.h address.c +rpki_validator_SOURCES += base64.h base64.c rpki_validator_SOURCES += common.h common.c rpki_validator_SOURCES += debug.h debug.c rpki_validator_SOURCES += file.h file.c diff --git a/src/asn1/oid.c b/src/asn1/oid.c index 6089ba93..bfb8cc3c 100644 --- a/src/asn1/oid.c +++ b/src/asn1/oid.c @@ -2,6 +2,7 @@ #include #include "common.h" +#include "log.h" #include "asn1/decode.h" #define MAX_ARCS 9 @@ -23,11 +24,14 @@ oid2arcs(OBJECT_IDENTIFIER_t *oid, struct oid_arcs *result) ssize_t count, count2; result->arcs = malloc(MAX_ARCS * sizeof(asn_oid_arc_t)); - if (result->arcs == NULL) + if (result->arcs == NULL) { + pr_err("Out of memory."); return -ENOMEM; + } count = OBJECT_IDENTIFIER_get_arcs(oid, result->arcs, MAX_ARCS); if (count < 0) { + pr_err("OBJECT_IDENTIFIER_get_arcs() returned %zd.", count); free(result->arcs); return count; } @@ -37,10 +41,14 @@ oid2arcs(OBJECT_IDENTIFIER_t *oid, struct oid_arcs *result) /* If necessary, reallocate arcs array and try again. */ if (count > MAX_ARCS) { result->arcs = realloc(result->arcs, count * sizeof(asn_oid_arc_t)); - if (!result->arcs) + if (!result->arcs) { + pr_err("Out of memory."); return -ENOMEM; + } count2 = OBJECT_IDENTIFIER_get_arcs(oid, result->arcs, count); if (count != count2) { + pr_err("OBJECT_IDENTIFIER_get_arcs() returned %zd. (expected %zd)", + count2, count); free(result->arcs); return -EINVAL; } diff --git a/src/asn1/oid.h b/src/asn1/oid.h index ba73b1ee..820ff1c9 100644 --- a/src/asn1/oid.h +++ b/src/asn1/oid.h @@ -33,6 +33,8 @@ typedef asn_oid_arc_t OID[]; #define OID_ROA { 1, 2, 840, 113549, 1, 9, 16, 1, 24 } #define OID_MANIFEST { 1, 2, 840, 113549, 1, 9, 16, 1, 26 } +#define OID_RSA { 1, 2, 840, 113549, 1, 1, 1 } + #define OID_SHA224 { 2, 16, 840, 1, 101, 3, 4, 2, 4 } #define OID_SHA256 { 2, 16, 840, 1, 101, 3, 4, 2, 1 } #define OID_SHA384 { 2, 16, 840, 1, 101, 3, 4, 2, 2 } diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c index ab549759..155d6ade 100644 --- a/src/asn1/signed_data.c +++ b/src/asn1/signed_data.c @@ -42,13 +42,14 @@ is_digest_algorithm(AlgorithmIdentifier_t *aid, bool *result) } static int -handle_sdata_certificate(ANY_t *any, struct resources *res) +handle_sdata_certificate(ANY_t *any, STACK_OF(X509_CRL) *crls, + struct resources *res) { const unsigned char *tmp; X509 *cert; int error; - pr_debug_add("Certificate (embedded) {"); + pr_debug_add("(EE?) Certificate (embedded) {"); /* * "If the call is successful *in is incremented to the byte following @@ -65,7 +66,10 @@ handle_sdata_certificate(ANY_t *any, struct resources *res) goto end1; } - error = certificate_validate(cert, NULL); /* TODO crls */ + error = certificate_validate_chain(cert, crls); + if (error) + goto end2; + error = certificate_validate_rfc6487(cert, false); if (error) goto end2; @@ -75,7 +79,7 @@ handle_sdata_certificate(ANY_t *any, struct resources *res) goto end2; } - /* TODO maybe spill a warning if the certificate has children? */ + error = certificate_traverse_ee(cert); end2: X509_free(cert); @@ -226,7 +230,8 @@ illegal_attrType: } static int -validate(struct SignedData *sdata, struct resources *res) +validate(struct SignedData *sdata, STACK_OF(X509_CRL) *crls, + struct resources *res) { struct SignerInfo *sinfo; bool is_digest; @@ -288,7 +293,7 @@ validate(struct SignedData *sdata, struct resources *res) } error = handle_sdata_certificate(sdata->certificates->list.array[0], - res); + crls, res); if (error) return error; @@ -371,7 +376,7 @@ validate(struct SignedData *sdata, struct resources *res) int signed_data_decode(ANY_t *coded, struct SignedData **result, - struct resources *res) + STACK_OF(X509_CRL) *crls, struct resources *res) { struct SignedData *sdata; int error; @@ -381,7 +386,7 @@ signed_data_decode(ANY_t *coded, struct SignedData **result, if (error) return error; - error = validate(sdata, res); + error = validate(sdata, crls, res); if (error) { signed_data_free(sdata); return error; diff --git a/src/asn1/signed_data.h b/src/asn1/signed_data.h index 49d10b67..4d06aa9c 100644 --- a/src/asn1/signed_data.h +++ b/src/asn1/signed_data.h @@ -3,10 +3,12 @@ /* Some wrappers for libcmscodec's SignedData. */ +#include #include #include "resource.h" -int signed_data_decode(ANY_t *, struct SignedData **, struct resources *); +int signed_data_decode(ANY_t *, struct SignedData **, STACK_OF(X509_CRL) *, + struct resources *); void signed_data_free(struct SignedData *); int get_content_type_attr(struct SignedData *, OBJECT_IDENTIFIER_t **); diff --git a/src/base64.c b/src/base64.c new file mode 100644 index 00000000..05eb7e45 --- /dev/null +++ b/src/base64.c @@ -0,0 +1,113 @@ +#include "base64.h" + +#include +#include + +/** + * Converts error from libcrypto representation to this project's + * representation. + */ +static int +error_ul2i(unsigned long error) +{ + /* I'm assuming int has at least 32 bits. Don't mess with the sign. */ + int interror = error & 0x7FFFFFFFul; + return interror ? interror : -EINVAL; +} + +/* + * Reference: openbsd/src/usr.bin/openssl/enc.c + * + * @in: The BIO that will stream the base64 encoded string you want to decode. + * @out: Buffer where this function will write the decoded string. + * @out_len: Total allocated size of @out. It's supposed to be the result of + * EVP_DECODE_LENGTH(). + * @out_written: This function will write the actual number of decoded bytes + * here. + * + * Returns error status. (Nonzero = error code, zero = success) + * + * If this returns error, do visit ERR_print_errors(), but also print an + * additional error message anyway. Functions such as BIO_new() don't always + * register a libcrypto stack error. + */ +int +base64_decode(BIO *in, unsigned char *out, size_t out_len, size_t *out_written) +{ + BIO *b64; + size_t offset = 0; + int written = 0; + unsigned long error; + + /* + * BTW: The libcrypto API was clearly designed by fucking idiots. + * Peeking at the error stack is the only way I found to figure out + * whether some of the functions error'd. + * But since it's not documented that it's supposed to work this way, + * there's no guarantee that it will catch all errors. + * But it will have to do. It's better than nothing. + */ + + /* Assume that the caller took care of handling any previous errors. */ + ERR_clear_error(); + + /* + * BIO_f_base64() cannot fail because it's dead-simple by definition. + * BIO_new() can, and it will lead to NULL. But only *some* errors will + * populate the error stack. + */ + b64 = BIO_new(BIO_f_base64()); + if (b64 == NULL) { + error = ERR_peek_last_error(); + return error ? error_ul2i(error) : -ENOMEM; + } + + /* TODO would changing this flag simplify file reading? */ + /* BIO_set_flags() cannot fail; it's dead-simple by definition. */ + BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL); + /* + * BIO_push() can technically fail through BIO_ctrl(), but it ignores + * the error. This will not cause it to revert the push, so we have to + * do it ourselves. + * Should we ignore this error? BIO_ctrl(BIO_CTRL_PUSH) performs some + * "internal, used to signify change" thing, whose importance is + * undefined due to BIO_ctrl()'s callback spaghetti. + * I'm not risking it. + */ + in = BIO_push(b64, in); + error = ERR_peek_last_error(); + if (error) + goto end; + + do { + /* + * Do not move this after BIO_read(). + * BIO_read() can return negative, which does not necessarily + * imply error, and which ruins the counter. + */ + offset += written; + /* + * According to the documentation, the first argument should + * be b64, not in. + * But this is how it's written in enc.c. + * It doesn't seem to make a difference either way. + */ + written = BIO_read(in, out + offset, out_len - offset); + } while (written > 0); + + /* BIO_read() can fail. It does not return status. */ + error = ERR_peek_last_error(); + *out_written = offset; + +end: + /* + * BIO_pop() can also fail due to BIO_ctrl(), but we will ignore this + * because whatever "signify change" crap happens, it can't possibly be + * damaging enough to prevent us from releasing b64. I hope. + */ + BIO_pop(b64); + /* Returns 0 on failure, but that's only if b64 is NULL. Meaningless. */ + BIO_free(b64); + + return error ? error_ul2i(error) : 0; +} diff --git a/src/base64.h b/src/base64.h new file mode 100644 index 00000000..c56b3e6f --- /dev/null +++ b/src/base64.h @@ -0,0 +1,9 @@ +#ifndef SRC_BASE64_H_ +#define SRC_BASE64_H_ + +#include +#include + +int base64_decode(BIO *, unsigned char *, size_t, size_t *); + +#endif /* SRC_BASE64_H_ */ diff --git a/src/common.c b/src/common.c index 1a87f54d..169a3555 100644 --- a/src/common.c +++ b/src/common.c @@ -5,15 +5,21 @@ #include "log.h" char const *repository; +size_t repository_len; int NID_rpkiManifest; -int NID_rpkiNotify; +int NID_signedObject; +/* @extension must include the period. */ bool -file_has_extension(char const *file_name, char const *extension) +file_has_extension(char const *filename, size_t filename_len, char const *ext) { - char *dot; - dot = strrchr(file_name, '.'); - return dot ? (!strcmp(dot + 1, extension)) : false; + size_t ext_len; + + ext_len = strlen(ext); + if (filename_len < ext_len) + return false; + + return strncmp(filename + filename_len - ext_len, ext, ext_len) == 0; } /** @@ -23,57 +29,41 @@ file_has_extension(char const *file_name, char const *extension) * "/tmp/rpki/rpki.ripe.net/repo/manifest.mft". * * You need to free the result once you're done. + * This function does not assume that @guri is null-terminated. */ int -uri_g2l(char const *guri, char **result) +uri_g2l(char const *guri, size_t guri_len, char **result) { - char const *const PREFIX = "rsync://"; + static char const *const PREFIX = "rsync://"; char *luri; - size_t repository_len; size_t prefix_len; - unsigned int offset; + size_t extra_slash; + size_t offset; prefix_len = strlen(PREFIX); - if (strncmp(PREFIX, guri, prefix_len) != 0) { - pr_err("Global URI %s does not begin with '%s'.", guri, - PREFIX); + if (guri_len < prefix_len || strncmp(PREFIX, guri, prefix_len) != 0) { + pr_err("Global URI does not begin with '%s'.", PREFIX); return -EINVAL; } - repository_len = strlen(repository); + guri += prefix_len; + guri_len -= prefix_len; + extra_slash = (repository[repository_len - 1] == '/') ? 0 : 1; - luri = malloc(repository_len - + 1 /* slash */ - + strlen(guri) - prefix_len - + 1); /* null chara */ + luri = malloc(repository_len + extra_slash + guri_len + 1); if (!luri) return -ENOMEM; offset = 0; strcpy(luri + offset, repository); offset += repository_len; - strcpy(luri + offset, "/"); - offset += 1; - strcpy(luri + offset, &guri[prefix_len]); + strncpy(luri + offset, "/", extra_slash); + offset += extra_slash; + strncpy(luri + offset, guri, guri_len); + offset += guri_len; + luri[offset] = '\0'; *result = luri; return 0; } - -int -gn2uri(GENERAL_NAME *gn, char const **uri) -{ - ASN1_STRING *asn_string; - int type; - - asn_string = GENERAL_NAME_get0_value(gn, &type); - if (type != GEN_URI) { - pr_err("Unknown GENERAL_NAME type: %d", type); - return -ENOTSUPPORTED; - } - - /* TODO is this cast safe? */ - *uri = (char const *) ASN1_STRING_get0_data(asn_string); - return 0; -} diff --git a/src/common.h b/src/common.h index 80ac2615..f18345ba 100644 --- a/src/common.h +++ b/src/common.h @@ -10,13 +10,13 @@ #define ENOTIMPLEMENTED 3173 extern char const *repository; +extern size_t repository_len; extern int NID_rpkiManifest; -extern int NID_rpkiNotify; +extern int NID_signedObject; #define ARRAY_LEN(array) (sizeof(array) / sizeof(array[0])) -bool file_has_extension(char const *, char const *); -int uri_g2l(char const *, char **); -int gn2uri(GENERAL_NAME *, char const **); +bool file_has_extension(char const *, size_t, char const *); +int uri_g2l(char const *, size_t, char **); #endif /* SRC_RTR_COMMON_H_ */ diff --git a/src/log.c b/src/log.c index 99976270..fa6eeac3 100644 --- a/src/log.c +++ b/src/log.c @@ -48,14 +48,17 @@ pr_rm_indent(void) fprintf(STDERR, "Programming error: Too many pr_rm_indent()s.\n"); } -static void + +#endif + +void pr_debug_prefix(void) { +#ifdef DEBUG fprintf(STDOUT, "DBG: "); pr_indent(STDOUT); -} - #endif +} void pr_debug(const char *format, ...) @@ -120,7 +123,6 @@ pr_err_prefix(void) va_start(args, format); \ vfprintf(STDERR, format, args); \ va_end(args); \ - fprintf(STDERR, "\n"); \ } while (0) /** @@ -131,6 +133,7 @@ pr_err(const char *format, ...) { va_list args; PR_ERR(args); + fprintf(STDERR, "\n"); } /** @@ -152,22 +155,19 @@ pr_errno(int error, const char *format, ...) { va_list args; - pr_err_prefix(); - pr_file_name(STDERR); - - va_start(args, format); - vfprintf(STDERR, format, args); - va_end(args); + PR_ERR(args); if (error) { fprintf(STDERR, ": %s", strerror(error)); } else { - /* We should assume that there WAS an error; go generic. */ + /* + * If this function was called, then we need to assume that + * there WAS an error; go generic. + */ error = -EINVAL; } fprintf(STDERR, "\n"); - return error; } @@ -187,59 +187,27 @@ pr_errno(int error, const char *format, ...) int crypto_err(const char *format, ...) { - struct validation *state; - BIO *bio; - bool bio_needs_free; - char const *file; va_list args; int error; - error = ERR_GET_REASON(ERR_peek_last_error()); - bio = NULL; - bio_needs_free = false; - - state = state_retrieve(); - if (state != NULL) - bio = validation_stderr(state); - if (bio == NULL) { - bio = BIO_new_fp(stderr, BIO_NOCLOSE); - if (bio == NULL) - goto trainwreck; - bio_needs_free = true; - } - - file = fnstack_peek(); - BIO_printf(bio, "%s: ", (file != NULL) ? file : "(Unknown file)"); - - va_start(args, format); - BIO_vprintf(bio, format, args); - va_end(args); - BIO_printf(bio, ": "); + PR_ERR(args); + error = ERR_GET_REASON(ERR_peek_last_error()); if (error) { /* * Reminder: This clears the error queue. * BTW: The string format is pretty ugly. Maybe override this. */ - ERR_print_errors(bio); + ERR_print_errors_fp(STDERR); } else { - /* We should assume that there WAS an error; go generic. */ - BIO_printf(bio, "(There are no error messages in the stack.)"); + /* + * If this function was called, then we need to assume that + * there WAS an error; go generic. + */ + fprintf(STDERR, "(There are no error messages in the stack.)"); error = -EINVAL; } - BIO_printf(bio, "\n"); - if (bio_needs_free) - BIO_free_all(bio); + fprintf(STDERR, "\n"); return error; - -trainwreck: - /* Fall back to behave just like pr_err(). */ - PR_ERR(args); - - pr_err_prefix(); - pr_file_name(STDERR); - fprintf(STDERR, "(Cannot print libcrypto error: Failed to initialise standard error's BIO.)\n"); - - return error ? error : -EINVAL; } diff --git a/src/log.h b/src/log.h index 2be6ad87..840146a0 100644 --- a/src/log.h +++ b/src/log.h @@ -4,6 +4,7 @@ void pr_debug(const char *, ...); void pr_debug_add(const char *, ...); void pr_debug_rm(const char *, ...); +void pr_debug_prefix(void); void pr_err(const char *, ...); int pr_errno(int, const char *, ...); diff --git a/src/main.c b/src/main.c index f3442b56..0b73986a 100644 --- a/src/main.c +++ b/src/main.c @@ -21,10 +21,33 @@ add_rpki_oids(void) "Resource Public Key Infrastructure (RPKI) manifest access method"); printf("rpkiManifest registered. Its nid is %d.\n", NID_rpkiManifest); - NID_rpkiNotify = OBJ_create("1.3.6.1.5.5.7.48.13", - "id-ad-rpkiNotify (RFC 8182)", - /* TODO */ "Blah blah"); - printf("rpkiNotify registered. Its nid is %d.\n", NID_rpkiNotify); + NID_signedObject = OBJ_create("1.3.6.1.5.5.7.48.11", + "id-ad-signedObject (RFC 6487)", + /* TODO */ ""); + printf("signedObject registered. Its nid is %d.\n", NID_signedObject); +} + +static int +handle_tal_certificate(char *uri) +{ + X509 *cert; + int error; + + fnstack_push(uri); + error = certificate_load(uri, &cert); + if (error) + goto end; + + error = certificate_validate_rfc6487(cert, true); + if (error) + goto revert; + error = certificate_traverse_ca(cert, NULL); + +revert: + X509_free(cert); +end: + fnstack_pop(); + return error; } /** @@ -32,38 +55,35 @@ add_rpki_oids(void) * have been extracted from a TAL. */ static int -handle_tal_uri(char const *uri) +handle_tal_uri(struct tal *tal, char const *guri) { struct validation *state; - char *cert_file; + char *luri; int error; - error = uri_g2l(uri, &cert_file); + error = validation_prepare(&state, tal); if (error) return error; - error = validation_create(&state, cert_file); - if (error) - goto end1; - - pr_debug_add("TAL URI %s {", uri); + pr_debug_add("TAL URI %s {", guri); - if (!is_certificate(uri)) { + if (!is_certificate(guri)) { pr_err("TAL file does not point to a certificate. (Expected .cer, got '%s')", - uri); + guri); error = -ENOTSUPPORTED; - goto end2; + goto end; } - fnstack_push(uri); - error = certificate_traverse(validation_peek_cert(state)); - fnstack_pop(); + error = uri_g2l(guri, strlen(guri), &luri); + if (error) + return error; -end2: - pr_debug_rm("}"); + error = handle_tal_certificate(luri); + free(luri); + +end: validation_destroy(state); -end1: - free(cert_file); + pr_debug_rm("}"); return error; } @@ -86,6 +106,7 @@ main(int argc, char **argv) fnstack_push(argv[2]); repository = argv[1]; + repository_len = strlen(repository); error = tal_load(argv[2], &tal); if (error) diff --git a/src/object/certificate.c b/src/object/certificate.c index ed997f8a..ee0046cd 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -2,6 +2,7 @@ #include #include +#include #include #include "common.h" @@ -9,6 +10,7 @@ #include "manifest.h" #include "thread_var.h" #include "asn1/decode.h" +#include "asn1/oid.h" /* * The X509V3_EXT_METHOD that references NID_sinfo_access uses the AIA item. @@ -19,7 +21,29 @@ typedef AUTHORITY_INFO_ACCESS SIGNATURE_INFO_ACCESS; bool is_certificate(char const *file_name) { - return file_has_extension(file_name, "cer"); + return file_has_extension(file_name, strlen(file_name), ".cer"); +} + +static int +validate_serial_number(X509 *cert) +{ + /* TODO implement this properly. */ + + BIGNUM *number; + + number = ASN1_INTEGER_to_BN(X509_get0_serialNumber(cert), NULL); + if (number == NULL) { + crypto_err("Could not parse certificate serial number"); + return 0; + } + + pr_debug_prefix(); + fprintf(stdout, "serial Number: "); + BN_print_fp(stdout, number); + fprintf(stdout, "\n"); + BN_free(number); + + return 0; } static int @@ -39,28 +63,118 @@ validate_signature_algorithm(X509 *cert) static int validate_name(X509_NAME *name, char *what) { - X509_NAME_ENTRY *entry; - int nid; - int i; +#ifdef DEBUG + char *str; +#endif + int str_len; + + str_len = X509_NAME_get_text_by_NID(name, NID_commonName, NULL, 0); + if (str_len < 0) { + pr_err("Certificate's %s lacks the CommonName atribute.", what); + return -ESRCH; + } + +#ifdef DEBUG + str = calloc(str_len + 1, 1); + if (str == NULL) { + pr_err("Out of memory."); + return -ENOMEM; + } + + X509_NAME_get_text_by_NID(name, NID_commonName, str, str_len + 1); + + pr_debug("%s: %s", what, str); + free(str); +#endif + + return 0; +} + +static int +validate_spki(const unsigned char *cert_spk, int cert_spk_len) +{ + struct validation *state; + struct tal *tal; + + struct SubjectPublicKeyInfo *tal_spki; + unsigned char const *_tal_spki; + size_t _tal_spki_len; + + static const OID oid_rsa = OID_RSA; + struct oid_arcs tal_alg_arcs; + + int error; + + state = state_retrieve(); + if (state == NULL) + return -EINVAL; + + tal = validation_tal(state); + if (tal == NULL) { + pr_err("Programming error: Validation state has no TAL."); + return -EINVAL; + } + + /* + * We have a problem at this point: + * + * RFC 7730 says "The public key used to verify the trust anchor MUST be + * the same as the subjectPublicKeyInfo in the CA certificate and in the + * TAL." + * + * It seems that libcrypto decodes the Subject Public Key Info (SPKI) + * and gives us the Subject Public Key (SPK) instead. So we can't just + * compare the two keys just like that. + * + * Luckily, the only other component of the SPKI is the algorithm + * identifier. So doing a field-by-field comparison is not too much + * trouble. We'll have to decode the TAL's SPKI though. + */ + + tal_get_spki(tal, &_tal_spki, &_tal_spki_len); + error = asn1_decode(_tal_spki, _tal_spki_len, + &asn_DEF_SubjectPublicKeyInfo, (void **) &tal_spki); + if (error) + return error; - 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)); - if (nid == NID_commonName) - return 0; + /* Algorithm Identifier */ + error = oid2arcs(&tal_spki->algorithm.algorithm, &tal_alg_arcs); + if (error) + goto fail; + + if (!ARCS_EQUAL_OIDS(&tal_alg_arcs, oid_rsa)) { + pr_err("TAL's public key format is not RSA PKCS#1 v1.5 with SHA-256."); + error = -EINVAL; + goto fail; } - pr_err("Certificate's %s lacks the CommonName atribute.", what); - return -ESRCH; + /* SPK */ + if (tal_spki->subjectPublicKey.size != cert_spk_len) + goto not_equal; + if (memcmp(tal_spki->subjectPublicKey.buf, cert_spk, cert_spk_len) != 0) + goto not_equal; + + ASN_STRUCT_FREE(asn_DEF_SubjectPublicKeyInfo, tal_spki); + return 0; + +not_equal: + pr_err("TAL's public key is different than the root certificate's public key."); + error = -EINVAL; +fail: + ASN_STRUCT_FREE(asn_DEF_SubjectPublicKeyInfo, tal_spki); + return error; } static int -validate_public_key(X509 *cert) +validate_public_key(X509 *cert, bool is_root) { X509_PUBKEY *pubkey; - ASN1_OBJECT *algorithm; - int nid; + ASN1_OBJECT *alg; + int alg_nid; + const unsigned char *bytes; + int bytes_len; int ok; + int error; pubkey = X509_get_X509_PUBKEY(cert); if (pubkey == NULL) { @@ -68,25 +182,25 @@ validate_public_key(X509 *cert) return -EINVAL; } - ok = X509_PUBKEY_get0_param(&algorithm, NULL, NULL, NULL, pubkey); + ok = X509_PUBKEY_get0_param(&alg, &bytes, &bytes_len, NULL, pubkey); if (!ok) { crypto_err("X509_PUBKEY_get0_param() returned %d.", ok); return -EINVAL; } - nid = OBJ_obj2nid(algorithm); + alg_nid = OBJ_obj2nid(alg); /* * TODO Everyone uses this algorithm, but at a quick glance, it doesn't * seem to match RFC 7935's public key algorithm. Wtf? */ - if (nid != NID_rsaEncryption) { + if (alg_nid != NID_rsaEncryption) { pr_err("Certificate's public key format is %d, not RSA PKCS#1 v1.5 with SHA-256.", - nid); + alg_nid); return -EINVAL; } /* - * BTW: WTF. + * BTW: WTF. About that algorithm: * * RFC 6485: "The value for the associated parameters from that clause * [RFC4055] MUST also be used for the parameters field." @@ -97,6 +211,12 @@ validate_public_key(X509 *cert) * getting the message. */ + if (is_root) { + error = validate_spki(bytes, bytes_len); + if (error) + return error; + } + return 0; } @@ -107,8 +227,8 @@ validate_extensions(X509 *cert) return 0; } -static int -rfc6487_validate(X509 *cert) +int +certificate_validate_rfc6487(X509 *cert, bool is_root) { int error; @@ -131,7 +251,10 @@ rfc6487_validate(X509 *cert) return -EINVAL; } - /* TODO rfc6487#section-4.2 */ + /* rfc6487#section-4.2 */ + error = validate_serial_number(cert); + if (error) + return error; /* rfc6487#section-4.3 */ error = validate_signature_algorithm(cert); @@ -159,7 +282,8 @@ rfc6487_validate(X509 *cert) /* libcrypto already does this. */ /* rfc6487#section-4.7 */ - error = validate_public_key(cert); + /* Fragment of rfc7730#section-2.2 */ + error = validate_public_key(cert, is_root); if (error) return error; @@ -178,33 +302,26 @@ certificate_load(const char *file, X509 **result) return crypto_err("BIO_new(BIO_s_file()) returned NULL"); if (BIO_read_filename(bio, file) <= 0) { error = crypto_err("Error reading certificate"); - goto abort1; + goto end; } cert = d2i_X509_bio(bio, NULL); if (cert == NULL) { error = crypto_err("Error parsing certificate"); - goto abort1; + goto end; } - error = rfc6487_validate(cert); - if (error) - goto abort2; - *result = cert; - BIO_free(bio); - return 0; - -abort2: - X509_free(cert); -abort1: + error = 0; +end: BIO_free(bio); return error; } int -certificate_validate(X509 *cert, STACK_OF(X509_CRL) *crls) +certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls) { + /* Reference: libcrypto/.../verify.c */ /* * TODO * The only difference between -CAfile and -trusted, as it seems, is @@ -237,7 +354,8 @@ certificate_validate(X509 *cert, STACK_OF(X509_CRL) *crls) } X509_STORE_CTX_trusted_stack(ctx, validation_certs(state)); - X509_STORE_CTX_set0_crls(ctx, crls); + if (crls != NULL) + X509_STORE_CTX_set0_crls(ctx, crls); /* * HERE'S THE MEAT OF LIBCRYPTO'S VALIDATION. @@ -285,14 +403,52 @@ abort: static int gn_g2l(GENERAL_NAME *name, char **luri) { - char const *uri; - int error; + ASN1_STRING *asn1_string; + int type; - error = gn2uri(name, &uri); - if (error) - return error; + asn1_string = GENERAL_NAME_get0_value(name, &type); + + /* + * RFC 6487: "This extension MUST have an instance of an + * AccessDescription with an accessMethod of id-ad-rpkiManifest, (...) + * with an rsync URI [RFC5781] form of accessLocation." + * + * Ehhhhhh. It's a little annoying in that it seems to be stucking more + * than one requirement in a single sentence, which I think is rather + * rare for an RFC. Normally they tend to hammer things more. + * + * Does it imply that the GeneralName CHOICE is constrained to type + * "uniformResourceIdentifier"? I guess so, though I don't see anything + * stopping a few of the other types from also being capable of storing + * URIs. Then again, DER is all about unique serialized representation. + * + * Also, nobody seems to be using the other types, and handling them + * would be a titanic pain in the ass. So this is what I'm committing + * to. + * + * I know that this is the logical conclusion; it's just that I know + * that at some point in the future I'm going find myself bewilderingly + * staring at this if again. + */ + if (type != GEN_URI) { + pr_err("Unknown GENERAL_NAME type: %d", type); + return -ENOTSUPPORTED; + } - return uri_g2l(uri, luri); + /* + * GEN_URI signals an IA5String. + * IA5String is a subset of ASCII, so this cast is safe. + * No guarantees of a NULL chara, though. + * + * TODO (testers) According to RFC 5280, accessLocation can be an IRI + * somehow converted into URI form. I don't think that's an issue + * because the RSYNC clone operation should not have performed the + * conversion, so we should be looking at precisely the IA5String + * directory our g2l version of @asn1_string should contain. + * But ask the testers to keep an eye on it anyway. + */ + return uri_g2l((char const *) ASN1_STRING_get0_data(asn1_string), + ASN1_STRING_length(asn1_string), luri); } static int @@ -300,6 +456,7 @@ handle_ip_extension(X509_EXTENSION *ext, struct resources *resources) { ASN1_OCTET_STRING *string; struct IPAddrBlocks *blocks; + OCTET_STRING_t *family; int i; int error; @@ -310,17 +467,40 @@ handle_ip_extension(X509_EXTENSION *ext, struct resources *resources) return error; /* - * TODO There MUST be only one IPAddressFamily SEQUENCE per AFI. - * Each SEQUENCE MUST be ordered by ascending addressFamily values. + * rfc3779#section-2.2.3.3, rfc6487#section-4.8.10: + * We're expecting either one element (IPv4 or IPv6) or two elements + * (IPv4 then IPv6). */ - for (i = 0; i < blocks->list.count; i++) { - error = resources_add_ip(resources, blocks->list.array[i]); - if (error) - break; + switch (blocks->list.count) { + case 1: + break; + case 2: + family = &blocks->list.array[0]->addressFamily; + if (get_addr_family(family) != AF_INET) { + pr_err("First IP address block listed is not v4."); + goto einval; + } + family = &blocks->list.array[1]->addressFamily; + if (get_addr_family(family) != AF_INET6) { + pr_err("Second IP address block listed is not v6."); + goto einval; + } + break; + default: + pr_err("Got %d IP address blocks Expected; 1 or 2 expected.", + blocks->list.count); + goto einval; } + for (i = 0; i < blocks->list.count && !error; i++) + error = resources_add_ip(resources, blocks->list.array[i]); + ASN_STRUCT_FREE(asn_DEF_IPAddrBlocks, blocks); return error; + +einval: + ASN_STRUCT_FREE(asn_DEF_IPAddrBlocks, blocks); + return -EINVAL; } static int @@ -347,45 +527,121 @@ certificate_get_resources(X509 *cert, struct resources *resources) { X509_EXTENSION *ext; int i; - int error = 0; + int error; + bool ip_ext_found = false; + bool asn_ext_found = false; /* Reference: X509_get_ext_d2i */ - /* - * TODO ensure that each extension can only be found once. - * TODO rfc6487#section-2 also ensure that at least one IP or ASN - * extension is found. - * TODO rfc6487#section-2 ensure that the IP/ASN extensions are - * critical. - */ + /* rfc6487#section-2 */ for (i = 0; i < X509_get_ext_count(cert); i++) { ext = X509_get_ext(cert, i); switch (OBJ_obj2nid(X509_EXTENSION_get_object(ext))) { case NID_sbgp_ipAddrBlock: + if (ip_ext_found) { + pr_err("Multiple IP extensions found."); + return -EINVAL; + } + if (!X509_EXTENSION_get_critical(ext)) { + pr_err("The IP extension is not marked as critical."); + return -EINVAL; + } + pr_debug_add("IP {"); error = handle_ip_extension(ext, resources); pr_debug_rm("}"); + ip_ext_found = true; + + if (error) + return error; break; + case NID_sbgp_autonomousSysNum: + if (asn_ext_found) { + pr_err("Multiple AS extensions found."); + return -EINVAL; + } + if (!X509_EXTENSION_get_critical(ext)) { + pr_err("The AS extension is not marked as critical."); + return -EINVAL; + } + pr_debug_add("ASN {"); error = handle_asn_extension(ext, resources); pr_debug_rm("}"); + asn_ext_found = true; + + if (error) + return error; break; } + } - if (error) - return error; + if (!ip_ext_found && !asn_ext_found) { + pr_err("Certificate lacks both IP and AS extension."); + return -EINVAL; } + return 0; +} + +static int +handle_rpkiManifest(ACCESS_DESCRIPTION *ad, STACK_OF(X509_CRL) *crls) +{ + char *uri; + int error; + + error = gn_g2l(ad->location, &uri); + if (error) + return error; + + error = handle_manifest(uri, crls); + + free(uri); + return error; +} + +static int +handle_caRepository(ACCESS_DESCRIPTION *ad) +{ + char *uri; + int error; + + error = gn_g2l(ad->location, &uri); + if (error) + return error; + + pr_debug("caRepository: %s", uri); + + free(uri); + return error; +} + +static int +handle_signedObject(ACCESS_DESCRIPTION *ad) +{ + char *uri; + int error; + + error = gn_g2l(ad->location, &uri); + if (error) + return error; + + pr_debug("signedObject: %s", uri); + + free(uri); return error; } -int certificate_traverse(X509 *cert) +int +certificate_traverse_ca(X509 *cert, STACK_OF(X509_CRL) *crls) { + struct validation *state; SIGNATURE_INFO_ACCESS *sia; ACCESS_DESCRIPTION *ad; - char *uri; + bool manifest_found = false; + int nid; int i; int error; @@ -395,18 +651,71 @@ int certificate_traverse(X509 *cert) return -ESRCH; } - error = 0; + state = state_retrieve(); + if (state == NULL) { + error = -EINVAL; + goto end2; + } + error = validation_push_cert(state, cert); + if (error) + goto end2; + for (i = 0; i < sk_ACCESS_DESCRIPTION_num(sia); i++) { ad = sk_ACCESS_DESCRIPTION_value(sia, i); + nid = OBJ_obj2nid(ad->method); - if (OBJ_obj2nid(ad->method) == NID_rpkiManifest) { - error = gn_g2l(ad->location, &uri); + if (nid == NID_rpkiManifest) { + error = handle_rpkiManifest(ad, crls); if (error) - goto end; - error = handle_manifest(uri); - free(uri); + goto end1; + manifest_found = true; + + } else if (nid == NID_caRepository) { + error = handle_caRepository(ad); + if (error) + goto end1; + } + } + + /* rfc6481#section-2 */ + if (!manifest_found) { + pr_err("Repository publication point seems to have no manifest."); + error = -ESRCH; + } + +end1: + validation_pop_cert(state); /* Error code is useless. */ +end2: + AUTHORITY_INFO_ACCESS_free(sia); + return error; +} + +int +certificate_traverse_ee(X509 *cert) +{ + SIGNATURE_INFO_ACCESS *sia; + ACCESS_DESCRIPTION *ad; + int i; + int error; + + sia = X509_get_ext_d2i(cert, NID_sinfo_access, NULL, NULL); + if (sia == NULL) { + pr_err("Certificate lacks a Subject Information Access extension."); + return -ESRCH; + } + + for (i = 0; i < sk_ACCESS_DESCRIPTION_num(sia); i++) { + ad = sk_ACCESS_DESCRIPTION_value(sia, i); + if (OBJ_obj2nid(ad->method) == NID_signedObject) { + error = handle_signedObject(ad); if (error) goto end; + + } else { + /* rfc6487#section-4.8.8.2 */ + pr_err("EE Certificate has an non-signedObject access description."); + error = -EINVAL; + goto end; } } diff --git a/src/object/certificate.h b/src/object/certificate.h index 28e822fe..53066ed6 100644 --- a/src/object/certificate.h +++ b/src/object/certificate.h @@ -8,15 +8,31 @@ bool is_certificate(char const *); int certificate_load(const char *, X509 **); -/* - * Note: You actually need all three of these functions for a full validation; - * certificate_validate() checks the certificate's relationship with its - * parents, certificate_get_resources() covers the IP and ASN extensions, and - * you will need certificate_traverse() to walk through the children. +/** + * Performs the basic (RFC 5280, presumably) chain validation. + * (Ignores the IP, AS and SIA extensions.) */ +int certificate_validate_chain(X509 *, STACK_OF(X509_CRL) *); +/** + * Validates RFC 6487 compliance. + * (Except IP, AS and SIA extensions.) + */ +int certificate_validate_rfc6487(X509 *, bool); -int certificate_validate(X509 *, STACK_OF(X509_CRL) *); +/** + * Returns the IP and AS resources declared in the respective extensions. + */ int certificate_get_resources(X509 *, struct resources *); -int certificate_traverse(X509 *); +/** + * Handles the SIA extension, CA style. + * (ie. Recursively walks through the certificate's children.) + */ +int certificate_traverse_ca(X509 *, STACK_OF(X509_CRL) *); +/** + * Handles the SIA extension, EE style. + * (Doesn't actually "traverse" anything. The name is just for the sake of + * mirroring.) + */ +int certificate_traverse_ee(X509 *); #endif /* SRC_OBJECT_CERTIFICATE_H_ */ diff --git a/src/object/crl.c b/src/object/crl.c index 88968aa6..22c47e9a 100644 --- a/src/object/crl.c +++ b/src/object/crl.c @@ -2,6 +2,46 @@ #include "log.h" +static void +print_serials(X509_CRL *crl) +{ +#ifdef DEBUG + STACK_OF(X509_REVOKED) *revokeds; + X509_REVOKED *revoked; + ASN1_INTEGER const *serial_int; + BIGNUM *serial_bn; + int i; + + revokeds = X509_CRL_get_REVOKED(crl); + + for (i = 0; i < sk_X509_REVOKED_num(revokeds); i++) { + revoked = sk_X509_REVOKED_value(revokeds, i); + if (revoked == NULL) { + pr_err("??"); + continue; + } + + serial_int = X509_REVOKED_get0_serialNumber(revoked); + if (serial_int == NULL) { + pr_err("??"); + continue; + } + + serial_bn = ASN1_INTEGER_to_BN(serial_int, NULL); + if (serial_bn == NULL) { + crypto_err("Could not parse revoked serial number"); + continue; + } + + pr_debug_prefix(); + fprintf(stdout, "Revoked: "); + BN_print_fp(stdout, serial_bn); + fprintf(stdout, "\n"); + BN_free(serial_bn); + } +#endif +} + static int __crl_load(const char *file, X509_CRL **result) { @@ -23,6 +63,8 @@ __crl_load(const char *file, X509_CRL **result) goto end; } + print_serials(crl); + *result = crl; error = 0; @@ -36,7 +78,7 @@ crl_load(char const *file, X509_CRL **result) { int error; - pr_debug_add("CRL {"); + pr_debug_add("CRL %s {", file); error = __crl_load(file, result); pr_debug_rm("}"); diff --git a/src/object/manifest.c b/src/object/manifest.c index 0f103974..33034dce 100644 --- a/src/object/manifest.c +++ b/src/object/manifest.c @@ -112,7 +112,7 @@ validate_manifest(struct Manifest *manifest) if (!is_hash) return -EINVAL; - /* fileList needs no validations for now.*/ + /* TODO (critical) validate the file hashes */ return 0; } @@ -126,7 +126,8 @@ validate_manifest(struct Manifest *manifest) * The result needs to be freed in the end. */ static int -get_relative_file(char const *mft, char const *file, char **result) +get_relative_file(char const *mft, char const *file, size_t file_len, + char **result) { char *joined; char *slash_pos; @@ -134,20 +135,22 @@ get_relative_file(char const *mft, char const *file, char **result) slash_pos = strrchr(mft, '/'); if (slash_pos == NULL) { - joined = malloc(strlen(file) + 1); + joined = malloc(file_len + 1); if (!joined) return -ENOMEM; - strcpy(joined, file); + strncpy(joined, file, file_len); + joined[file_len] = '\0'; goto succeed; } dir_len = (slash_pos + 1) - mft; - joined = malloc(dir_len + strlen(file) + 1); + joined = malloc(dir_len + file_len + 1); if (!joined) return -ENOMEM; strncpy(joined, mft, dir_len); - strcpy(joined + dir_len, file); + strncpy(joined + dir_len, file, file_len); + joined[dir_len + file_len] = '\0'; succeed: *result = joined; @@ -156,25 +159,27 @@ succeed: typedef int (*foreach_cb)(char *, void *); -struct foreach_args { - STACK_OF(X509_CRL) *crls; - struct resources *resources; -}; - static int foreach_file(struct manifest *mft, char *extension, foreach_cb cb, void *arg) { char *uri; + size_t uri_len; char *luri; /* "Local URI". As in "URI that we can easily reference." */ int i; int error; for (i = 0; i < mft->obj->fileList.list.count; i++) { - /* TODO This cast is probably not correct. */ + /* + * IA5String is just a subset of ASCII, so this cast is fine. + * I don't see any guarantees that the string will be + * zero-terminated though, so we'll handle that the hard way. + */ uri = (char *) mft->obj->fileList.list.array[i]->file.buf; + uri_len = mft->obj->fileList.list.array[i]->file.size; - if (file_has_extension(uri, extension)) { - error = get_relative_file(mft->file_path, uri, &luri); + if (file_has_extension(uri, uri_len, extension)) { + error = get_relative_file(mft->file_path, uri, uri_len, + &luri); if (error) return error; error = cb(luri, arg); @@ -212,116 +217,90 @@ end: return error; } +/* + * Speaking of CA certs: I still don't get the CA/EE cert duality at the + * implementation level. + * + * Right now, I'm assuming that file certs are CA certs, and CMS-embedded certs + * are EE certs. None of the RFCs seem to mandate this, but I can't think of any + * other way to interpret it. + * + * It's really weird because the RFCs actually define requirements like "other + * AccessMethods MUST NOT be used for an EE certificates's SIA," (RFC6481) which + * seems to imply that there's some contextual way to already know whether a + * certificate is CA or EE. But it just doesn't exist. + */ static int -pile_addr_ranges(char *file, void *__args) +traverse_ca_certs(char *file, void *crls) { - struct foreach_args *args = __args; - struct resources *resources; X509 *cert; - int error = 0; - pr_debug_add("Certificate {"); + pr_debug_add("(CA?) Certificate {"); fnstack_push(file); /* - * Errors on some of these functions should not interrupt the tree - * traversal, so ignore them. + * Errors on at least some of these functions should not interrupt the + * traversal of sibling nodes, so ignore them. * (Error messages should have been printed in stderr.) */ if (certificate_load(file, &cert)) - goto end; /* Fine */ - - if (certificate_validate(cert, args->crls)) - goto revert; /* Fine */ - - resources = resources_create(); - if (resources == NULL) { - error = -ENOMEM; /* Not fine */ - goto revert; - } + goto revert1; /* Fine */ - if (certificate_get_resources(cert, resources)) + if (certificate_validate_chain(cert, crls)) goto revert2; /* Fine */ - - if (validation_push_cert(cert, resources)) { - /* - * Validation_push_cert() only fails on OPENSSL_sk_push(). - * The latter really only fails on memory allocation fault. - * That's grounds to interrupt tree traversal. - */ - error = -EINVAL; /* Not fine */ - goto revert2; - } - certificate_traverse(cert); /* Error code is useless. */ - validation_pop_cert(); /* Error code is useless. */ - - error = resources_join(args->resources, resources); /* Not fine */ + if (certificate_validate_rfc6487(cert, false)) + goto revert2; /* Fine */ + certificate_traverse_ca(cert, crls); /* Error code is useless. */ revert2: - resources_destroy(resources); -revert: X509_free(cert); -end: +revert1: pr_debug_rm("}"); fnstack_pop(); - return error; + return 0; } static int print_roa(char *file, void *arg) { - /* - * TODO to validate the ROA's cert, the parent cert must not have been - * popped at this point. - */ - handle_roa(file); + handle_roa(file, arg); return 0; } static int __handle_manifest(struct manifest *mft) { - struct foreach_args args; + STACK_OF(X509_CRL) *crls; int error; /* Init */ - args.crls = sk_X509_CRL_new_null(); - if (args.crls == NULL) { + crls = sk_X509_CRL_new_null(); + if (crls == NULL) { pr_err("Out of memory."); return -ENOMEM; } - args.resources = resources_create(); - if (args.resources == NULL) { - sk_X509_CRL_free(args.crls); - return -ENOMEM; - } - /* Get CRLs as a stack. There will usually only be one. */ - error = foreach_file(mft, "crl", pile_crls, args.crls); + error = foreach_file(mft, ".crl", pile_crls, crls); if (error) goto end; - /* - * Use CRL stack to validate certificates. - * Pile up valid address ranges from the valid certificates. - */ - error = foreach_file(mft, "cer", pile_addr_ranges, &args); + /* Use CRL stack to validate certificates, and also traverse them. */ + error = foreach_file(mft, ".cer", traverse_ca_certs, crls); if (error) goto end; /* Use valid address ranges to print ROAs that match them. */ - error = foreach_file(mft, "roa", print_roa, &args); + error = foreach_file(mft, ".roa", print_roa, crls); end: - resources_destroy(args.resources); - sk_X509_CRL_pop_free(args.crls, X509_CRL_free); + sk_X509_CRL_pop_free(crls, X509_CRL_free); return error; } int -handle_manifest(char const *file_path) +handle_manifest(char const *file_path, STACK_OF(X509_CRL) *crls) { static OID oid = OID_MANIFEST; struct oid_arcs arcs = OID2ARCS(oid); @@ -338,7 +317,7 @@ handle_manifest(char const *file_path) * certificate contains some. */ error = signed_object_decode(file_path, &asn_DEF_Manifest, &arcs, - (void **) &mft.obj, NULL); + (void **) &mft.obj, crls, NULL); if (error) goto end; diff --git a/src/object/manifest.h b/src/object/manifest.h index ab2bd7f1..f0a4240f 100644 --- a/src/object/manifest.h +++ b/src/object/manifest.h @@ -1,6 +1,8 @@ #ifndef SRC_OBJECT_MANIFEST_H_ #define SRC_OBJECT_MANIFEST_H_ -int handle_manifest(char const *); +#include + +int handle_manifest(char const *, STACK_OF(X509_CRL) *); #endif /* SRC_OBJECT_MANIFEST_H_ */ diff --git a/src/object/roa.c b/src/object/roa.c index 01193970..31e3aea1 100644 --- a/src/object/roa.c +++ b/src/object/roa.c @@ -151,7 +151,7 @@ family_error: return -EINVAL; } -int handle_roa(char const *file) +int handle_roa(char const *file, STACK_OF(X509_CRL) *crls) { static OID oid = OID_ROA; struct oid_arcs arcs = OID2ARCS(oid); @@ -171,7 +171,7 @@ int handle_roa(char const *file) } error = signed_object_decode(file, &asn_DEF_RouteOriginAttestation, - &arcs, (void **) &roa, cert_resources); + &arcs, (void **) &roa, crls, cert_resources); if (error) goto end2; error = __handle_roa(roa, cert_resources); diff --git a/src/object/roa.h b/src/object/roa.h index ff134268..7fffc4e5 100644 --- a/src/object/roa.h +++ b/src/object/roa.h @@ -1,6 +1,8 @@ #ifndef SRC_OBJECT_ROA_H_ #define SRC_OBJECT_ROA_H_ -int handle_roa(char const *); +#include + +int handle_roa(char const *, STACK_OF(X509_CRL) *); #endif /* SRC_OBJECT_ROA_H_ */ diff --git a/src/object/signed_object.c b/src/object/signed_object.c index 0cc7acee..b3fbbf47 100644 --- a/src/object/signed_object.c +++ b/src/object/signed_object.c @@ -62,6 +62,7 @@ signed_object_decode(char const *file, asn_TYPE_descriptor_t const *descriptor, struct oid_arcs const *oid, void **result, + STACK_OF(X509_CRL) *crls, struct resources *res) { struct ContentInfo *cinfo; @@ -72,7 +73,7 @@ signed_object_decode(char const *file, if (error) goto end1; - error = signed_data_decode(&cinfo->content, &sdata, res); + error = signed_data_decode(&cinfo->content, &sdata, crls, res); if (error) goto end2; diff --git a/src/object/signed_object.h b/src/object/signed_object.h index 9565d513..2c5abf79 100644 --- a/src/object/signed_object.h +++ b/src/object/signed_object.h @@ -1,10 +1,11 @@ #ifndef SRC_OBJECT_SIGNED_OBJECT_H_ #define SRC_OBJECT_SIGNED_OBJECT_H_ +#include #include "asn1/oid.h" #include "resource.h" int signed_object_decode(char const *, asn_TYPE_descriptor_t const *, - struct oid_arcs const *, void **, struct resources *); + struct oid_arcs const *, void **, STACK_OF(X509_CRL) *, struct resources *); #endif /* SRC_OBJECT_SIGNED_OBJECT_H_ */ diff --git a/src/object/tal.c b/src/object/tal.c index 1aaa1b62..5e0e0d42 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -1,11 +1,14 @@ #include "tal.h" -#include #include #include #include #include +#include +#include +#include +#include "base64.h" #include "line_file.h" #include "log.h" @@ -18,9 +21,8 @@ SLIST_HEAD(uri_list, uri); struct tal { struct uri_list uris; - /* Decoded; not base64. */ - void *spki; - size_t spki_size; + unsigned char *spki; /* Decoded; not base64. */ + size_t spki_len; }; static void @@ -100,6 +102,95 @@ read_uris(struct line_file *lfile, struct uri_list *uris) } while (true); } +/* + * Will usually allocate slightly more because of the newlines, but I'm fine + * with it. + */ +static size_t +get_spki_alloc_size(struct line_file *lfile) +{ + struct stat st; + size_t result; + + stat(lfile_name(lfile), &st); + result = st.st_size - lfile_offset(lfile); + + return EVP_DECODE_LENGTH(result); +} + +static int +lf2bio(struct line_file *lfile, BIO **result) +{ + BIO *bio; + char *line; + size_t line_len; + size_t written; + int error; + + bio = BIO_new(BIO_s_mem()); + if (bio == NULL) { + pr_err("Out of memory."); + return -ENOMEM; + } + + *result = NULL; + do { + line = NULL; + error = lfile_read(lfile, &line); + if (error) { + BIO_free(bio); + return error; + } + if (line == NULL) { + *result = bio; + return 0; + } + + line_len = strlen(line); + if (line_len == 0) { + free(line); + /* TODO maybe we're supposed to abort instead */ + continue; + } + + /* TODO error out if written != line_len? */ + + written = BIO_write(bio, line, line_len); + free(line); + if (written <= 0) { + BIO_free(bio); + return crypto_err("Could not write into memory BIO"); + } + + } while (true); +} + +static int +read_spki(struct line_file *lfile, struct tal *tal) +{ + BIO *encoded; /* base64 encoded. */ + size_t alloc_size; + int error; + + alloc_size = get_spki_alloc_size(lfile); + tal->spki = malloc(alloc_size); + if (tal->spki == NULL) + return -ENOMEM; + + error = lf2bio(lfile, &encoded); + if (error) { + free(tal->spki); + return error; + } + + error = base64_decode(encoded, tal->spki, alloc_size, &tal->spki_len); + if (error) + free(tal->spki); + + BIO_free(encoded); + return error; +} + int tal_load(const char *file_name, struct tal **result) { @@ -124,9 +215,13 @@ tal_load(const char *file_name, struct tal **result) return err; } - /* TODO */ - tal->spki = NULL; - tal->spki_size = 0; + err = read_spki(lfile, tal); + if (err) { + uris_destroy(&tal->uris); + free(tal); + lfile_close(lfile); + return err; + } lfile_close(lfile); *result = tal; @@ -150,10 +245,17 @@ foreach_uri(struct tal *tal, foreach_uri_cb cb) int error; SLIST_FOREACH(cursor, &tal->uris, next) { - error = cb(cursor->string); + error = cb(tal, cursor->string); if (error) return error; } return 0; } + +void +tal_get_spki(struct tal *tal, unsigned char const **buffer, size_t *len) +{ + *buffer = tal->spki; + *len = tal->spki_len; +} diff --git a/src/object/tal.h b/src/object/tal.h index 9e897042..d20a9025 100644 --- a/src/object/tal.h +++ b/src/object/tal.h @@ -3,12 +3,16 @@ /* This is RFC 7730. */ +#include + struct tal; int tal_load(const char *, struct tal **); void tal_destroy(struct tal *); -typedef int (*foreach_uri_cb)(char const *); +typedef int (*foreach_uri_cb)(struct tal *, char const *); int foreach_uri(struct tal *, foreach_uri_cb); +void tal_get_spki(struct tal *, unsigned char const **, size_t *); + #endif /* TAL_OBJECT_H_ */ diff --git a/src/resource.c b/src/resource.c index 29885faa..0cb2a040 100644 --- a/src/resource.c +++ b/src/resource.c @@ -60,7 +60,7 @@ resources_destroy(struct resources *resources) free(resources); } -static int +int get_addr_family(OCTET_STRING_t *octets) { if (octets->size != 2) { @@ -92,7 +92,7 @@ get_parent_resources(void) } static void -pr_debug_prefix(int family, void *addr, unsigned int length) +pr_debug_ip_prefix(int family, void *addr, unsigned int length) { #ifdef DEBUG char buffer[INET6_ADDRSTRLEN]; @@ -156,6 +156,7 @@ inherit_aors(struct resources *resources, int family) } resources->ip4s = parent->ip4s; res4_get(resources->ip4s); + pr_debug(""); return 0; case AF_INET6: @@ -169,6 +170,7 @@ inherit_aors(struct resources *resources, int family) } resources->ip6s = parent->ip6s; res6_get(resources->ip6s); + pr_debug(""); return 0; } @@ -214,7 +216,7 @@ add_prefix4(struct resources *resources, IPAddress2_t *addr) return error; } - pr_debug_prefix(AF_INET, &prefix.addr, prefix.len); + pr_debug_ip_prefix(AF_INET, &prefix.addr, prefix.len); return 0; } @@ -256,7 +258,7 @@ add_prefix6(struct resources *resources, IPAddress2_t *addr) return error; } - pr_debug_prefix(AF_INET6, &prefix.addr, prefix.len); + pr_debug_ip_prefix(AF_INET6, &prefix.addr, prefix.len); return 0; } @@ -452,6 +454,7 @@ inherit_asiors(struct resources *resources) } resources->asns = parent->asns; rasn_get(resources->asns); + pr_debug(""); return 0; } @@ -569,30 +572,6 @@ resources_contains_ipv6(struct resources *res, struct ipv6_prefix *prefix) return res6_contains_prefix(res->ip6s, prefix); } -int -resources_join(struct resources *r1, struct resources *r2) -{ - int error; - - if (r1->ip4s != NULL) { - error = res4_join(r1->ip4s, r2->ip4s); - if (error) - return error; - } - if (r1->ip6s != NULL) { - error = res6_join(r1->ip6s, r2->ip6s); - if (error) - return error; - } - if (r1->asns != NULL) { - error = rasn_join(r1->asns, r2->asns); - if (error) - return error; - } - - return 0; -} - struct restack * restack_create(void) { diff --git a/src/resource.h b/src/resource.h index 678c3b43..f70c9549 100644 --- a/src/resource.h +++ b/src/resource.h @@ -6,6 +6,8 @@ #include #include "address.h" +int get_addr_family(OCTET_STRING_t *); + struct resources; struct resources *resources_create(void); @@ -18,8 +20,7 @@ bool resources_contains_asn(struct resources *, ASId_t); bool resources_contains_ipv4(struct resources *, struct ipv4_prefix *); bool resources_contains_ipv6(struct resources *, struct ipv6_prefix *); -int resources_join(struct resources *, struct resources *); - +/* "Resource stack." Stack of struct resources. */ struct restack; struct restack *restack_create(void); diff --git a/src/resource/asn.c b/src/resource/asn.c index de51d4d2..1dd436a8 100644 --- a/src/resource/asn.c +++ b/src/resource/asn.c @@ -65,10 +65,3 @@ rasn_contains(struct resources_asn *asns, ASId_t min, ASId_t max) struct asn_node n = { min, max }; return sarray_contains((struct sorted_array *) asns, &n); } - -int -rasn_join(struct resources_asn *r1, struct resources_asn *r2) -{ - return sarray_join((struct sorted_array *) r1, - (struct sorted_array *) r2); -} diff --git a/src/resource/asn.h b/src/resource/asn.h index 5805d634..0d812bf2 100644 --- a/src/resource/asn.h +++ b/src/resource/asn.h @@ -12,6 +12,5 @@ void rasn_put(struct resources_asn *); int rasn_add(struct resources_asn *, ASId_t, ASId_t); bool rasn_contains(struct resources_asn *, ASId_t, ASId_t); -int rasn_join(struct resources_asn *, struct resources_asn *); #endif /* SRC_RESOURCE_ASN_H_ */ diff --git a/src/resource/ip4.c b/src/resource/ip4.c index a81f5002..4612b635 100644 --- a/src/resource/ip4.c +++ b/src/resource/ip4.c @@ -96,10 +96,3 @@ res4_contains_range(struct resources_ipv4 *ips, struct ipv4_range *range) rton(range, &n); return sarray_contains((struct sorted_array *) ips, &n); } - -int -res4_join(struct resources_ipv4 *r1, struct resources_ipv4 *r2) -{ - return sarray_join((struct sorted_array *) r1, - (struct sorted_array *) r2); -} diff --git a/src/resource/ip4.h b/src/resource/ip4.h index 7d2fbf11..8abf0b27 100644 --- a/src/resource/ip4.h +++ b/src/resource/ip4.h @@ -14,6 +14,5 @@ int res4_add_prefix(struct resources_ipv4 *, struct ipv4_prefix *); int res4_add_range(struct resources_ipv4 *, struct ipv4_range *); bool res4_contains_prefix(struct resources_ipv4 *, struct ipv4_prefix *); bool res4_contains_range(struct resources_ipv4 *, struct ipv4_range *); -int res4_join(struct resources_ipv4 *, struct resources_ipv4 *); #endif /* SRC_RESOURCE_IP4_H_ */ diff --git a/src/resource/ip6.c b/src/resource/ip6.c index 2dbfcf3b..42346554 100644 --- a/src/resource/ip6.c +++ b/src/resource/ip6.c @@ -106,10 +106,3 @@ res6_contains_range(struct resources_ipv6 *ips, struct ipv6_range *range) { return sarray_contains((struct sorted_array *) ips, range); } - -int -res6_join(struct resources_ipv6 *r1, struct resources_ipv6 *r2) -{ - return sarray_join((struct sorted_array *) r1, - (struct sorted_array *) r2); -} diff --git a/src/resource/ip6.h b/src/resource/ip6.h index 96b39f42..74aa40ce 100644 --- a/src/resource/ip6.h +++ b/src/resource/ip6.h @@ -14,6 +14,5 @@ int res6_add_prefix(struct resources_ipv6 *ps, struct ipv6_prefix *); int res6_add_range(struct resources_ipv6 *, struct ipv6_range *); bool res6_contains_prefix(struct resources_ipv6 *, struct ipv6_prefix *); bool res6_contains_range(struct resources_ipv6 *, struct ipv6_range *); -int res6_join(struct resources_ipv6 *, struct resources_ipv6 *); #endif /* SRC_RESOURCE_IP6_H_ */ diff --git a/src/sorted_array.c b/src/sorted_array.c index cd378c29..3e8e28af 100644 --- a/src/sorted_array.c +++ b/src/sorted_array.c @@ -127,53 +127,6 @@ sarray_add(struct sorted_array *sarray, void *element) return 0; } -/* https://stackoverflow.com/questions/364985 */ -static int -pow2roundup(unsigned int x) -{ - --x; - x |= x >> 1; - x |= x >> 2; - x |= x >> 4; - x |= x >> 8; - x |= x >> 16; - return x + 1; -} - -/* - * Appends the elements from @addend to @sarray's tail. - */ -int -sarray_join(struct sorted_array *sarray, struct sorted_array *addend) -{ - int error; - unsigned int new_count; - size_t new_len; - void *tmp; - - if (addend == NULL || addend->count == 0) - return 0; - - error = compare(sarray, addend->array); - if (error) - return error; - - new_count = sarray->count + addend->count; - if (new_count > sarray->len) { - new_len = pow2roundup(new_count); - tmp = realloc(sarray->array, new_len * sarray->size); - if (tmp == NULL) - return -ENOMEM; - sarray->array = tmp; - sarray->len = new_len; - } - - memcpy(get_nth_element(sarray, sarray->count), addend->array, - new_count * sarray->size); - sarray->count += addend->count; - return 0; -} - bool sarray_contains(struct sorted_array *sarray, void *elem) { diff --git a/src/sorted_array.h b/src/sorted_array.h index 62d820d3..92281f02 100644 --- a/src/sorted_array.h +++ b/src/sorted_array.h @@ -39,7 +39,6 @@ void sarray_put(struct sorted_array *); #define EINTERSECTION 7900 int sarray_add(struct sorted_array *, void *); -int sarray_join(struct sorted_array *, struct sorted_array *); bool sarray_contains(struct sorted_array *, void *); char const *sarray_err2str(int); diff --git a/src/state.c b/src/state.c index 5eab07b8..c9ada7cc 100644 --- a/src/state.c +++ b/src/state.c @@ -13,11 +13,7 @@ * uses it to traverse the tree and keep track of validated data. */ struct validation { - /** - * Encapsulated standard error. - * Needed because the crypto library won't write to stderr directly. - */ - BIO *err; + struct tal *tal; /** https://www.openssl.org/docs/man1.1.1/man3/X509_STORE_load_locations.html */ X509_STORE *store; @@ -37,8 +33,6 @@ struct validation { * seemingly not intended to be used outside of its library.) */ struct restack *rsrcs; - - struct filename_stack *files; }; /* @@ -76,49 +70,11 @@ cb(int ok, X509_STORE_CTX *ctx) return (error == X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION) ? 1 : ok; } -static int -init_trusted(struct validation *result, char *root) -{ - X509 *cert; - int ok; - int error; - - fnstack_push(root); - - error = certificate_load(root, &cert); - if (error) - goto abort1; - - result->trusted = sk_X509_new_null(); - if (result->trusted == NULL) { - error = -EINVAL; - goto abort2; - } - - ok = sk_X509_push(result->trusted, cert); - if (ok <= 0) { - error = crypto_err( - "Could not add certificate to trusted stack: %d", ok); - goto abort3; - } - - fnstack_pop(); - return 0; - -abort3: - sk_X509_free(result->trusted); -abort2: - X509_free(cert); -abort1: - fnstack_pop(); - return error; -} - +/** Creates a struct validation, puts it in thread local, and returns it. */ int -validation_create(struct validation **out, char *root) +validation_prepare(struct validation **out, struct tal *tal) { struct validation *result; - struct resources *resources; int error; result = malloc(sizeof(struct validation)); @@ -129,54 +85,35 @@ validation_create(struct validation **out, char *root) if (error) goto abort1; - result->err = BIO_new_fp(stderr, BIO_NOCLOSE); - if (result->err == NULL) { - fprintf(stderr, "Failed to initialise standard error's BIO.\n"); - error = -ENOMEM; - goto abort1; - } + result->tal = tal; result->store = X509_STORE_new(); if (!result->store) { error = crypto_err("X509_STORE_new() returned NULL"); - goto abort2; + goto abort1; } X509_STORE_set_verify_cb(result->store, cb); - error = init_trusted(result, root); - if (error) - goto abort3; + result->trusted = sk_X509_new_null(); + if (result->trusted == NULL) { + error = crypto_err("sk_X509_new_null() returned NULL"); + goto abort2; + } result->rsrcs = restack_create(); - if (!result->rsrcs) - goto abort4; - - resources = resources_create(); - if (resources == NULL) - goto abort5; - - fnstack_push(root); - error = certificate_get_resources(validation_peek_cert(result), - resources); - fnstack_pop(); - if (error) - goto abort6; + if (!result->rsrcs) { + error = -ENOMEM; + goto abort3; + } - restack_push(result->rsrcs, resources); *out = result; return 0; -abort6: - resources_destroy(resources); -abort5: - restack_destroy(result->rsrcs); -abort4: - sk_X509_pop_free(result->trusted, X509_free); abort3: - X509_STORE_free(result->store); + sk_X509_pop_free(result->trusted, X509_free); abort2: - BIO_free_all(result->err); + X509_STORE_free(result->store); abort1: free(result); return error; @@ -187,27 +124,22 @@ validation_destroy(struct validation *state) { int cert_num; - /* - * Only the certificate created during validation_create() should - * remain. - */ cert_num = sk_X509_num(state->trusted); - if (cert_num != 1) { - pr_err("Error: validation state has %d certificates. (1 expected)", + if (cert_num != 0) { + pr_err("Error: validation state has %d certificates. (0 expected)", cert_num); } restack_destroy(state->rsrcs); sk_X509_pop_free(state->trusted, X509_free); X509_STORE_free(state->store); - BIO_free_all(state->err); free(state); } -BIO * -validation_stderr(struct validation *state) +struct tal * +validation_tal(struct validation *state) { - return state->err; + return state->tal; } X509_STORE * @@ -229,53 +161,55 @@ validation_resources(struct validation *state) } int -validation_push_cert(X509 *cert, struct resources *resources) +validation_push_cert(struct validation *state, X509 *cert) { - struct validation *state; + struct resources *resources; int ok; + int error; - state = state_retrieve(); - if (state == NULL) - return -EINVAL; + resources = resources_create(); + if (resources == NULL) + return -ENOMEM; + + error = certificate_get_resources(cert, resources); + if (error) + goto fail; ok = sk_X509_push(state->trusted, cert); if (ok <= 0) { - crypto_err("Couldn't add certificate to trusted stack: %d", ok); - return -ENOMEM; /* Presumably */ + error = crypto_err( + "Couldn't add certificate to trusted stack: %d", ok); + goto fail; } restack_push(state->rsrcs, resources); - return 0; + +fail: + resources_destroy(resources); + return error; } int -validation_pop_cert(void) +validation_pop_cert(struct validation *state) { - struct validation *state; - - state = state_retrieve(); - if (state == NULL) - return -EINVAL; + struct resources *resources; if (sk_X509_pop(state->trusted) == NULL) { return crypto_err( "Programming error: Attempted to pop empty cert stack"); } - if (restack_pop(state->rsrcs) == NULL) { + + resources = restack_pop(state->rsrcs); + if (resources == NULL) { pr_err("Programming error: Attempted to pop empty resource stack"); return -EINVAL; } + resources_destroy(resources); return 0; } -X509 * -validation_peek_cert(struct validation *state) -{ - return sk_X509_value(state->trusted, sk_X509_num(state->trusted) - 1); -} - struct resources * validation_peek_resource(struct validation *state) { diff --git a/src/state.h b/src/state.h index 2c4f18fe..9b5ac89f 100644 --- a/src/state.h +++ b/src/state.h @@ -1,23 +1,22 @@ #ifndef SRC_STATE_H_ #define SRC_STATE_H_ -#include #include #include "resource.h" +#include "object/tal.h" struct validation; -int validation_create(struct validation **, char *); +int validation_prepare(struct validation **, struct tal *); void validation_destroy(struct validation *); -BIO *validation_stderr(struct validation *); +struct tal *validation_tal(struct validation *); X509_STORE *validation_store(struct validation *); STACK_OF(X509) *validation_certs(struct validation *); struct restack *validation_resources(struct validation *); -int validation_push_cert(X509 *, struct resources *); -int validation_pop_cert(void); -X509 *validation_peek_cert(struct validation *); +int validation_push_cert(struct validation *, X509 *); +int validation_pop_cert(struct validation *); struct resources *validation_peek_resource(struct validation *); diff --git a/src/thread_var.c b/src/thread_var.c index 068dcacf..dd7ca943 100644 --- a/src/thread_var.c +++ b/src/thread_var.c @@ -68,7 +68,7 @@ state_retrieve(void) state = pthread_getspecific(state_key); if (state == NULL) - fprintf(stderr, "This thread lacks a validation state.\n"); + fprintf(stderr, "Programming error: This thread lacks a validation state.\n"); return state; } diff --git a/test/tal_test.c b/test/tal_test.c index 19da1782..4f6b9623 100644 --- a/test/tal_test.c +++ b/test/tal_test.c @@ -8,11 +8,8 @@ START_TEST(tal_load_normal) { struct tal *tal; struct uri *uri; - /* unsigned int i; - */ /* Got this by feeding the subjectPublicKeyInfo to `base64 -d`. */ - /* unsigned char decoded[] = { 0x30, 0x82, 0x01, 0x22, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, @@ -42,7 +39,6 @@ START_TEST(tal_load_normal) 0xD4, 0x32, 0xB7, 0x11, 0x38, 0x71, 0xCF, 0xF3, 0xA4, 0x0F, 0x64, 0x83, 0x63, 0x0D, 0x02, 0x03, 0x01, 0x00, 0x01 }; - */ ck_assert_int_eq(tal_load("tal/lacnic.tal", &tal), 0); @@ -55,11 +51,9 @@ START_TEST(tal_load_normal) uri = SLIST_NEXT(uri, next); ck_assert(uri == NULL); - /* - ck_assert_uint_eq(ARRAY_LEN(decoded), tal->spki_size); + ck_assert_uint_eq(ARRAY_LEN(decoded), tal->spki_len); for (i = 0; i < ARRAY_LEN(decoded); i++) ck_assert_uint_eq(tal->spki[i], decoded[i]); - */ tal_destroy(tal); }