From 63cd39a56d2a9d8c5ef95b1ef72d0c0d6e5359a3 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Thu, 20 Dec 2018 11:09:17 -0600 Subject: [PATCH] Validate signed object hashes And address several other minor TODOs --- src/Makefile.am | 5 +- src/address.c | 33 +++---- src/asn1/decode.c | 11 +-- src/asn1/oid.c | 24 ++--- src/asn1/oid.h | 11 +-- src/asn1/signed_data.c | 160 ++++++++++++++------------------- src/common.c | 6 +- src/{ => crypto}/base64.c | 3 - src/{ => crypto}/base64.h | 0 src/crypto/hash.c | 173 ++++++++++++++++++++++++++++++++++++ src/crypto/hash.h | 13 +++ src/file.c | 83 +++++++++-------- src/file.h | 5 ++ src/hash.c | 119 ------------------------- src/hash.h | 9 -- src/line_file.c | 26 +++--- src/line_file.h | 2 + src/log.c | 30 ++++++- src/log.h | 5 +- src/main.c | 8 +- src/object/certificate.c | 97 +++++++------------- src/object/manifest.c | 45 +++------- src/object/roa.c | 56 +++++------- src/object/signed_object.c | 6 +- src/object/tal.c | 66 ++------------ src/resource.c | 176 +++++++++++++------------------------ src/sorted_array.c | 5 +- src/state.c | 12 +-- 28 files changed, 533 insertions(+), 656 deletions(-) rename src/{ => crypto}/base64.c (95%) rename src/{ => crypto}/base64.h (100%) create mode 100644 src/crypto/hash.c create mode 100644 src/crypto/hash.h delete mode 100644 src/hash.c delete mode 100644 src/hash.h diff --git a/src/Makefile.am b/src/Makefile.am index 8da3e429..4a29af19 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -7,11 +7,9 @@ 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 -rpki_validator_SOURCES += hash.h hash.c rpki_validator_SOURCES += line_file.h line_file.c rpki_validator_SOURCES += log.h log.c rpki_validator_SOURCES += resource.h resource.c @@ -24,6 +22,9 @@ rpki_validator_SOURCES += asn1/decode.h asn1/decode.c rpki_validator_SOURCES += asn1/oid.h asn1/oid.c rpki_validator_SOURCES += asn1/signed_data.h asn1/signed_data.c +rpki_validator_SOURCES += crypto/base64.h crypto/base64.c +rpki_validator_SOURCES += crypto/hash.h crypto/hash.c + rpki_validator_SOURCES += object/certificate.h object/certificate.c rpki_validator_SOURCES += object/crl.h object/crl.c rpki_validator_SOURCES += object/manifest.h object/manifest.c diff --git a/src/address.c b/src/address.c index 3c276eb9..6aae8f42 100644 --- a/src/address.c +++ b/src/address.c @@ -42,13 +42,12 @@ prefix4_decode(IPAddress2_t *str, struct ipv4_prefix *result) int len; if (str->size > 4) { - pr_err("IPv4 address has too many octets. (%u)", str->size); - return -EINVAL; + return pr_err("IPv4 address has too many octets. (%u)", + str->size); } if (str->bits_unused < 0 || 7 < str->bits_unused) { - pr_err("Bit string IPv4 address's unused bits count (%d) is out of range (0-7).", + return pr_err("Bit string IPv4 address's unused bits count (%d) is out of range (0-7).", str->bits_unused); - return -EINVAL; } memset(&result->addr, 0, sizeof(result->addr)); @@ -56,16 +55,14 @@ prefix4_decode(IPAddress2_t *str, struct ipv4_prefix *result) len = 8 * str->size - str->bits_unused; if (len < 0 || 32 < len) { - pr_err("IPv4 prefix length (%d) is out of bounds (0-32).", len); - return -EINVAL; + return pr_err("IPv4 prefix length (%d) is out of bounds (0-32).", + len); } result->len = len; - if ((result->addr.s_addr & ipv4_suffix_mask(result->len)) != 0) { - pr_err("IPv4 prefix has enabled suffix bits."); - return -EINVAL; - } + if ((result->addr.s_addr & ipv4_suffix_mask(result->len)) != 0) + return pr_err("IPv4 prefix has enabled suffix bits."); return 0; } @@ -77,13 +74,12 @@ prefix6_decode(IPAddress2_t *str, struct ipv6_prefix *result) int len; if (str->size > 16) { - pr_err("IPv6 address has too many octets. (%u)", str->size); - return -EINVAL; + return pr_err("IPv6 address has too many octets. (%u)", + str->size); } if (str->bits_unused < 0 || 7 < str->bits_unused) { - pr_err("Bit string IPv6 address's unused bits count (%d) is out of range (0-7).", + return pr_err("Bit string IPv6 address's unused bits count (%d) is out of range (0-7).", str->bits_unused); - return -EINVAL; } memset(&result->addr, 0, sizeof(result->addr)); @@ -91,9 +87,8 @@ prefix6_decode(IPAddress2_t *str, struct ipv6_prefix *result) len = 8 * str->size - str->bits_unused; if (len < 0 || 128 < len) { - pr_err("IPv6 prefix length (%u) is out of bounds (0-128).", + return pr_err("IPv6 prefix length (%u) is out of bounds (0-128).", len); - return -EINVAL; } result->len = len; @@ -103,10 +98,8 @@ prefix6_decode(IPAddress2_t *str, struct ipv6_prefix *result) if ( (result->addr.s6_addr32[0] & suffix.s6_addr32[0]) || (result->addr.s6_addr32[1] & suffix.s6_addr32[1]) || (result->addr.s6_addr32[2] & suffix.s6_addr32[2]) - || (result->addr.s6_addr32[3] & suffix.s6_addr32[3])) { - pr_err("IPv6 prefix has enabled suffix bits."); - return -EINVAL; - } + || (result->addr.s6_addr32[3] & suffix.s6_addr32[3])) + return pr_err("IPv6 prefix has enabled suffix bits."); return 0; } diff --git a/src/asn1/decode.c b/src/asn1/decode.c index 9aa4cb39..087848cf 100644 --- a/src/asn1/decode.c +++ b/src/asn1/decode.c @@ -15,10 +15,8 @@ validate(asn_TYPE_descriptor_t const *descriptor, void *result) error_msg_size = sizeof(error_msg); error = asn_check_constraints(descriptor, result, error_msg, &error_msg_size); - if (error == -1) { - pr_err("Error validating ASN.1 object: %s", error_msg); - return -EINVAL; - } + if (error == -1) + return pr_err("Error validating ASN.1 object: %s", error_msg); return 0; } @@ -34,11 +32,10 @@ asn1_decode(const void *buffer, size_t buffer_size, rval = ber_decode(0, descriptor, result, buffer, buffer_size); if (rval.code != RC_OK) { - /* TODO if rval.code == RC_WMORE (1), more work is needed */ - pr_err("Error decoding ASN.1 object: %d", rval.code); /* Must free partial object according to API contracts. */ ASN_STRUCT_FREE(*descriptor, *result); - return -EINVAL; + /* TODO if rval.code == RC_WMORE (1), more work is needed */ + return pr_err("Error decoding ASN.1 object: %d", rval.code); } error = validate(descriptor, *result); diff --git a/src/asn1/oid.c b/src/asn1/oid.c index bfb8cc3c..6828628a 100644 --- a/src/asn1/oid.c +++ b/src/asn1/oid.c @@ -5,8 +5,6 @@ #include "log.h" #include "asn1/decode.h" -#define MAX_ARCS 9 - void free_arcs(struct oid_arcs *arcs) { @@ -21,13 +19,13 @@ free_arcs(struct oid_arcs *arcs) int oid2arcs(OBJECT_IDENTIFIER_t *oid, struct oid_arcs *result) { - ssize_t count, count2; + static const size_t MAX_ARCS = 9; + ssize_t count; + ssize_t count2; result->arcs = malloc(MAX_ARCS * sizeof(asn_oid_arc_t)); - if (result->arcs == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (result->arcs == NULL) + return pr_enomem(); count = OBJECT_IDENTIFIER_get_arcs(oid, result->arcs, MAX_ARCS); if (count < 0) { @@ -41,10 +39,9 @@ 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) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (!result->arcs) + return pr_enomem(); + count2 = OBJECT_IDENTIFIER_get_arcs(oid, result->arcs, count); if (count != count2) { pr_err("OBJECT_IDENTIFIER_get_arcs() returned %zd. (expected %zd)", @@ -74,6 +71,11 @@ any2arcs(ANY_t *any, struct oid_arcs *result) return error; } +bool oid_equal(OBJECT_IDENTIFIER_t *a, OBJECT_IDENTIFIER_t *b) +{ + return (a->size == b->size) && (memcmp(a->buf, b->buf, a->size) == 0); +} + static bool __arcs_equal(asn_oid_arc_t const *a, size_t a_count, asn_oid_arc_t const *b, size_t b_count) { diff --git a/src/asn1/oid.h b/src/asn1/oid.h index 820ff1c9..da9e9984 100644 --- a/src/asn1/oid.h +++ b/src/asn1/oid.h @@ -19,9 +19,9 @@ void free_arcs(struct oid_arcs *); typedef asn_oid_arc_t OID[]; /* - * !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - * ! Please update MAX_ARCS if you add an OID that has more arcs. ! - * !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + * !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + * ! Please update oid2arcs().MAX_ARCS if you add an OID that has more arcs. ! + * !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */ #define OID_SIGNED_DATA { 1, 2, 840, 113549, 1, 7, 2 } @@ -34,15 +34,12 @@ typedef asn_oid_arc_t OID[]; #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 } -#define OID_SHA512 { 2, 16, 840, 1, 101, 3, 4, 2, 3 } int oid2arcs(OBJECT_IDENTIFIER_t *, struct oid_arcs *); int any2arcs(ANY_t *, struct oid_arcs *); +bool oid_equal(OBJECT_IDENTIFIER_t *, OBJECT_IDENTIFIER_t *); bool arcs_equal(struct oid_arcs const *, struct oid_arcs const *); /* Use ARCS_EQUAL_OID() instead. */ bool arcs_equal_oids(struct oid_arcs *, asn_oid_arc_t const *, size_t); diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c index 155d6ade..d1a8497e 100644 --- a/src/asn1/signed_data.c +++ b/src/asn1/signed_data.c @@ -2,42 +2,34 @@ #include #include +#include + #include "log.h" #include "oid.h" #include "asn1/decode.h" +#include "crypto/hash.h" #include "object/certificate.h" -/* TODO more consistent and informative error/warning messages.*/ - -static const OID oid_sha224 = OID_SHA224; -static const OID oid_sha256 = OID_SHA256; -static const OID oid_sha384 = OID_SHA384; -static const OID oid_sha512 = OID_SHA512; static const OID oid_cta = OID_CONTENT_TYPE_ATTR; static const OID oid_mda = OID_MESSAGE_DIGEST_ATTR; static const OID oid_sta = OID_SIGNING_TIME_ATTR; static const OID oid_bsta = OID_BINARY_SIGNING_TIME_ATTR; -/* - * The correctness of this function depends on @MAX_ARCS being faithful to all - * the known OIDs declared *in the project*. - */ static int -is_digest_algorithm(AlgorithmIdentifier_t *aid, bool *result) +is_digest_algorithm(AlgorithmIdentifier_t *id, char *what) { - struct oid_arcs arcs; + bool is_hash; int error; - error = oid2arcs(&aid->algorithm, &arcs); + if (id == NULL) + return pr_err("The %s algorithm is NULL.", what); + + error = hash_is_valid_algorithm(&id->algorithm, &is_hash); if (error) return error; + if (!is_hash) + return pr_err("The %s algorithm is not SHA256.", what); - *result = ARCS_EQUAL_OIDS(&arcs, oid_sha224) - || ARCS_EQUAL_OIDS(&arcs, oid_sha256) - || ARCS_EQUAL_OIDS(&arcs, oid_sha384) - || ARCS_EQUAL_OIDS(&arcs, oid_sha512); - - free_arcs(&arcs); return 0; } @@ -88,41 +80,49 @@ end1: return error; } +/* rfc6488#section-2.1.6.4.1 */ static int validate_content_type_attribute(CMSAttributeValue_t *value, EncapsulatedContentInfo_t *eci) { - struct oid_arcs attrValue_arcs; - struct oid_arcs EncapContentInfo_arcs; + OBJECT_IDENTIFIER_t *attrValues; + OBJECT_IDENTIFIER_t *eContentType; int error; - /* rfc6488#section-3.1.h */ - /* TODO (performance) Can't we just do a memcmp? */ - - error = any2arcs(value, &attrValue_arcs); + error = asn1_decode_any(value, &asn_DEF_OBJECT_IDENTIFIER, + (void **) &attrValues); if (error) return error; + eContentType = &eci->eContentType; - error = oid2arcs(&eci->eContentType, &EncapContentInfo_arcs); - if (error) { - free_arcs(&attrValue_arcs); - return error; - } - - if (!arcs_equal(&attrValue_arcs, &EncapContentInfo_arcs)) { - pr_err("The eContentType in the EncapsulatedContentInfo does not match the attrValues in the content-type attribute."); - error = -EINVAL; - } + if (!oid_equal(attrValues, eContentType)) + error = pr_err("The attrValues for the content-type attribute does not match the eContentType in the EncapsulatedContentInfo."); - free_arcs(&EncapContentInfo_arcs); - free_arcs(&attrValue_arcs); + ASN_STRUCT_FREE(asn_DEF_OBJECT_IDENTIFIER, attrValues); return error; } static int -validate_message_digest_attribute(CMSAttributeValue_t *value) +validate_message_digest_attribute(CMSAttributeValue_t *value, + EncapsulatedContentInfo_t *eci) { - return 0; /* TODO need the content being signed */ + MessageDigest_t *digest; + int error; + + if (eci->eContent == NULL) { + pr_err("There's no content being signed."); + return -EINVAL; + } + + error = asn1_decode_any(value, &asn_DEF_MessageDigest, + (void **) &digest); + if (error) + return error; + + error = hash_validate_octet_string(eci->eContent, digest); + + free(digest); + return error; } static int @@ -138,10 +138,8 @@ validate_signed_attrs(struct SignerInfo *sinfo, EncapsulatedContentInfo_t *eci) bool binary_signing_time_found = false; int error; - if (sinfo->signedAttrs == NULL) { - pr_err("The SignerInfo's signedAttrs field is NULL."); - return -EINVAL; - } + if (sinfo->signedAttrs == NULL) + return pr_err("The SignerInfo's signedAttrs field is NULL."); for (i = 0; i < sinfo->signedAttrs->list.count; i++) { attr = sinfo->signedAttrs->list.array[i]; @@ -152,15 +150,12 @@ validate_signed_attrs(struct SignerInfo *sinfo, EncapsulatedContentInfo_t *eci) attrs = &attr->attrValues; if (attrs->list.count != 1) { - pr_err(0, + return pr_err(0, "signedAttrs's attribute set size (%d) is different than 1.", attr->attrValues.list.count); - return -EINVAL; - } - if (attrs->list.array == NULL || attrs->list.array[0] == NULL) { - pr_err("Programming error: Array size is 1 but array itself is NULL."); - return -EINVAL; } + if (attrs->list.array == NULL || attrs->list.array[0] == NULL) + return pr_crit("Array size is 1 but array is NULL."); error = oid2arcs(&attr->attrType, &attrType); if (error) @@ -181,7 +176,7 @@ validate_signed_attrs(struct SignerInfo *sinfo, EncapsulatedContentInfo_t *eci) goto illegal_attrType; } error = validate_message_digest_attribute( - attr->attrValues.list.array[0]); + attr->attrValues.list.array[0], eci); message_digest_found = true; } else if (ARCS_EQUAL_OIDS(&attrType, oid_sta)) { @@ -213,14 +208,10 @@ validate_signed_attrs(struct SignerInfo *sinfo, EncapsulatedContentInfo_t *eci) } /* rfc6488#section-3.1.f */ - if (!content_type_found) { - pr_err("SignerInfo lacks a ContentType attribute."); - return -EINVAL; - } - if (!message_digest_found) { - pr_err("SignerInfo lacks a MessageDigest attribute."); - return -EINVAL; - } + if (!content_type_found) + return pr_err("SignerInfo lacks a ContentType attribute."); + if (!message_digest_found) + return pr_err("SignerInfo lacks a MessageDigest attribute."); return 0; @@ -234,30 +225,26 @@ validate(struct SignedData *sdata, STACK_OF(X509_CRL) *crls, struct resources *res) { struct SignerInfo *sinfo; - bool is_digest; int error; /* rfc6488#section-2.1 */ if (sdata->signerInfos.list.count != 1) { - pr_err("The SignedData's SignerInfo set is supposed to have only one element. (%d given.)", + return pr_err("The SignedData's SignerInfo set is supposed to have only one element. (%d given.)", sdata->signerInfos.list.count); - return -EINVAL; } /* rfc6488#section-2.1.1 */ /* rfc6488#section-3.1.b */ if (sdata->version != 3) { - pr_err("The SignedData version is only allowed to be 3. (Was %ld.)", + return pr_err("The SignedData version is only allowed to be 3. (Was %ld.)", sdata->version); - return -EINVAL; } /* rfc6488#section-2.1.2 */ /* rfc6488#section-3.1.j 1/2 */ if (sdata->digestAlgorithms.list.count != 1) { - pr_err("The SignedData's digestAlgorithms set is supposed to have only one element. (%d given.)", + return pr_err("The SignedData's digestAlgorithms set is supposed to have only one element. (%d given.)", sdata->digestAlgorithms.list.count); - return -EINVAL; } /* @@ -268,28 +255,21 @@ validate(struct SignedData *sdata, STACK_OF(X509_CRL) *crls, */ error = is_digest_algorithm( (AlgorithmIdentifier_t *) sdata->digestAlgorithms.list.array[0], - &is_digest); + "SignedData"); if (error) return error; - if (!is_digest) { - pr_err("The SignedData's digestAlgorithm OID is not listed in RFC 5754."); - return -EINVAL; - } /* section-2.1.3 */ /* Specific sub-validations will be performed later by calling code. */ /* rfc6488#section-2.1.4 */ /* rfc6488#section-3.1.c 1/2 */ - if (sdata->certificates == NULL) { - pr_err("The SignedData does not contain certificates."); - return -EINVAL; - } + if (sdata->certificates == NULL) + return pr_err("The SignedData does not contain certificates."); if (sdata->certificates->list.count != 1) { - pr_err("The SignedData contains %d certificates, one expected.", + return pr_err("The SignedData contains %d certificates, one expected.", sdata->certificates->list.count); - return -EINVAL; } error = handle_sdata_certificate(sdata->certificates->list.array[0], @@ -299,22 +279,17 @@ validate(struct SignedData *sdata, STACK_OF(X509_CRL) *crls, /* rfc6488#section-2.1.5 */ /* rfc6488#section-3.1.d */ - if (sdata->crls != NULL && sdata->crls->list.count > 0) { - pr_err("The SignedData contains at least one crls."); - return -EINVAL; - } + if (sdata->crls != NULL && sdata->crls->list.count > 0) + return pr_err("The SignedData contains at least one CRL."); /* rfc6488#section-2.1.6.1 */ /* rfc6488#section-3.1.e */ sinfo = sdata->signerInfos.list.array[0]; - if (sinfo == NULL) { - pr_err("The SignerInfo object is NULL."); - return -EINVAL; - } + if (sinfo == NULL) + return pr_err("The SignerInfo object is NULL."); if (sinfo->version != 3) { - pr_err("The SignerInfo version is only allowed to be 3. (Was %ld.)", + return pr_err("The SignerInfo version is only allowed to be 3. (Was %ld.)", sinfo->version); - return -EINVAL; } /* rfc6488#section-2.1.6.2 */ @@ -325,14 +300,9 @@ validate(struct SignedData *sdata, STACK_OF(X509_CRL) *crls, /* rfc6488#section-2.1.6.3 */ /* rfc6488#section-3.1.j 2/2 */ - error = is_digest_algorithm( - (AlgorithmIdentifier_t *) &sinfo->digestAlgorithm, &is_digest); + error = is_digest_algorithm(&sinfo->digestAlgorithm, "SignerInfo"); if (error) return error; - if (!is_digest) { - pr_err("The SignerInfo digestAlgorithm OID is not listed in RFC 5754."); - return -EINVAL; - } /* rfc6488#section-2.1.6.4 */ error = validate_signed_attrs(sinfo, &sdata->encapContentInfo); @@ -362,10 +332,8 @@ validate(struct SignedData *sdata, STACK_OF(X509_CRL) *crls, /* rfc6488#section-2.1.6.7 */ /* rfc6488#section-3.1.i */ - if (sinfo->unsignedAttrs != NULL && sinfo->unsignedAttrs->list.count > 0) { - pr_err("SignerInfo has at least one unsignedAttr."); - return -EINVAL; - } + if (sinfo->unsignedAttrs != NULL && sinfo->unsignedAttrs->list.count > 0) + return pr_err("SignerInfo has at least one unsignedAttr."); /* rfc6488#section-3.2 */ /* rfc6488#section-3.3 */ diff --git a/src/common.c b/src/common.c index 169a3555..f2ac0812 100644 --- a/src/common.c +++ b/src/common.c @@ -42,10 +42,8 @@ uri_g2l(char const *guri, size_t guri_len, char **result) prefix_len = strlen(PREFIX); - if (guri_len < prefix_len || strncmp(PREFIX, guri, prefix_len) != 0) { - pr_err("Global URI does not begin with '%s'.", PREFIX); - return -EINVAL; - } + if (guri_len < prefix_len || strncmp(PREFIX, guri, prefix_len) != 0) + return pr_err("Global URI does not begin with '%s'.", PREFIX); guri += prefix_len; guri_len -= prefix_len; diff --git a/src/base64.c b/src/crypto/base64.c similarity index 95% rename from src/base64.c rename to src/crypto/base64.c index 05eb7e45..efb5ecc6 100644 --- a/src/base64.c +++ b/src/crypto/base64.c @@ -62,9 +62,6 @@ base64_decode(BIO *in, unsigned char *out, size_t out_len, size_t *out_written) 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 diff --git a/src/base64.h b/src/crypto/base64.h similarity index 100% rename from src/base64.h rename to src/crypto/base64.h diff --git a/src/crypto/hash.c b/src/crypto/hash.c new file mode 100644 index 00000000..1d998842 --- /dev/null +++ b/src/crypto/hash.c @@ -0,0 +1,173 @@ +#include "hash.h" + +#include +#include +#include + +#include "file.h" +#include "log.h" +#include "asn1/oid.h" + +#define ALGORITHM "sha256" +static EVP_MD const *md; + +int +hash_init(void) +{ + md = EVP_get_digestbyname(ALGORITHM); + if (md == NULL) { + printf("Unknown message digest %s\n", ALGORITHM); + return -EINVAL; + } + + return 0; +} + +int +hash_is_valid_algorithm(OBJECT_IDENTIFIER_t *oid, bool *result) +{ + static const OID sha_oid = OID_SHA256; + struct oid_arcs arcs; + int error; + + error = oid2arcs(oid, &arcs); + if (error) + return error; + + *result = ARCS_EQUAL_OIDS(&arcs, sha_oid); + + free_arcs(&arcs); + return 0; +} + +static bool +hash_matches(unsigned char *expected, size_t expected_len, + unsigned char *actual, unsigned int actual_len) +{ + return (expected_len == actual_len) + && (memcmp(expected, actual, expected_len) == 0); +} + +static int +hash_file(char *filename, unsigned char *result, unsigned int *result_len) +{ + FILE *file; + struct stat stat; + unsigned char *buffer; + __blksize_t buffer_len; + size_t consumed; + EVP_MD_CTX *ctx; + int error; + + error = file_open(filename, &file, &stat); + if (error) + return error; + + buffer_len = stat.st_blksize; + buffer = malloc(buffer_len); + if (buffer == NULL) { + error = pr_enomem(); + goto end1; + } + + ctx = EVP_MD_CTX_new(); + if (ctx == NULL) { + error = pr_enomem(); + goto end2; + } + + if (!EVP_DigestInit_ex(ctx, md, NULL)) { + error = crypto_err("EVP_DigestInit_ex() failed"); + goto end3; + } + + do { + consumed = fread(buffer, 1, buffer_len, file); + error = ferror(file); + if (error) { + pr_errno(error, + "File reading error. Error message (apparently)"); + goto end3; + } + + if (!EVP_DigestUpdate(ctx, buffer, consumed)) { + error = crypto_err("EVP_DigestUpdate() failed"); + goto end3; + } + + } while (!feof(file)); + + if (!EVP_DigestFinal_ex(ctx, result, result_len)) + error = crypto_err("EVP_DigestFinal_ex() failed"); + +end3: + EVP_MD_CTX_free(ctx); +end2: + free(buffer); +end1: + file_close(file); + return error; +} + +/** + * Computes the hash of the file whose name is @filename, and compares it to + * @expected (The "expected" hash). Returns 0 if no errors happened and the + * hashes match. + */ +int +hash_validate_file(char *filename, BIT_STRING_t *expected) +{ + unsigned char actual[EVP_MAX_MD_SIZE]; + unsigned int actual_len; + int error; + + if (expected->bits_unused != 0) + return pr_err("Hash string has unused bits."); + + error = hash_file(filename, actual, &actual_len); + if (error) + return error; + + if (!hash_matches(expected->buf, expected->size, actual, actual_len)) + return pr_err("File does not match its hash."); + + return 0; +} + +static int +hash_buffer(unsigned char *content, size_t content_len, unsigned char *hash, + unsigned int *hash_len) +{ + EVP_MD_CTX *ctx; + int error = 0; + + ctx = EVP_MD_CTX_new(); + if (ctx == NULL) + return pr_enomem(); + + if (!EVP_DigestInit_ex(ctx, md, NULL) + || !EVP_DigestUpdate(ctx, content, content_len) + || !EVP_DigestFinal_ex(ctx, hash, hash_len)) { + error = crypto_err("Buffer hashing failed"); + } + + EVP_MD_CTX_free(ctx); + return error; +} + +int +hash_validate_octet_string(OCTET_STRING_t *content, OCTET_STRING_t *expected) +{ + unsigned char actual[EVP_MAX_MD_SIZE]; + unsigned int actual_len; + int error; + + error = hash_buffer(content->buf, content->size, actual, &actual_len); + if (error) + return error; + + if (!hash_matches(expected->buf, expected->size, actual, actual_len)) + return pr_err("File does not match its hash."); + + return 0; +} diff --git a/src/crypto/hash.h b/src/crypto/hash.h new file mode 100644 index 00000000..3e771c5f --- /dev/null +++ b/src/crypto/hash.h @@ -0,0 +1,13 @@ +#ifndef SRC_HASH_H_ +#define SRC_HASH_H_ + +#include +#include +#include + +int hash_init(void); +int hash_is_valid_algorithm(OBJECT_IDENTIFIER_t *, bool *); +int hash_validate_file(char *, BIT_STRING_t *); +int hash_validate_octet_string(OCTET_STRING_t *, OCTET_STRING_t *); + +#endif /* SRC_HASH_H_ */ diff --git a/src/file.c b/src/file.c index 064f98cb..afa899be 100644 --- a/src/file.c +++ b/src/file.c @@ -1,51 +1,60 @@ #include "file.h" #include -#include #include #include "log.h" -/* - * Will also rewind the file as a side effect. - * This is currently perfect for calling users. - */ -static int -get_file_size(FILE *file, long int *size) +int +file_open(const char *filename, FILE **result, struct stat *stat) { - if (fseek(file, 0L, SEEK_END) == -1) - return errno ? errno : -EINVAL; - *size = ftell(file); - rewind(file); + FILE *file; + int error; + + file = fopen(filename, "rb"); + if (file == NULL) + return pr_errno(errno, "Could not open file '%s'", filename); + + if (fstat(fileno(file), stat) == -1) { + error = pr_errno(errno, "fstat(%s) failed", filename); + goto fail; + } + if (!S_ISREG(stat->st_mode)) { + error = pr_err("%s does not seem to be a file", filename); + goto fail; + } + + *result = file; return 0; + +fail: + file_close(file); + return error; +} + +void +file_close(FILE *file) +{ + if (fclose(file) == -1) + pr_errno(errno, "fclose() failed"); } int -file_load(const char *file_name, struct file_contents *fc) +file_load(const char *filename, struct file_contents *fc) { FILE *file; - long int file_size; + struct stat stat; size_t fread_result; int error; - file = fopen(file_name, "rb"); - if (file == NULL) - return pr_errno(errno, "Could not open file '%s'", file_name); - - /* TODO if @file is a directory, this returns a very large integer. */ - error = get_file_size(file, &file_size); - if (error) { - pr_errno(error, "Could not compute the file size of %s", - file_name); - fclose(file); + error = file_open(filename, &file, &stat); + if (error) return error; - } - fc->buffer_size = file_size; + fc->buffer_size = stat.st_size; fc->buffer = malloc(fc->buffer_size); if (fc->buffer == NULL) { - pr_err("Out of memory."); - fclose(file); - return -ENOMEM; + error = pr_enomem(); + goto end; } fread_result = fread(fc->buffer, 1, fc->buffer_size, file); @@ -54,14 +63,13 @@ file_load(const char *file_name, struct file_contents *fc) if (error) { /* * The manpage doesn't say that the result is an error - * code. It literally doesn't say how to obtain the - * error code. + * code. It literally doesn't say how to get an error + * code. */ pr_errno(error, "File reading error. Error message (apparently)"); free(fc->buffer); - fclose(file); - return error; + goto end; } /* @@ -73,12 +81,15 @@ file_load(const char *file_name, struct file_contents *fc) pr_err("fr:%zu bs:%zu EOF:%d", fread_result, fc->buffer_size, feof(file)); free(fc->buffer); - fclose(file); - return -EINVAL; + error = -EINVAL; + goto end; } - fclose(file); - return 0; + error = 0; + +end: + file_close(file); + return error; } void diff --git a/src/file.h b/src/file.h index 063c85db..4264d956 100644 --- a/src/file.h +++ b/src/file.h @@ -2,6 +2,8 @@ #define SRC_FILE_H_ #include +#include +#include /* * The entire contents of the file, loaded into a buffer. @@ -13,6 +15,9 @@ struct file_contents { size_t buffer_size; }; +int file_open(const char *, FILE **, struct stat *); +void file_close(FILE *file); + int file_load(const char *, struct file_contents *); void file_free(struct file_contents *); diff --git a/src/hash.c b/src/hash.c deleted file mode 100644 index 51b152b3..00000000 --- a/src/hash.c +++ /dev/null @@ -1,119 +0,0 @@ -#include "hash.h" - -#include -#include -#include - -#include "log.h" - -#define ALGORITHM "sha256" -static EVP_MD const *md; - -int -hash_init(void) -{ - md = EVP_get_digestbyname(ALGORITHM); - if (md == NULL) { - printf("Unknown message digest %s\n", ALGORITHM); - return -EINVAL; - } - - return 0; -} - -static int -hash_file(char *filename, unsigned char *result, unsigned int *result_len) -{ - FILE *file; - struct stat stat; - unsigned char *buffer; - __blksize_t buffer_len; - size_t consumed; - EVP_MD_CTX *ctx; - int error; - - file = fopen(filename, "rb"); - if (file == NULL) - return pr_errno(errno, "Could not open file '%s'", filename); - - buffer_len = (fstat(fileno(file), &stat) == 0) ? stat.st_blksize : 1024; - buffer = malloc(buffer_len); - if (buffer == NULL) { - pr_err("Out of memory."); - error = -ENOMEM; - goto end1; - } - - ctx = EVP_MD_CTX_new(); - if (ctx == NULL) { - pr_err("Out of memory."); - error = -ENOMEM; - goto end2; - } - - if (!EVP_DigestInit_ex(ctx, md, NULL)) { - error = crypto_err("EVP_DigestInit_ex() failed"); - goto end3; - } - - do { - consumed = fread(buffer, 1, buffer_len, file); - error = ferror(file); - if (error) { - pr_errno(error, - "File reading error. Error message (apparently)"); - goto end3; - } - - if (!EVP_DigestUpdate(ctx, buffer, consumed)) { - error = crypto_err("EVP_DigestUpdate() failed"); - goto end3; - } - - } while (!feof(file)); - - if (!EVP_DigestFinal_ex(ctx, result, result_len)) - error = crypto_err("EVP_DigestFinal_ex() failed"); - -end3: - EVP_MD_CTX_free(ctx); -end2: - free(buffer); -end1: - if (fclose(file) == -1) - pr_errno(errno, "fclose() failed"); - return error; -} - -/** - * Computes the hash of the file whose name is @filename, and compares it to - * @expected (The "expected" hash). Returns 0 if no errors happened and the - * hashes match. - */ -int -hash_validate(char *filename, BIT_STRING_t *expected) -{ - unsigned char actual[EVP_MAX_MD_SIZE]; - unsigned int actual_len; - int error; - - if (expected->bits_unused != 0) { - pr_err("Hash string has unused bits."); - return -EINVAL; - } - - error = hash_file(filename, actual, &actual_len); - if (error) - return error; - - if (expected->size != actual_len) - goto mismatch; - if (memcmp(expected->buf, actual, actual_len) != 0) - goto mismatch; - - return 0; - -mismatch: - pr_err("File does not match its hash."); - return -EINVAL; -} diff --git a/src/hash.h b/src/hash.h deleted file mode 100644 index b259de31..00000000 --- a/src/hash.h +++ /dev/null @@ -1,9 +0,0 @@ -#ifndef SRC_HASH_H_ -#define SRC_HASH_H_ - -#include - -int hash_init(void); -int hash_validate(char *file, BIT_STRING_t *hash); - -#endif /* SRC_HASH_H_ */ diff --git a/src/line_file.c b/src/line_file.c index f98cb88a..ec3d154b 100644 --- a/src/line_file.c +++ b/src/line_file.c @@ -2,8 +2,9 @@ #include #include -#include #include + +#include "file.h" #include "log.h" struct line_file { @@ -19,7 +20,7 @@ int lfile_open(const char *file_name, struct line_file **result) { struct line_file *lfile; - int err; + int error; lfile = malloc(sizeof(struct line_file)); if (lfile == NULL) @@ -27,9 +28,9 @@ lfile_open(const char *file_name, struct line_file **result) lfile->file = fopen(file_name, "r"); if (lfile->file == NULL) { - err = errno; + error = errno; free(lfile); - return err; + return error; } lfile->file_name = file_name; lfile->offset = 0; @@ -41,7 +42,7 @@ lfile_open(const char *file_name, struct line_file **result) void lfile_close(struct line_file *lf) { - fclose(lf->file); + file_close(lf->file); free(lf); } @@ -59,7 +60,7 @@ lfile_read(struct line_file *lfile, char **result) size_t alloc_len; ssize_t len; ssize_t i; - int err; + int error; /* * Note to myself: @@ -99,16 +100,15 @@ lfile_read(struct line_file *lfile, char **result) len = getline(&string, &alloc_len, lfile->file); if (len == -1) { - err = errno; + error = errno; free(string); *result = NULL; if (ferror(lfile->file)) - return err; + return pr_errno(error, "Error while reading file"); if (feof(lfile->file)) return 0; - pr_err("Supposedly unreachable code reached. ferror:%d feof:%d", + return pr_crit("Supposedly unreachable code reached. ferror:%d feof:%d", ferror(lfile->file), feof(lfile->file)); - return -EINVAL; } lfile->offset += len; @@ -140,6 +140,12 @@ lfile_read(struct line_file *lfile, char **result) return 0; } +FILE * +lfile_fd(struct line_file *lfile) +{ + return lfile->file; +} + const char * lfile_name(struct line_file *lfile) { diff --git a/src/line_file.h b/src/line_file.h index ef882ac6..52c66aff 100644 --- a/src/line_file.h +++ b/src/line_file.h @@ -10,6 +10,7 @@ */ #include +#include struct line_file; @@ -18,6 +19,7 @@ void lfile_close(); int lfile_read(struct line_file *, char **); +FILE *lfile_fd(struct line_file *); const char *lfile_name(struct line_file *); size_t lfile_offset(struct line_file *); diff --git a/src/log.c b/src/log.c index 250bc50b..165d13cd 100644 --- a/src/log.c +++ b/src/log.c @@ -126,14 +126,15 @@ pr_err_prefix(void) } while (0) /** - * Always appends a newline at the end. + * Always appends a newline at the end. Always returs -EINVAL. */ -void +int pr_err(const char *format, ...) { va_list args; PR_ERR(args); fprintf(STDERR, "\n"); + return -EINVAL; } /** @@ -164,6 +165,7 @@ pr_errno(int error, const char *format, ...) * If this function was called, then we need to assume that * there WAS an error; go generic. */ + fprintf(STDERR, ": (Unknown)"); error = -EINVAL; } @@ -212,3 +214,27 @@ crypto_err(const char *format, ...) fprintf(STDERR, "\n"); return error; } + +int +pr_enomem(void) +{ + pr_err("Out of memory."); + return -ENOMEM; +} + +int +pr_crit(const char *format, ...) +{ + va_list args; + + pr_err_prefix(); + pr_file_name(STDERR); + + fprintf(STDERR, "Programming error: "); + va_start(args, format); + vfprintf(STDERR, format, args); + va_end(args); + fprintf(STDERR, "\n"); + + return -EINVAL; +} diff --git a/src/log.h b/src/log.h index 840146a0..ac4dd963 100644 --- a/src/log.h +++ b/src/log.h @@ -6,9 +6,12 @@ void pr_debug_add(const char *, ...); void pr_debug_rm(const char *, ...); void pr_debug_prefix(void); -void pr_err(const char *, ...); +int pr_err(const char *, ...); int pr_errno(int, const char *, ...); int crypto_err(const char *, ...); +int pr_enomem(void); + +int pr_crit(const char *, ...); #define PR_DEBUG printf("%s:%d (%s())\n", __FILE__, __LINE__, __func__) #define PR_DEBUG_MSG(msg, ...) printf("%s:%d (%s()): " msg "\n", \ diff --git a/src/main.c b/src/main.c index 3e9b7dfa..b91aa4c4 100644 --- a/src/main.c +++ b/src/main.c @@ -4,9 +4,9 @@ #include "common.h" #include "debug.h" -#include "hash.h" #include "log.h" #include "thread_var.h" +#include "crypto/hash.h" #include "object/certificate.h" #include "object/tal.h" @@ -96,10 +96,8 @@ main(int argc, char **argv) print_stack_trace_on_segfault(); - if (argc < 3) { - pr_err("Repository path as first argument and TAL file as second argument, please."); - return -EINVAL; - } + if (argc < 3) + return pr_err("Repository path as first argument and TAL file as second argument, please."); error = hash_init(); if (error) diff --git a/src/object/certificate.c b/src/object/certificate.c index ee0046cd..c9dfe3ee 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -27,7 +27,7 @@ bool is_certificate(char const *file_name) static int validate_serial_number(X509 *cert) { - /* TODO implement this properly. */ + /* TODO (field) implement this properly. */ BIGNUM *number; @@ -52,10 +52,8 @@ validate_signature_algorithm(X509 *cert) int nid; nid = OBJ_obj2nid(X509_get0_tbs_sigalg(cert)->algorithm); - if (nid != NID_sha256WithRSAEncryption) { - pr_err("Certificate's Signature Algorithm is not RSASSA-PKCS1-v1_5."); - return -EINVAL; - } + if (nid != NID_sha256WithRSAEncryption) + return pr_err("Certificate's Signature Algorithm is not RSASSA-PKCS1-v1_5."); return 0; } @@ -110,10 +108,8 @@ validate_spki(const unsigned char *cert_spk, int cert_spk_len) return -EINVAL; tal = validation_tal(state); - if (tal == NULL) { - pr_err("Programming error: Validation state has no TAL."); - return -EINVAL; - } + if (tal == NULL) + return pr_crit("Validation state has no TAL."); /* * We have a problem at this point: @@ -143,8 +139,7 @@ validate_spki(const unsigned char *cert_spk, int cert_spk_len) 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; + error = pr_err("TAL's public key format is not RSA PKCS#1 v1.5 with SHA-256."); goto fail; } @@ -158,8 +153,7 @@ validate_spki(const unsigned char *cert_spk, int cert_spk_len) return 0; not_equal: - pr_err("TAL's public key is different than the root certificate's public key."); - error = -EINVAL; + error = pr_err("TAL's public key is different than the root certificate's public key."); fail: ASN_STRUCT_FREE(asn_DEF_SubjectPublicKeyInfo, tal_spki); return error; @@ -178,13 +172,13 @@ validate_public_key(X509 *cert, bool is_root) pubkey = X509_get_X509_PUBKEY(cert); if (pubkey == NULL) { - crypto_err("X509_get_X509_PUBKEY() returned NULL."); + crypto_err("X509_get_X509_PUBKEY() returned NULL"); return -EINVAL; } ok = X509_PUBKEY_get0_param(&alg, &bytes, &bytes_len, NULL, pubkey); if (!ok) { - crypto_err("X509_PUBKEY_get0_param() returned %d.", ok); + crypto_err("X509_PUBKEY_get0_param() returned %d", ok); return -EINVAL; } @@ -194,9 +188,8 @@ validate_public_key(X509 *cert, bool is_root) * seem to match RFC 7935's public key algorithm. Wtf? */ if (alg_nid != NID_rsaEncryption) { - pr_err("Certificate's public key format is %d, not RSA PKCS#1 v1.5 with SHA-256.", + return pr_err("Certificate's public key format is %d, not RSA PKCS#1 v1.5 with SHA-256.", alg_nid); - return -EINVAL; } /* @@ -223,7 +216,7 @@ validate_public_key(X509 *cert, bool is_root) static int validate_extensions(X509 *cert) { - /* TODO */ + /* TODO (field) */ return 0; } @@ -246,10 +239,8 @@ certificate_validate_rfc6487(X509 *cert, bool is_root) */ /* rfc6487#section-4.1 */ - if (X509_get_version(cert) != 2) { - pr_err("Certificate version is not v3."); - return -EINVAL; - } + if (X509_get_version(cert) != 2) + return pr_err("Certificate version is not v3."); /* rfc6487#section-4.2 */ error = validate_serial_number(cert); @@ -269,7 +260,7 @@ certificate_validate_rfc6487(X509 *cert, bool is_root) /* * rfc6487#section-4.5 * - * TODO "Each distinct subordinate CA and + * TODO (field) "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." @@ -321,15 +312,7 @@ end: int 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 - * that -CAfile consults the default file location, while -trusted does - * not. As far as I can tell, this means that we absolutely need to use - * -trusted. - * So, just in case, enable -no-CAfile and -no-CApath. - */ + /* Reference: openbsd/src/usr.bin/openssl/verify.c */ struct validation *state; X509_STORE_CTX *ctx; @@ -477,30 +460,27 @@ handle_ip_extension(X509_EXTENSION *ext, struct resources *resources) 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; + error = pr_err("First IP address block listed is not v4."); + goto end; } 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; + error = pr_err("Second IP address block listed is not v6."); + goto end; } break; default: - pr_err("Got %d IP address blocks Expected; 1 or 2 expected.", + error = pr_err("Got %d IP address blocks Expected; 1 or 2 expected.", blocks->list.count); - goto einval; + goto end; } for (i = 0; i < blocks->list.count && !error; i++) error = resources_add_ip(resources, blocks->list.array[i]); +end: ASN_STRUCT_FREE(asn_DEF_IPAddrBlocks, blocks); return error; - -einval: - ASN_STRUCT_FREE(asn_DEF_IPAddrBlocks, blocks); - return -EINVAL; } static int @@ -539,14 +519,10 @@ certificate_get_resources(X509 *cert, struct resources *resources) 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; - } + if (ip_ext_found) + return pr_err("Multiple IP extensions found."); + if (!X509_EXTENSION_get_critical(ext)) + return pr_err("The IP extension is not marked as critical."); pr_debug_add("IP {"); error = handle_ip_extension(ext, resources); @@ -558,14 +534,10 @@ certificate_get_resources(X509 *cert, struct resources *resources) 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; - } + if (asn_ext_found) + return pr_err("Multiple AS extensions found."); + if (!X509_EXTENSION_get_critical(ext)) + return pr_err("The AS extension is not marked as critical."); pr_debug_add("ASN {"); error = handle_asn_extension(ext, resources); @@ -578,10 +550,8 @@ certificate_get_resources(X509 *cert, struct resources *resources) } } - if (!ip_ext_found && !asn_ext_found) { - pr_err("Certificate lacks both IP and AS extension."); - return -EINVAL; - } + if (!ip_ext_found && !asn_ext_found) + return pr_err("Certificate lacks both IP and AS extension."); return 0; } @@ -713,8 +683,7 @@ certificate_traverse_ee(X509 *cert) } else { /* rfc6487#section-4.8.8.2 */ - pr_err("EE Certificate has an non-signedObject access description."); - error = -EINVAL; + error = pr_err("EE Certificate has an non-signedObject access description."); goto end; } } diff --git a/src/object/manifest.c b/src/object/manifest.c index 464d467f..4a31e5d8 100644 --- a/src/object/manifest.c +++ b/src/object/manifest.c @@ -4,10 +4,10 @@ #include #include -#include "hash.h" #include "log.h" #include "thread_var.h" #include "asn1/oid.h" +#include "crypto/hash.h" #include "object/certificate.h" #include "object/crl.h" #include "object/roa.h" @@ -25,23 +25,6 @@ validate_dates(GeneralizedTime_t *this, GeneralizedTime_t *next) return (GeneralizedTime_compare(def, this, next) < 0) ? 0 : -EINVAL; } -static int -is_hash_algorithm(OBJECT_IDENTIFIER_t *aid, bool *result) -{ - static const OID sha_oid = OID_SHA256; - struct oid_arcs arcs; - int error; - - error = oid2arcs(aid, &arcs); - if (error) - return error; - - *result = ARCS_EQUAL_OIDS(&arcs, sha_oid); - - free_arcs(&arcs); - return 0; -} - static int validate_manifest(struct Manifest *manifest) { @@ -51,7 +34,7 @@ validate_manifest(struct Manifest *manifest) /* rfc6486#section-4.2.1 */ /* - * TODO + * TODO (field) * * If a "one-time-use" EE certificate is employed to verify a manifest, * the EE certificate MUST have a validity period that coincides with @@ -76,7 +59,7 @@ validate_manifest(struct Manifest *manifest) /* manifest->manifestNumber; */ /* - * TODO + * TODO (field) * * "CRL issuers conforming to this profile MUST encode thisUpdate as * UTCTime for dates through the year 2049. CRL issuers conforming to @@ -92,7 +75,7 @@ validate_manifest(struct Manifest *manifest) /* manifest->thisUpdate */ /* - * TODO again, same bullshit: + * TODO (field) again, same bullshit: * * "CRL issuers conforming to this profile MUST encode nextUpdate as * UTCTime for dates through the year 2049. CRL issuers conforming to @@ -108,13 +91,11 @@ validate_manifest(struct Manifest *manifest) return error; /* rfc6486#section-6.6 (I guess) */ - error = is_hash_algorithm(&manifest->fileHashAlg, &is_hash); + error = hash_is_valid_algorithm(&manifest->fileHashAlg, &is_hash); if (error) return error; - if (!is_hash) { - pr_err("The hash algorithm is not SHA256."); - return -EINVAL; - } + if (!is_hash) + return pr_err("The hash algorithm is not SHA256."); /* The file hashes will be validated during the traversal. */ @@ -190,7 +171,7 @@ foreach_file(struct manifest *mft, char *extension, foreach_cb cb, void *arg) if (error) return error; - error = hash_validate(luri, &fah->hash); + error = hash_validate_file(luri, &fah->hash); if (error) { free(luri); continue; @@ -291,10 +272,8 @@ __handle_manifest(struct manifest *mft) /* Init */ crls = sk_X509_CRL_new_null(); - if (crls == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (crls == NULL) + return pr_enomem(); /* Get CRLs as a stack. There will usually only be one. */ error = foreach_file(mft, ".crl", pile_crls, crls); @@ -327,10 +306,6 @@ handle_manifest(char const *file_path, STACK_OF(X509_CRL) *crls) mft.file_path = file_path; - /* - * TODO about those NULL resources: Maybe print a warning if the - * certificate contains some. - */ error = signed_object_decode(file_path, &asn_DEF_Manifest, &arcs, (void **) &mft.obj, crls, NULL); if (error) diff --git a/src/object/roa.c b/src/object/roa.c index 31e3aea1..4cd1d923 100644 --- a/src/object/roa.c +++ b/src/object/roa.c @@ -22,10 +22,8 @@ print_addr4(struct resources *parent, long asn, struct ROAIPAddress *roa_addr) return error; str2 = inet_ntop(AF_INET, &prefix.addr, str, sizeof(str)); - if (str2 == NULL) { - pr_err("inet_ntop() returned NULL."); - return -EINVAL; - } + if (str2 == NULL) + return pr_err("inet_ntop() returned NULL."); if (roa_addr->maxLength != NULL) if (prefix.len > *roa_addr->maxLength) @@ -37,8 +35,8 @@ print_addr4(struct resources *parent, long asn, struct ROAIPAddress *roa_addr) return 0; unallowed: - pr_err("ROA is not allowed to advertise %s/%u.", str2, prefix.len); - return -EINVAL; + return pr_err("ROA is not allowed to advertise %s/%u.", str2, + prefix.len); } static int @@ -54,10 +52,8 @@ print_addr6(struct resources *parent, long asn, struct ROAIPAddress *roa_addr) return error; str2 = inet_ntop(AF_INET6, &prefix.addr, str, sizeof(str)); - if (str2 == NULL) { - pr_err("inet_ntop() returned NULL."); - return -EINVAL; - } + if (str2 == NULL) + return pr_err("inet_ntop() returned NULL."); if (roa_addr->maxLength != NULL) if (prefix.len > *roa_addr->maxLength) @@ -69,8 +65,8 @@ print_addr6(struct resources *parent, long asn, struct ROAIPAddress *roa_addr) return 0; unallowed: - pr_err("ROA is not allowed to advertise %s/%u.", str2, prefix.len); - return -EINVAL; + return pr_err("ROA is not allowed to advertise %s/%u.", str2, + prefix.len); } static int @@ -84,8 +80,7 @@ print_addr(struct resources *parent, long asn, uint8_t family, return print_addr6(parent, asn, roa_addr); } - pr_err("Unknown family value: %u", family); - return -EINVAL; + return pr_err("Unknown family value: %u", family); } static int @@ -97,31 +92,24 @@ __handle_roa(struct RouteOriginAttestation *roa, struct resources *parent) int error; /* rfc6482#section-3.1 */ - if (roa->version != 0) { - pr_err("ROA's version (%ld) is nonzero.", roa->version); - return -EINVAL; - } + if (roa->version != 0) + return pr_err("ROA's version (%ld) is nonzero.", roa->version); /* rfc6482#section-3.2 (more or less.) */ if (!resources_contains_asn(parent, roa->asID)) { - pr_err("ROA is not allowed to attest for AS %d", + return pr_err("ROA is not allowed to attest for AS %d", roa->asID); - return -EINVAL; } /* rfc6482#section-3.3 */ - if (roa->ipAddrBlocks.list.array == NULL) { - pr_err("Programming error: ipAddrBlocks array is NULL."); - return -EINVAL; - } + if (roa->ipAddrBlocks.list.array == NULL) + return pr_crit("ipAddrBlocks array is NULL."); for (b = 0; b < roa->ipAddrBlocks.list.count; b++) { block = roa->ipAddrBlocks.list.array[0]; - if (block == NULL) { - pr_err("Address block array element is NULL."); - return -EINVAL; - } + if (block == NULL) + return pr_err("Address block array element is NULL."); if (block->addressFamily.size != 2) goto family_error; @@ -131,10 +119,8 @@ __handle_roa(struct RouteOriginAttestation *roa, struct resources *parent) && block->addressFamily.buf[1] != 2) goto family_error; - if (block->addresses.list.array == NULL) { - pr_err("ROA's address list array is NULL."); - return -EINVAL; - } + if (block->addresses.list.array == NULL) + return pr_err("ROA's address list array is NULL."); for (a = 0; a < block->addresses.list.count; a++) { error = print_addr(parent, roa->asID, block->addressFamily.buf[1], @@ -147,8 +133,7 @@ __handle_roa(struct RouteOriginAttestation *roa, struct resources *parent) return 0; family_error: - pr_err("ROA's IP family is not v4 or v6."); - return -EINVAL; + return pr_err("ROA's IP family is not v4 or v6."); } int handle_roa(char const *file, STACK_OF(X509_CRL) *crls) @@ -165,8 +150,7 @@ int handle_roa(char const *file, STACK_OF(X509_CRL) *crls) cert_resources = resources_create(); if (cert_resources == NULL) { - pr_err("Out of memory"); - error = -ENOMEM; + error = pr_enomem(); goto end1; } diff --git a/src/object/signed_object.c b/src/object/signed_object.c index b3fbbf47..2a52fe04 100644 --- a/src/object/signed_object.c +++ b/src/object/signed_object.c @@ -21,9 +21,8 @@ validate_eContentType(struct SignedData *sdata, equals = arcs_equal(&arcs, oid); free_arcs(&arcs); if (!equals) { - pr_err("SignedObject's encapContentInfo lacks the OID of a %s.", + return pr_err("SignedObject's encapContentInfo lacks the OID of a %s.", descriptor->name); - return -EINVAL; } return 0; @@ -49,9 +48,8 @@ validate_content_type(struct SignedData *sdata, equals = arcs_equal(&arcs, oid); free_arcs(&arcs); if (!equals) { - pr_err("SignedObject's content type attribute lacks the OID of a %s.", + return pr_err("SignedObject's content type attribute lacks the OID of a %s.", descriptor->name); - return -EINVAL; } return 0; diff --git a/src/object/tal.c b/src/object/tal.c index 5e0e0d42..ffc49631 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -8,9 +8,9 @@ #include #include -#include "base64.h" #include "line_file.h" #include "log.h" +#include "crypto/base64.h" struct uri { char *string; @@ -51,14 +51,11 @@ read_uri(struct line_file *lfile, struct uri **result) int err; uri = malloc(sizeof(struct uri)); - if (uri == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (uri == NULL) + return pr_enomem(); err = lfile_read(lfile, &uri->string); if (err) { - /* TODO have lfile_read print error msg */ free(uri); return err; } @@ -79,8 +76,8 @@ read_uris(struct line_file *lfile, struct uri_list *uris) if (strcmp(uri->string, "") == 0) { uri_destroy(uri); - pr_err("TAL file %s contains no URIs", lfile_name(lfile)); - return -EINVAL; + return pr_err("TAL file %s contains no URIs", + lfile_name(lfile)); } SLIST_INIT(uris); @@ -118,53 +115,6 @@ get_spki_alloc_size(struct line_file *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) { @@ -177,10 +127,10 @@ read_spki(struct line_file *lfile, struct tal *tal) if (tal->spki == NULL) return -ENOMEM; - error = lf2bio(lfile, &encoded); - if (error) { + encoded = BIO_new_fp(lfile_fd(lfile), BIO_NOCLOSE); + if (encoded == NULL) { free(tal->spki); - return error; + return crypto_err("BIO_new_fp() returned NULL"); } error = base64_decode(encoded, tal->spki, alloc_size, &tal->spki_len); diff --git a/src/resource.c b/src/resource.c index 0cb2a040..6b84a5dd 100644 --- a/src/resource.c +++ b/src/resource.c @@ -37,7 +37,7 @@ resources_create(void) result = malloc(sizeof(struct resources)); if (result == NULL) { - pr_err("Out of memory."); + pr_enomem(); return NULL; } @@ -139,43 +139,32 @@ inherit_aors(struct resources *resources, int family) struct resources *parent; parent = get_parent_resources(); - if (!parent) { - pr_err("Certificate inherits IP resources, but parent does not define any resources."); - return -EINVAL; - } + if (!parent) + return pr_err("Certificate inherits IP resources, but parent does not define any resources."); switch (family) { case AF_INET: - if (resources->ip4s != NULL) { - pr_err("Certificate inherits IPv4 resources while also defining others of its own."); - return -EINVAL; - } - if (parent->ip4s == NULL) { - pr_err("Certificate inherits IPv4 resources from parent, but parent lacks IPv4 resources."); - return -EINVAL; - } + if (resources->ip4s != NULL) + return pr_err("Certificate inherits IPv4 resources while also defining others of its own."); + if (parent->ip4s == NULL) + return pr_err("Certificate inherits IPv4 resources from parent, but parent lacks IPv4 resources."); resources->ip4s = parent->ip4s; res4_get(resources->ip4s); pr_debug(""); return 0; case AF_INET6: - if (resources->ip6s != NULL) { - pr_err("Certificate inherits IPv6 resources while also defining others of its own."); - return -EINVAL; - } - if (parent->ip6s == NULL) { - pr_err("Certificate inherits IPv6 resources from parent, but parent lacks IPv6 resources."); - return -EINVAL; - } + if (resources->ip6s != NULL) + return pr_err("Certificate inherits IPv6 resources while also defining others of its own."); + if (parent->ip6s == NULL) + return pr_err("Certificate inherits IPv6 resources from parent, but parent lacks IPv6 resources."); resources->ip6s = parent->ip6s; res6_get(resources->ip6s); pr_debug(""); return 0; } - pr_err("Programming error: Unknown address family '%d'", family); - return -EINVAL; + return pr_crit("Unknown address family '%d'", family); } static int @@ -187,26 +176,20 @@ add_prefix4(struct resources *resources, IPAddress2_t *addr) parent = get_parent_resources(); - if ((parent != NULL) && (resources->ip4s == parent->ip4s)) { - pr_err("Certificate defines IPv4 prefixes while also inheriting his parent's."); - return -EINVAL; - } + if ((parent != NULL) && (resources->ip4s == parent->ip4s)) + return pr_err("Certificate defines IPv4 prefixes while also inheriting his parent's."); error = prefix4_decode(addr, &prefix); if (error) return error; - if (parent && !res4_contains_prefix(parent->ip4s, &prefix)) { - pr_err("Parent certificate doesn't own child's IPv4 resource."); - return -EINVAL; - } + if (parent && !res4_contains_prefix(parent->ip4s, &prefix)) + return pr_err("Parent certificate doesn't own child's IPv4 resource."); if (resources->ip4s == NULL) { resources->ip4s = res4_create(); - if (resources->ip4s == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (resources->ip4s == NULL) + return pr_enomem(); } error = res4_add_prefix(resources->ip4s, &prefix); @@ -229,26 +212,20 @@ add_prefix6(struct resources *resources, IPAddress2_t *addr) parent = get_parent_resources(); - if ((parent != NULL) && (resources->ip6s == parent->ip6s)) { - pr_err("Certificate defines IPv6 prefixes while also inheriting his parent's."); - return -EINVAL; - } + if ((parent != NULL) && (resources->ip6s == parent->ip6s)) + return pr_err("Certificate defines IPv6 prefixes while also inheriting his parent's."); error = prefix6_decode(addr, &prefix); if (error) return error; - if (parent && !res6_contains_prefix(parent->ip6s, &prefix)) { - pr_err("Parent certificate doesn't own child's IPv6 resource."); - return -EINVAL; - } + if (parent && !res6_contains_prefix(parent->ip6s, &prefix)) + return pr_err("Parent certificate doesn't own child's IPv6 resource."); if (resources->ip6s == NULL) { resources->ip6s = res6_create(); - if (resources->ip6s == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (resources->ip6s == NULL) + return pr_enomem(); } error = res6_add_prefix(resources->ip6s, &prefix); @@ -272,8 +249,7 @@ add_prefix(struct resources *resources, int family, IPAddress2_t *addr) return add_prefix6(resources, addr); } - pr_err("Programming error: Unknown address family '%d'", family); - return -EINVAL; + return pr_crit("Unknown address family '%d'", family); } static int @@ -285,26 +261,20 @@ add_range4(struct resources *resources, IPAddressRange_t *input) parent = get_parent_resources(); - if ((parent != NULL) && (resources->ip4s == parent->ip4s)) { - pr_err("Certificate defines IPv4 ranges while also inheriting his parent's."); - return -EINVAL; - } + if ((parent != NULL) && (resources->ip4s == parent->ip4s)) + return pr_err("Certificate defines IPv4 ranges while also inheriting his parent's."); error = range4_decode(input, &range); if (error) return error; - if (parent && !res4_contains_range(parent->ip4s, &range)) { - pr_err("Parent certificate doesn't own child's IPv4 resource."); - return -EINVAL; - } + if (parent && !res4_contains_range(parent->ip4s, &range)) + return pr_err("Parent certificate doesn't own child's IPv4 resource."); if (resources->ip4s == NULL) { resources->ip4s = res4_create(); - if (resources->ip4s == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (resources->ip4s == NULL) + return pr_enomem(); } error = res4_add_range(resources->ip4s, &range); @@ -327,26 +297,20 @@ add_range6(struct resources *resources, IPAddressRange_t *input) parent = get_parent_resources(); - if ((parent != NULL) && (resources->ip6s == parent->ip6s)) { - pr_err("Certificate defines IPv6 ranges while also inheriting his parent's."); - return -EINVAL; - } + if ((parent != NULL) && (resources->ip6s == parent->ip6s)) + return pr_err("Certificate defines IPv6 ranges while also inheriting his parent's."); error = range6_decode(input, &range); if (error) return error; - if (parent && !res6_contains_range(parent->ip6s, &range)) { - pr_err("Parent certificate doesn't own child's IPv6 resource."); - return -EINVAL; - } + if (parent && !res6_contains_range(parent->ip6s, &range)) + return pr_err("Parent certificate doesn't own child's IPv6 resource."); if (resources->ip6s == NULL) { resources->ip6s = res6_create(); - if (resources->ip6s == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (resources->ip6s == NULL) + return pr_enomem(); } error = res6_add_range(resources->ip6s, &range); @@ -370,8 +334,7 @@ add_range(struct resources *resources, int family, IPAddressRange_t *range) return add_range6(resources, range); } - pr_err("Programming error: Unknown address family '%d'", family); - return -EINVAL; + return pr_crit("Unknown address family '%d'", family); } static int @@ -399,9 +362,8 @@ add_aors(struct resources *resources, int family, break; case IPAddressOrRange_PR_NOTHING: /* rfc3779#section-2.2.3.7 */ - pr_err("Unknown IPAddressOrRange type: %d", + return pr_err("Unknown IPAddressOrRange type: %d", aor->present); - break; } } @@ -428,9 +390,8 @@ resources_add_ip(struct resources *resources, struct IPAddressFamily *obj) } /* rfc3779#section-2.2.3.4 */ - pr_err("Unknown ipAddressChoice type: %d", + return pr_err("Unknown ipAddressChoice type: %d", obj->ipAddressChoice.present); - return -EINVAL; } static int @@ -439,19 +400,14 @@ inherit_asiors(struct resources *resources) struct resources *parent; parent = get_parent_resources(); - if (!parent) { - pr_err("Certificate inherits ASN resources, but parent does not define any resources."); - return -EINVAL; - } + if (!parent) + return pr_err("Certificate inherits ASN resources, but parent does not define any resources."); + + if (resources->asns != NULL) + return pr_err("Certificate inherits ASN resources while also defining others of its own."); + if (parent->asns == NULL) + return pr_err("Certificate inherits ASN resources from parent, but parent lacks ASN resources."); - if (resources->asns != NULL) { - pr_err("Certificate inherits ASN resources while also defining others of its own."); - return -EINVAL; - } - if (parent->asns == NULL) { - pr_err("Certificate inherits ASN resources from parent, but parent lacks ASN resources."); - return -EINVAL; - } resources->asns = parent->asns; rasn_get(resources->asns); pr_debug(""); @@ -464,17 +420,13 @@ add_asn(struct resources *resources, ASId_t min, ASId_t max, { int error; - if (parent && !rasn_contains(parent->asns, min, max)) { - pr_err("Parent certificate doesn't own child's ASN resource."); - return -EINVAL; - } + if (parent && !rasn_contains(parent->asns, min, max)) + return pr_err("Parent certificate doesn't own child's ASN resource."); if (resources->asns == NULL) { resources->asns = rasn_create(); - if (resources->asns == NULL) { - pr_err("Out of memory."); - return -ENOMEM; - } + if (resources->asns == NULL) + return pr_enomem(); } error = rasn_add(resources->asns, min, max); @@ -498,10 +450,8 @@ add_asior(struct resources *resources, struct ASIdOrRange *obj) parent = get_parent_resources(); - if ((parent != NULL) && (resources->asns == parent->asns)) { - pr_err("Certificate defines ASN resources while also inheriting his parent's."); - return -EINVAL; - } + if ((parent != NULL) && (resources->asns == parent->asns)) + return pr_err("Certificate defines ASN resources while also inheriting his parent's."); switch (obj->present) { case ASIdOrRange_PR_NOTHING: @@ -514,8 +464,7 @@ add_asior(struct resources *resources, struct ASIdOrRange *obj) obj->choice.range.max, parent); } - pr_err("Unknown ASIdOrRange type: %d", obj->present); - return -EINVAL; + return pr_err("Unknown ASIdOrRange type: %d", obj->present); } int @@ -525,14 +474,10 @@ resources_add_asn(struct resources *resources, struct ASIdentifiers *ids) int i; int error; - if (ids->asnum == NULL) { - pr_err("ASN extension lacks 'asnum' element."); - return -EINVAL; - } - if (ids->rdi != NULL) { - pr_err("ASN extension has 'rdi' element. (Prohibited by RFC6487)"); - return -EINVAL; - } + if (ids->asnum == NULL) + return pr_err("ASN extension lacks 'asnum' element."); + if (ids->rdi != NULL) + return pr_err("ASN extension has 'rdi' element. (Prohibited by RFC6487)"); switch (ids->asnum->present) { case ASIdentifierChoice_PR_inherit: @@ -550,8 +495,7 @@ resources_add_asn(struct resources *resources, struct ASIdentifiers *ids) break; } - pr_err("Unknown ASIdentifierChoice: %d", ids->asnum->present); - return -EINVAL; + return pr_err("Unknown ASIdentifierChoice: %d", ids->asnum->present); } bool @@ -579,7 +523,7 @@ restack_create(void) result = malloc(sizeof(struct restack)); if (result == NULL) { - pr_err("Out of memory."); + pr_enomem(); return NULL; } diff --git a/src/sorted_array.c b/src/sorted_array.c index 3e8e28af..6c535593 100644 --- a/src/sorted_array.c +++ b/src/sorted_array.c @@ -100,8 +100,7 @@ compare(struct sorted_array *sarray, void *new) return -EINTERSECTION; } - pr_err("Programming error: Unknown comparison value: %d", cmp); - return -EINVAL; + return pr_crit("Unknown comparison value: %d", cmp); } int @@ -161,7 +160,7 @@ sarray_contains(struct sorted_array *sarray, void *elem) break; } - pr_err("Programming error: Unknown comparison value: %d", cmp); + pr_crit("Unknown comparison value: %d", cmp); return false; } diff --git a/src/state.c b/src/state.c index c9ada7cc..3dff9e60 100644 --- a/src/state.c +++ b/src/state.c @@ -195,16 +195,12 @@ validation_pop_cert(struct validation *state) { struct resources *resources; - if (sk_X509_pop(state->trusted) == NULL) { - return crypto_err( - "Programming error: Attempted to pop empty cert stack"); - } + if (sk_X509_pop(state->trusted) == NULL) + return pr_crit("Attempted to pop empty certificate stack"); resources = restack_pop(state->rsrcs); - if (resources == NULL) { - pr_err("Programming error: Attempted to pop empty resource stack"); - return -EINVAL; - } + if (resources == NULL) + return pr_crit("Attempted to pop empty resource stack"); resources_destroy(resources); return 0; -- 2.47.3