indent: Reformat and extend some comments in dirmngr.
authorWerner Koch <wk@gnupg.org>
Thu, 16 Feb 2017 09:35:18 +0000 (10:35 +0100)
committerWerner Koch <wk@gnupg.org>
Thu, 16 Feb 2017 10:01:19 +0000 (11:01 +0100)
--

Signed-off-by: Werner Koch <wk@gnupg.org>
dirmngr/certcache.c
dirmngr/crlfetch.c
dirmngr/misc.c
dirmngr/server.c
dirmngr/validate.c

index 10757c8..d13d80b 100644 (file)
@@ -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,
 
 \f
 /* 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)
 
 \f
 /* 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);
index 337fe6e..f7a23ff 100644 (file)
@@ -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)
 {
index 2ee6d82..6d7c963 100644 (file)
@@ -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)
 {
index 32ce5bb..bc373f5 100644 (file)
@@ -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;
index b3dc9d8..68e1bb3 100644 (file)
@@ -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) )