From 1af733f37bf6fd55ccac787a7e34c3b3ca002126 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 16 Feb 2017 10:35:18 +0100 Subject: [PATCH] indent: Reformat and extend some comments in dirmngr. -- Signed-off-by: Werner Koch --- dirmngr/certcache.c | 58 +++++++++++++++---------------- dirmngr/crlfetch.c | 19 +++++----- dirmngr/misc.c | 2 ++ dirmngr/server.c | 29 ++++++++-------- dirmngr/validate.c | 84 ++++++++++++++++++++++++--------------------- 5 files changed, 100 insertions(+), 92 deletions(-) diff --git a/dirmngr/certcache.c b/dirmngr/certcache.c index 10757c890..d13d80b44 100644 --- a/dirmngr/certcache.c +++ b/dirmngr/certcache.c @@ -154,8 +154,8 @@ compare_serialno (ksba_sexp_t serial1, ksba_sexp_t serial2 ) /* Return a malloced canonical S-Expression with the serial number - converted from the hex string HEXSN. Return NULL on memory - error. */ + * converted from the hex string HEXSN. Return NULL on memory + * error. */ ksba_sexp_t hexsn_to_sexp (const char *hexsn) { @@ -981,7 +981,7 @@ get_certs_bypattern (const char *pattern, /* Return the certificate matching ISSUER_DN and SERIALNO; if it is - not already in the cache, try to find it from other resources. */ + * not already in the cache, try to find it from other resources. */ ksba_cert_t find_cert_bysn (ctrl_t ctrl, const char *issuer_dn, ksba_sexp_t serialno) { @@ -996,8 +996,8 @@ find_cert_bysn (ctrl_t ctrl, const char *issuer_dn, ksba_sexp_t serialno) return cert; /* Ask back to the service requester to return the certificate. - This is because we can assume that he already used the - certificate while checking for the CRL. */ + * This is because we can assume that he already used the + * certificate while checking for the CRL. */ hexsn = serial_hex (serialno); if (!hexsn) { @@ -1093,10 +1093,10 @@ find_cert_bysn (ctrl_t ctrl, const char *issuer_dn, ksba_sexp_t serialno) /* Return the certificate matching SUBJECT_DN and (if not NULL) - KEYID. If it is not already in the cache, try to find it from other - resources. Note, that the external search does not work for user - certificates because the LDAP lookup is on the caCertificate - attribute. For our purposes this is just fine. */ + * KEYID. If it is not already in the cache, try to find it from other + * resources. Note, that the external search does not work for user + * certificates because the LDAP lookup is on the caCertificate + * attribute. For our purposes this is just fine. */ ksba_cert_t find_cert_bysubject (ctrl_t ctrl, const char *subject_dn, ksba_sexp_t keyid) { @@ -1107,11 +1107,11 @@ find_cert_bysubject (ctrl_t ctrl, const char *subject_dn, ksba_sexp_t keyid) ksba_sexp_t subj; /* If we have certificates from an OCSP request we first try to use - them. This is because these certificates will really be the - required ones and thus even in the case that they can't be - uniquely located by the following code we can use them. This is - for example required by Telesec certificates where a keyId is - used but the issuer certificate comes without a subject keyId! */ + * them. This is because these certificates will really be the + * required ones and thus even in the case that they can't be + * uniquely located by the following code we can use them. This is + * for example required by Telesec certificates where a keyId is + * used but the issuer certificate comes without a subject keyId! */ if (ctrl->ocsp_certs && subject_dn) { cert_item_t ci; @@ -1136,8 +1136,7 @@ find_cert_bysubject (ctrl_t ctrl, const char *subject_dn, ksba_sexp_t keyid) log_debug ("find_cert_bysubject: certificate not in ocsp_certs\n"); } - - /* First we check whether the certificate is cached. */ + /* No check whether the certificate is cached. */ for (seq=0; (cert = get_cert_bysubject (subject_dn, seq)); seq++) { if (!keyid) @@ -1158,15 +1157,15 @@ find_cert_bysubject (ctrl_t ctrl, const char *subject_dn, ksba_sexp_t keyid) log_debug ("find_cert_bysubject: certificate not in cache\n"); /* Ask back to the service requester to return the certificate. - This is because we can assume that he already used the - certificate while checking for the CRL. */ + * This is because we can assume that he already used the + * certificate while checking for the CRL. */ if (keyid) cert = get_cert_local_ski (ctrl, subject_dn, keyid); else { /* In contrast to get_cert_local_ski, get_cert_local uses any - passed pattern, so we need to make sure that an exact subject - search is done. */ + * passed pattern, so we need to make sure that an exact subject + * search is done. */ char *buf; buf = strconcat ("/", subject_dn, NULL); @@ -1263,7 +1262,6 @@ find_cert_bysubject (ctrl_t ctrl, const char *subject_dn, ksba_sexp_t keyid) } - /* Return 0 if the certificate is a trusted certificate. Returns GPG_ERR_NOT_TRUSTED if it is not trusted or other error codes in case of systems errors. */ @@ -1294,8 +1292,8 @@ is_trusted_cert (ksba_cert_t cert) /* Given the certificate CERT locate the issuer for this certificate - and return it at R_CERT. Returns 0 on success or - GPG_ERR_NOT_FOUND. */ + * and return it at R_CERT. Returns 0 on success or + * GPG_ERR_NOT_FOUND. */ gpg_error_t find_issuing_cert (ctrl_t ctrl, ksba_cert_t cert, ksba_cert_t *r_cert) { @@ -1331,16 +1329,18 @@ find_issuing_cert (ctrl_t ctrl, ksba_cert_t cert, ksba_cert_t *r_cert) { issuer_cert = find_cert_bysn (ctrl, s, authidno); } + if (!issuer_cert && keyid) { /* Not found by issuer+s/n. Now that we have an AKI - keyIdentifier look for a certificate with a matching - SKI. */ + * keyIdentifier look for a certificate with a matching + * SKI. */ issuer_cert = find_cert_bysubject (ctrl, issuer_dn, keyid); } + /* Print a note so that the user does not feel too helpless when - an issuer certificate was found and gpgsm prints BAD - signature because it is not the correct one. */ + * an issuer certificate was found and gpgsm prints BAD + * signature because it is not the correct one. */ if (!issuer_cert) { log_info ("issuer certificate "); @@ -1366,8 +1366,8 @@ find_issuing_cert (ctrl_t ctrl, ksba_cert_t cert, ksba_cert_t *r_cert) } /* If this did not work, try just with the issuer's name and assume - that there is only one such certificate. We only look into our - cache then. */ + * that there is only one such certificate. We only look into our + * cache then. */ if (err || !issuer_cert) { issuer_cert = get_cert_bysubject (issuer_dn, 0); diff --git a/dirmngr/crlfetch.c b/dirmngr/crlfetch.c index 337fe6e4d..f7a23ffed 100644 --- a/dirmngr/crlfetch.c +++ b/dirmngr/crlfetch.c @@ -167,10 +167,11 @@ crl_fetch (ctrl_t ctrl, const char *url, ksba_reader_t *reader) http_release_parsed_uri (uri); if (err && !strncmp (url, "https:", 6)) { - /* Our HTTP code does not support TLS, thus we can't use this - scheme and it is frankly not useful for CRL retrieval anyway. - We resort to using http, assuming that the server also - provides plain http access. */ + /* FIXME: We now support https. + * Our HTTP code does not support TLS, thus we can't use this + * scheme and it is frankly not useful for CRL retrieval anyway. + * We resort to using http, assuming that the server also + * provides plain http access. */ free_this = xtrymalloc (strlen (url) + 1); if (free_this) { @@ -343,10 +344,10 @@ crl_fetch_default (ctrl_t ctrl, const char *issuer, ksba_reader_t *reader) } -/* Fetch a CA certificate for DN using the default server. This - function only initiates the fetch; fetch_next_cert must be used to - actually read the certificate; end_cert_fetch to end the - operation. */ +/* Fetch a CA certificate for DN using the default server. This + * function only initiates the fetch; fetch_next_cert must be used to + * actually read the certificate; end_cert_fetch to end the + * operation. */ gpg_error_t ca_cert_fetch (ctrl_t ctrl, cert_fetch_context_t *context, const char *dn) { @@ -417,7 +418,7 @@ fetch_next_cert (cert_fetch_context_t context, /* Fetch the next data from CONTEXT, assuming it is a certificate and return - it as a cert object in R_CERT. */ + * it as a cert object in R_CERT. */ gpg_error_t fetch_next_ksba_cert (cert_fetch_context_t context, ksba_cert_t *r_cert) { diff --git a/dirmngr/misc.c b/dirmngr/misc.c index 2ee6d82bd..6d7c963db 100644 --- a/dirmngr/misc.c +++ b/dirmngr/misc.c @@ -62,6 +62,8 @@ hashify_data( const char* data, size_t len ) return hexify_data (buf, 20, 0); } + +/* FIXME: Replace this by hextobin. */ char* hexify_data (const unsigned char* data, size_t len, int with_prefix) { diff --git a/dirmngr/server.c b/dirmngr/server.c index 32ce5bb33..bc373f5b0 100644 --- a/dirmngr/server.c +++ b/dirmngr/server.c @@ -403,12 +403,11 @@ do_get_cert_local (ctrl_t ctrl, const char *name, const char *command) -/* Ask back to return a certificate for name, given as a regular - gpgsm certificate indentificates (e.g. fingerprint or one of the - other methods). Alternatively, NULL may be used for NAME to - return the current target certificate. Either return the certificate - in a KSBA object or NULL if it is not available. -*/ +/* Ask back to return a certificate for NAME, given as a regular gpgsm + * certificate identifier (e.g. fingerprint or one of the other + * methods). Alternatively, NULL may be used for NAME to return the + * current target certificate. Either return the certificate in a + * KSBA object or NULL if it is not available. */ ksba_cert_t get_cert_local (ctrl_t ctrl, const char *name) { @@ -422,13 +421,12 @@ get_cert_local (ctrl_t ctrl, const char *name) } -/* Ask back to return the issuing certificate for name, given as a - regular gpgsm certificate indentificates (e.g. fingerprint or one - of the other methods). Alternatively, NULL may be used for NAME to - return thecurrent target certificate. Either return the certificate - in a KSBA object or NULL if it is not available. -*/ +/* Ask back to return the issuing certificate for NAME, given as a + * regular gpgsm certificate identifier (e.g. fingerprint or one + * of the other methods). Alternatively, NULL may be used for NAME to + * return the current target certificate. Either return the certificate + * in a KSBA object or NULL if it is not available. */ ksba_cert_t get_issuing_cert_local (ctrl_t ctrl, const char *name) { @@ -441,8 +439,9 @@ get_issuing_cert_local (ctrl_t ctrl, const char *name) return do_get_cert_local (ctrl, name, "SENDISSUERCERT"); } + /* Ask back to return a certificate with subject NAME and a - subjectKeyIdentifier of KEYID. */ + * subjectKeyIdentifier of KEYID. */ ksba_cert_t get_cert_local_ski (ctrl_t ctrl, const char *name, ksba_sexp_t keyid) { @@ -1773,8 +1772,8 @@ cmd_validate (assuan_context_t ctx, char *line) goto leave; /* If we have this certificate already in our cache, use the cached - version for validation because this will take care of any cached - results. */ + * version for validation because this will take care of any cached + * results. */ { unsigned char fpr[20]; ksba_cert_t tmpcert; diff --git a/dirmngr/validate.c b/dirmngr/validate.c index b3dc9d8c6..68e1bb387 100644 --- a/dirmngr/validate.c +++ b/dirmngr/validate.c @@ -371,7 +371,8 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, int depth, maxdepth; char *issuer = NULL; char *subject = NULL; - ksba_cert_t subject_cert = NULL, issuer_cert = NULL; + ksba_cert_t subject_cert = NULL; + ksba_cert_t issuer_cert = NULL; ksba_isotime_t current_time; ksba_isotime_t exptime; int any_expired = 0; @@ -438,7 +439,7 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, /* We walk up the chain until we find a trust anchor. */ subject_cert = cert; - maxdepth = 10; + maxdepth = 10; /* Sensible limit on the length of the chain. */ chain = NULL; depth = 0; for (;;) @@ -520,7 +521,7 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, goto leave; /* Is this a self-signed certificate? */ - if (is_root_cert ( subject_cert, issuer, subject)) + if (is_root_cert (subject_cert, issuer, subject)) { /* Yes, this is our trust anchor. */ if (check_cert_sig (subject_cert, subject_cert) ) @@ -630,9 +631,9 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, dump_cert ("issuer", issuer_cert); } - /* Now check the signature of the certificate. Well, we - should delay this until later so that faked certificates - can't be turned into a DoS easily. */ + /* Now check the signature of the certificate. FIXME: we should + * delay this until later so that faked certificates can't be + * turned into a DoS easily. */ err = check_cert_sig (issuer_cert, subject_cert); if (err) { @@ -669,14 +670,14 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, } } #endif - /* We give a more descriptive error code than the one - returned from the signature checking. */ + /* Return a more descriptive error code than the one + * returned from the signature checking. */ err = gpg_error (GPG_ERR_BAD_CERT_CHAIN); goto leave; } /* Check that the length of the chain is not longer than allowed - by the CA. */ + * by the CA. */ { int chainlen; @@ -722,9 +723,11 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, issuer_cert = NULL; } + /* Even if we have no error here we need to check whether we + * encountered an error somewhere during the checks. Set the error + * code to the most critical one. */ if (!err) - { /* If we encountered an error somewhere during the checks, set - the error code to the most critical one */ + { if (any_expired) err = gpg_error (GPG_ERR_CERT_EXPIRED); else if (any_no_policy_match) @@ -742,19 +745,19 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, if (!err && mode != VALIDATE_MODE_CRL) { /* Now that everything is fine, walk the chain and check each - certificate for revocations. - - 1. item in the chain - The root certificate. - 2. item - the CA below the root - last item - the target certificate. - - Now for each certificate in the chain check whether it has - been included in a CRL and thus be revoked. We don't do OCSP - here because this does not seem to make much sense. This - might become a recursive process and we should better cache - our validity results to avoid double work. Far worse a - catch-22 may happen for an improper setup hierarchy and we - need a way to break up such a deadlock. */ + * certificate for revocations. + * + * 1. item in the chain - The root certificate. + * 2. item - the CA below the root + * last item - the target certificate. + * + * Now for each certificate in the chain check whether it has + * been included in a CRL and thus be revoked. We don't do OCSP + * here because this does not seem to make much sense. This + * might become a recursive process and we should better cache + * our validity results to avoid double work. Far worse a + * catch-22 may happen for an improper setup hierarchy and we + * need a way to break up such a deadlock. */ err = check_revocations (ctrl, chain); } @@ -773,11 +776,11 @@ validate_cert_chain (ctrl_t ctrl, ksba_cert_t cert, ksba_isotime_t r_exptime, if (!err && !(r_trust_anchor && *r_trust_anchor)) { /* With no error we can update the validation cache. We do this - for all certificates in the chain. Note that we can't use - the cache if the caller requested to check the trustiness of - the root certificate himself. Adding such a feature would - require us to also store the fingerprint of root - certificate. */ + * for all certificates in the chain. Note that we can't use + * the cache if the caller requested to check the trustiness of + * the root certificate himself. Adding such a feature would + * require us to also store the fingerprint of root + * certificate. */ chain_item_t citem; time_t validated_at = gnupg_get_time (); @@ -853,8 +856,8 @@ pk_algo_from_sexp (gcry_sexp_t pkey) /* Check the signature on CERT using the ISSUER_CERT. This function - does only test the cryptographic signature and nothing else. It is - assumed that the ISSUER_CERT is valid. */ + * does only test the cryptographic signature and nothing else. It is + * assumed that the ISSUER_CERT is valid. */ static gpg_error_t check_cert_sig (ksba_cert_t issuer_cert, ksba_cert_t cert) { @@ -952,20 +955,23 @@ check_cert_sig (ksba_cert_t issuer_cert, ksba_cert_t cert) /* Prepare the values for signature verification. At this point we - have these values: - - S_PKEY - S-expression with the issuer's public key. - S_SIG - Signature value as given in the certrificate. - MD - Finalized hash context with hash of the certificate. - ALGO_NAME - Lowercase hash algorithm name + * have these values: + * + * S_PKEY - S-expression with the issuer's public key. + * S_SIG - Signature value as given in the certificate. + * MD - Finalized hash context with hash of the certificate. + * ALGO_NAME - Lowercase hash algorithm name */ digestlen = gcry_md_get_algo_dlen (algo); digest = gcry_md_read (md, algo); if (pk_algo_from_sexp (s_pkey) == GCRY_PK_DSA) { + /* NB.: We support only SHA-1 here because we had problems back + * then to get test data for DSA-2. Meanwhile DSA has been + * replaced by ECDSA which we do not yet support. */ if (digestlen != 20) { - log_error (_("DSA requires the use of a 160 bit hash algorithm\n")); + log_error ("DSA requires the use of a 160 bit hash algorithm\n"); gcry_md_close (md); gcry_sexp_release (s_sig); gcry_sexp_release (s_pkey); @@ -975,7 +981,7 @@ check_cert_sig (ksba_cert_t issuer_cert, ksba_cert_t cert) (int)digestlen, digest) ) BUG (); } - else /* Not DSA. */ + else /* Not DSA - we assume RSA */ { if ( gcry_sexp_build (&s_hash, NULL, "(data(flags pkcs1)(hash %s %b))", algo_name, (int)digestlen, digest) )