From: Alberto Leiva Popper Date: Fri, 22 Mar 2019 01:19:20 +0000 (-0600) Subject: Redo RFC 7935 X-Git-Tag: v0.0.2~63 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cbf6bba0aba64fdeb3b1d24c68987d574339cdcc;p=thirdparty%2FFORT-validator.git Redo RFC 7935 It's a little faster, and also adds some missing validations. --- diff --git a/src/algorithm.c b/src/algorithm.c index 84a10d5d..cec068b7 100644 --- a/src/algorithm.c +++ b/src/algorithm.c @@ -1,19 +1,142 @@ #include "algorithm.h" #include +#include "log.h" +/** + * This function can also be used for CRLs. + */ int -rpki_signature_algorithm(void) +validate_certificate_signature_algorithm(int nid, char const *what) { - return NID_sha256WithRSAEncryption; + if (nid == NID_sha256WithRSAEncryption) + return 0; + + return pr_err("%s's signature algorithm is NID '%d', not RSA+SHA256.", + what, nid); +} + +int +validate_certificate_public_key_algorithm(int nid) +{ + /* + * TODO (whatever) RFC says sha256WithRSAEncryption, but everyone uses + * rsaEncryption. + */ + if (nid == NID_rsaEncryption || nid == NID_sha256WithRSAEncryption) + return 0; + + return pr_err("Certificate's public key format is NID '%d', not RSA nor RSA+SHA256.", + nid); +} + +int +validate_cms_hashing_algorithm(AlgorithmIdentifier_t *id, char const *what) +{ + int error; + + if (id == NULL) + return pr_err("The hash algorithm of the '%s' is absent", what); + + error = validate_cms_hashing_algorithm_oid(&id->algorithm, what); + if (error) + return error; + + /* + * RFC 5754: + * There are two possible encodings for the AlgorithmIdentifier + * parameters field associated with these object identifiers. + * (...) + * some implementations encode parameters as a NULL element + * while others omit them entirely. The correct encoding is to omit the + * parameters field; + */ + if (id->parameters != NULL) + pr_warn("The hash algorithm of the '%s' has parameters", what); + + return 0; } int -rpki_public_key_algorithm(void) +validate_cms_hashing_algorithm_oid(OBJECT_IDENTIFIER_t *oid, char const *what) { /* - * TODO Everyone uses this algorithm, but the RFC says that it should - * be NID_sha256WithRSAEncryption. Wtf? + * RFC 7935: + * In CMS SignedData (...) The object identifier and + * parameters for SHA-256 (...) MUST be used for the + * SignedData digestAlgorithms field and the SignerInfo digestAlgorithm + * field. */ - return NID_rsaEncryption; + + static const unsigned char sha256[] = { + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, + }; + + if (oid == NULL) + return pr_err("The hash algorithm of the '%s' is absent", what); + + if (oid->size != sizeof(sha256)) + goto incorrect_oid; + if (memcmp(sha256, oid->buf, sizeof(sha256)) != 0) + goto incorrect_oid; + + return 0; + +incorrect_oid: + return pr_err("The hash algorithm of the '%s' is not SHA256.", what); +} + +int +validate_cms_signature_algorithm(AlgorithmIdentifier_t *id) +{ + /* + * RFC 7935: + * In CMS SignedData, (...) RPKI implementations MUST + * accept either rsaEncryption or sha256WithRSAEncryption for the + * SignerInfo signatureAlgorithm field when verifying CMS SignedData + * objects (...). + */ + + static const unsigned char pkcs1[] = { + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, + }; + uint8_t last; + + if (id == NULL) + return pr_err("The signature algorithm is absent."); + + /* + * rsaEncryption is { pkcs-1 1 }, and sha256WithRSAEncryption is + * { pkcs-1 11 }. + */ + if (id->algorithm.size != sizeof(pkcs1) + 1) + goto incorrect_oid; + if (memcmp(pkcs1, id->algorithm.buf, sizeof(pkcs1)) != 0) + goto incorrect_oid; + last = id->algorithm.buf[sizeof(pkcs1)]; + if (last != 1 && last != 11) + goto incorrect_oid; + + /* + * RFC 7935: + * The object identifier and parameters for rsaEncryption + * [RFC3370] MUST be used for the SignerInfo signatureAlgorithm field + * when generating CMS SignedData objects. + * + * RFC 3370 (1): + * When the rsaEncryption algorithm identifier is used, the + * AlgorithmIdentifier parameters field MUST contain NULL. + * + * RFC 4055 (11): + * When any of these four object identifiers appears within an + * AlgorithmIdentifier, the parameters MUST be NULL. Implementations + * MUST accept the parameters being absent as well as present. + */ + if (id->parameters != NULL) + pr_warn("The signature algorithm has parameters."); + + return 0; + +incorrect_oid: + return pr_err("The Signature algorithm is not RSA nor RSA+SHA256."); } diff --git a/src/algorithm.h b/src/algorithm.h index 0bd149fc..c1e88ec8 100644 --- a/src/algorithm.h +++ b/src/algorithm.h @@ -1,15 +1,18 @@ #ifndef SRC_ALGORITHM_H_ #define SRC_ALGORITHM_H_ +#include +#include + /** - * This file is an implementation of RFC 7935 (previously 6485) and its update, - * 8208. - * - * It's just a bunch of functions that return the NIDs of the algorithms RPKI - * validations are supposed to employ. + * This file is an implementation of RFC 7935 (previously 6485). */ -int rpki_signature_algorithm(void); -int rpki_public_key_algorithm(void); +int validate_certificate_signature_algorithm(int, char const *); +int validate_certificate_public_key_algorithm(int); + +int validate_cms_hashing_algorithm(AlgorithmIdentifier_t *, char const *); +int validate_cms_hashing_algorithm_oid(OBJECT_IDENTIFIER_t *, char const *); +int validate_cms_signature_algorithm(AlgorithmIdentifier_t *); #endif /* SRC_ALGORITHM_H_ */ diff --git a/src/asn1/oid.h b/src/asn1/oid.h index 14d0a1bc..2106d7db 100644 --- a/src/asn1/oid.h +++ b/src/asn1/oid.h @@ -39,9 +39,6 @@ typedef asn_oid_arc_t OID[]; #define OID_MANIFEST { 1, 2, 840, 113549, 1, 9, 16, 1, 26 } #define OID_GHOSTBUSTERS { 1, 2, 840, 113549, 1, 9, 16, 1, 35 } -#define OID_RSA { 1, 2, 840, 113549, 1, 1, 1 } -#define OID_SHA256 { 2, 16, 840, 1, 101, 3, 4, 2, 1 } - int oid2arcs(OBJECT_IDENTIFIER_t *, struct oid_arcs *); bool oid_equal(OBJECT_IDENTIFIER_t *, OBJECT_IDENTIFIER_t *); diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c index 04f41ba9..69671982 100644 --- a/src/asn1/signed_data.c +++ b/src/asn1/signed_data.c @@ -4,6 +4,7 @@ #include #include +#include "algorithm.h" #include "config.h" #include "log.h" #include "oid.h" @@ -40,24 +41,6 @@ signed_object_args_cleanup(struct signed_object_args *args) refs_cleanup(&args->refs); } -static int -is_digest_algorithm(AlgorithmIdentifier_t *id, char const *what) -{ - bool is_hash; - int error; - - if (id == NULL) - return pr_err("The %s algorithm is NULL.", what); - - error = hash_is_sha256(&id->algorithm, &is_hash); - if (error) - return error; - if (!is_hash) - return pr_err("The %s algorithm is not SHA256.", what); - - return 0; -} - static int get_sid(struct SignerInfo *sinfo, OCTET_STRING_t **result) { @@ -313,7 +296,7 @@ validate(struct SignedData *sdata, ANY_t *sdata_encoded, * AlgorithmIdentifier instead. There's no API. * This seems to work fine. */ - error = is_digest_algorithm( + error = validate_cms_hashing_algorithm( (AlgorithmIdentifier_t *) sdata->digestAlgorithms.list.array[0], "SignedData"); if (error) @@ -359,7 +342,8 @@ validate(struct SignedData *sdata, ANY_t *sdata_encoded, /* rfc6488#section-2.1.6.3 */ /* rfc6488#section-3.1.j 2/2 */ - error = is_digest_algorithm(&sinfo->digestAlgorithm, "SignerInfo"); + error = validate_cms_hashing_algorithm(&sinfo->digestAlgorithm, + "SignerInfo"); if (error) return error; @@ -370,28 +354,12 @@ validate(struct SignedData *sdata, ANY_t *sdata_encoded, /* rfc6488#section-2.1.6.5 */ /* rfc6488#section-3.1.k */ - /* - * RFC 6485 was obsoleted by 7935. 7935 simply refers to 5652. - * - * RFC 5652: - * - * > Since each signer can employ a different digital signature - * > technique, and future specifications could update the syntax, all - * > implementations MUST gracefully handle unimplemented versions of - * > SignerInfo. Further, since all implementations will not support - * > every possible signature algorithm, all implementations MUST - * > gracefully handle unimplemented signature algorithms when they are - * > encountered. - * - * So, nothing to do for now. - * - * TODO (field) "In the certificate, the OID appears in the signature - * and signatureAlgorithm fields [RFC4055]." So it has to be the same as - * some other field? - */ + error = validate_cms_signature_algorithm(&sinfo->signatureAlgorithm); + if (error) + return error; /* rfc6488#section-2.1.6.6 */ - /* Again, nothing to do for now. */ + /* Signature handled below. */ /* rfc6488#section-2.1.6.7 */ /* rfc6488#section-3.1.i */ diff --git a/src/crypto/hash.c b/src/crypto/hash.c index 31736b90..79926870 100644 --- a/src/crypto/hash.c +++ b/src/crypto/hash.c @@ -8,23 +8,6 @@ #include "log.h" #include "asn1/oid.h" -int -hash_is_sha256(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 int get_md(char const *algorithm, EVP_MD const **result) { diff --git a/src/crypto/hash.h b/src/crypto/hash.h index b44b76bf..c6693ad9 100644 --- a/src/crypto/hash.h +++ b/src/crypto/hash.h @@ -4,10 +4,8 @@ #include #include #include -#include #include "uri.h" -int hash_is_sha256(OBJECT_IDENTIFIER_t *, bool *); int hash_validate_file(char const *, struct rpki_uri const *uri, BIT_STRING_t const *); int hash_validate(char const *, unsigned char const *, size_t, diff --git a/src/object/certificate.c b/src/object/certificate.c index 31d0e5c0..968cfe01 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -68,13 +68,8 @@ validate_serial_number(X509 *cert) static int validate_signature_algorithm(X509 *cert) { - int nid; - - nid = OBJ_obj2nid(X509_get0_tbs_sigalg(cert)->algorithm); - if (nid != rpki_signature_algorithm()) - return pr_err("Certificate's Signature Algorithm is not RSASSA-PKCS1-v1_5."); - - return 0; + int nid = OBJ_obj2nid(X509_get0_tbs_sigalg(cert)->algorithm); + return validate_certificate_signature_algorithm(nid, "Certificate"); } static int @@ -119,20 +114,62 @@ validate_subject(X509 *cert) } static int -validate_spki(const unsigned char *cert_spk, int cert_spk_len) +spki_validate_algorithm(OBJECT_IDENTIFIER_t *tal_alg, + X509_PUBKEY *cert_public_key) +{ + ASN1_OBJECT *cert_alg; + int ok; + + ok = X509_PUBKEY_get0_param(&cert_alg, NULL, NULL, NULL, + cert_public_key); + if (!ok) + return crypto_err("X509_PUBKEY_get0_param() returned %d", ok); + + if (tal_alg->size != OBJ_length(cert_alg)) + goto fail; + if (memcmp(tal_alg->buf, OBJ_get0_data(cert_alg), tal_alg->size) != 0) + goto fail; + + return 0; + +fail: + return pr_err("TAL's public key algorithm is different than the root certificate's public key algorithm."); +} + +static int +spki_validate_key(BIT_STRING_t *tal_key, X509_PUBKEY *cert_public_key) +{ + unsigned char const *cert_spk; + int cert_spk_len; + int ok; + + ok = X509_PUBKEY_get0_param(NULL, &cert_spk, &cert_spk_len, NULL, + cert_public_key); + if (!ok) + return crypto_err("X509_PUBKEY_get0_param() returned %d", ok); + + if (tal_key->size != cert_spk_len) + goto fail; + if (memcmp(tal_key->buf, cert_spk, cert_spk_len) != 0) + goto fail; + + return 0; + +fail: + return pr_err("TAL's public key is different than the root certificate's public key."); +} + +static int +validate_spki(X509_PUBKEY *cert_public_key) { struct validation *state; struct tal *tal; + int error; 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; @@ -166,31 +203,18 @@ validate_spki(const unsigned char *cert_spk, int cert_spk_len) if (error) goto fail1; - /* Algorithm Identifier */ - error = oid2arcs(&tal_spki->algorithm.algorithm, &tal_alg_arcs); + error = spki_validate_algorithm(&tal_spki->algorithm.algorithm, + cert_public_key); + if (error) + goto fail2; + error = spki_validate_key(&tal_spki->subjectPublicKey, cert_public_key); if (error) goto fail2; - if (!ARCS_EQUAL_OIDS(&tal_alg_arcs, oid_rsa)) { - error = pr_err("TAL's public key format is not RSA PKCS#1 v1.5 with SHA-256."); - goto fail3; - } - - /* SPK */ - if (tal_spki->subjectPublicKey.size != cert_spk_len) - goto fail4; - if (memcmp(tal_spki->subjectPublicKey.buf, cert_spk, cert_spk_len) != 0) - goto fail4; - - free_arcs(&tal_alg_arcs); ASN_STRUCT_FREE(asn_DEF_SubjectPublicKeyInfo, tal_spki); validation_pubkey_valid(state); return 0; -fail4: - error = pr_err("TAL's public key is different than the root certificate's public key."); -fail3: - free_arcs(&tal_alg_arcs); fail2: ASN_STRUCT_FREE(asn_DEF_SubjectPublicKeyInfo, tal_spki); fail1: @@ -203,9 +227,6 @@ validate_public_key(X509 *cert, bool is_root) { X509_PUBKEY *pubkey; ASN1_OBJECT *alg; - int alg_nid; - const unsigned char *bytes; - int bytes_len; int ok; int error; @@ -213,15 +234,13 @@ validate_public_key(X509 *cert, bool is_root) if (pubkey == NULL) return crypto_err("X509_get_X509_PUBKEY() returned NULL"); - ok = X509_PUBKEY_get0_param(&alg, &bytes, &bytes_len, NULL, pubkey); + ok = X509_PUBKEY_get0_param(&alg, NULL, NULL, NULL, pubkey); if (!ok) return crypto_err("X509_PUBKEY_get0_param() returned %d", ok); - alg_nid = OBJ_obj2nid(alg); - if (alg_nid != rpki_public_key_algorithm()) { - return pr_err("Certificate's public key format is %d, not RSA PKCS#1 v1.5 with SHA-256.", - alg_nid); - } + error = validate_certificate_public_key_algorithm(OBJ_obj2nid(alg)); + if (error) + return error; /* * BTW: WTF. About that algorithm: @@ -236,7 +255,7 @@ validate_public_key(X509 *cert, bool is_root) */ if (is_root) { - error = validate_spki(bytes, bytes_len); + error = validate_spki(pubkey); if (error) return error; } diff --git a/src/object/crl.c b/src/object/crl.c index e36b49f6..aa15a5fb 100644 --- a/src/object/crl.c +++ b/src/object/crl.c @@ -120,8 +120,10 @@ crl_validate(X509_CRL *crl) if (version != 1) return pr_err("CRL version (%ld) is not v2 (%d).", version, 1); - if (X509_CRL_get_signature_nid(crl) != rpki_signature_algorithm()) - return pr_err("CRL's signature algorithm is not RSASSA-PKCS1-v1_5."); + error = validate_certificate_signature_algorithm( + X509_CRL_get_signature_nid(crl), "CRL"); + if (error) + return error; error = validate_issuer_name("CRL", X509_CRL_get_issuer(crl)); if (error) diff --git a/src/object/manifest.c b/src/object/manifest.c index 8ad79662..7d2d7753 100644 --- a/src/object/manifest.c +++ b/src/object/manifest.c @@ -4,6 +4,7 @@ #include #include +#include "algorithm.h" #include "log.h" #include "thread_var.h" #include "asn1/decode.h" @@ -81,7 +82,6 @@ static int validate_manifest(struct Manifest *manifest) { unsigned long version; - bool is_sha256; int error; /* rfc6486#section-4.2.1 */ @@ -127,12 +127,18 @@ validate_manifest(struct Manifest *manifest) if (error) return error; - /* rfc6486#section-6.6 (I guess) */ - error = hash_is_sha256(&manifest->fileHashAlg, &is_sha256); + /* rfc6486#section-4.2.1.fileHashAlg */ + /* + * Um, RFC 7935 does not declare a hash algorithm specifically intended + * for manifest hashes. But all the hashes it declares are SHA256, so + * I guess we'll just default to that. + * I'm going with the signed object hash function, since it appears to + * be the closest match. + */ + error = validate_cms_hashing_algorithm_oid(&manifest->fileHashAlg, + "manifest file"); if (error) return error; - if (!is_sha256) - return pr_err("The hash algorithm is not SHA256."); /* The file hashes will be validated during the traversal. */