indent: Reformat and extend some comments in dirmngr.

--

Signed-off-by: Werner Koch <wk@gnupg.org>

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
+ * converted from the hex string HEXSN.  Return NULL on memory
-   error. */
+ * 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
+   * This is because we can assume that he already used the
-     certificate while checking for the CRL. */
+   * 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
+ * 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
+ * resources.  Note, that the external search does not work for user
-   certificates because the LDAP lookup is on the caCertificate
+ * certificates because the LDAP lookup is on the caCertificate
-   attribute. For our purposes this is just fine.  */
+ * 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
+   * them.  This is because these certificates will really be the
-     required ones and thus even in the case that they can't be
+   * 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
+   * uniquely located by the following code we can use them.  This is
-     for example required by Telesec certificates where a keyId is
+   * for example required by Telesec certificates where a keyId is
-     used but the issuer certificate comes without a subject keyId! */
+   * 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");
     }
 
-
+  /* No check whether the certificate is cached.  */
-  /* First we 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
+   * This is because we can assume that he already used the
-     certificate while checking for the CRL. */
+   * 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
+       * passed pattern, so we need to make sure that an exact subject
-         search is done. */
+       * 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
+ * and return it at R_CERT.  Returns 0 on success or
-   GPG_ERR_NOT_FOUND.  */
+ * 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
+           * keyIdentifier look for a certificate with a matching
-             SKI. */
+           * 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
+       * an issuer certificate was found and gpgsm prints BAD
-         signature because it is not the correct one. */
+       * 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
+   * that there is only one such certificate.  We only look into our
-     cache then. */
+   * cache then.  */
   if (err || !issuer_cert)
     {
       issuer_cert = get_cert_bysubject (issuer_dn, 0);

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
+      /* FIXME: We now support https.
-         scheme and it is frankly not useful for CRL retrieval anyway.
+       * Our HTTP code does not support TLS, thus we can't use this
-         We resort to using http, assuming that the server also
+       * scheme and it is frankly not useful for CRL retrieval anyway.
-         provides plain http access. */
+       * 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
+/* Fetch a CA certificate for DN using the default server.  This
-   function only initiates the fetch; fetch_next_cert must be used to
+ * function only initiates the fetch; fetch_next_cert must be used to
-   actually read the certificate; end_cert_fetch to end the
+ * actually read the certificate; end_cert_fetch to end the
-   operation. */
+ * 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)
 {

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)
 {

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
+/* Ask back to return a certificate for NAME, given as a regular gpgsm
-   gpgsm certificate indentificates (e.g. fingerprint or one of the
+ * certificate identifier (e.g. fingerprint or one of the other
-   other methods).  Alternatively, NULL may be used for NAME to
+ * methods).  Alternatively, NULL may be used for NAME to return the
-   return the current target certificate. Either return the certificate
+ * current target certificate.  Either return the certificate in a
-   in a KSBA object or NULL if it is not available.
+ * 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
+   * version for validation because this will take care of any cached
-     results. */
+   * results. */
   {
     unsigned char fpr[20];
     ksba_cert_t tmpcert;

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
+      /* Now check the signature of the certificate.  FIXME: we should
-         should delay this until later so that faked certificates
+       * delay this until later so that faked certificates can't be
-         can't be turned into a DoS easily.  */
+       * 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
+          /* Return a more descriptive error code than the one
-             returned from the signature checking. */
+           * 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.
+       * certificate for revocations.
-
+       *
-         1. item in the chain  - The root certificate.
+       * 1. item in the chain  - The root certificate.
-         2. item               - the CA below the root
+       * 2. item               - the CA below the root
-         last item             - the target certificate.
+       * last item             - the target certificate.
-
+       *
-         Now for each certificate in the chain check whether it has
+       * 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
+       * 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
+       * here because this does not seem to make much sense.  This
-         might become a recursive process and we should better cache
+       * might become a recursive process and we should better cache
-         our validity results to avoid double work.  Far worse a
+       * our validity results to avoid double work.  Far worse a
-         catch-22 may happen for an improper setup hierarchy and we
+       * catch-22 may happen for an improper setup hierarchy and we
-         need a way to break up such a deadlock. */
+       * 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
+       * for all certificates in the chain.  Note that we can't use
-         the cache if the caller requested to check the trustiness of
+       * the cache if the caller requested to check the trustiness of
-         the root certificate himself.  Adding such a feature would
+       * the root certificate himself.  Adding such a feature would
-         require us to also store the fingerprint of root
+       * require us to also store the fingerprint of root
-         certificate.  */
+       * 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
+ * does only test the cryptographic signature and nothing else.  It is
-   assumed that the ISSUER_CERT is valid. */
+ * 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:
+   * have these values:
-
+   *
-     S_PKEY    - S-expression with the issuer's public key.
+   * S_PKEY    - S-expression with the issuer's public key.
-     S_SIG     - Signature value as given in the certrificate.
+   * S_SIG     - Signature value as given in the certificate.
-     MD        - Finalized hash context with hash of the certificate.
+   * MD        - Finalized hash context with hash of the certificate.
-     ALGO_NAME - Lowercase hash algorithm name
+   * 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) )