Fix log.c comments about assert vs tor_assert vs raw_assert.

doc/tor.1.txt

@@ -2827,7 +2827,7 @@ The following options are used to configure a hidden service.
 
 [[HiddenServiceVersion]] **HiddenServiceVersion** **2**|**3**::
     A list of rendezvous service descriptor versions to publish for the hidden
-    service. Currently, versions 2 and 3 are supported. (Default: 2)
+    service. Currently, versions 2 and 3 are supported. (Default: 3)
 
 [[HiddenServiceAuthorizeClient]] **HiddenServiceAuthorizeClient** __auth-type__ __client-name__,__client-name__,__...__::
     If configured, the hidden service is accessible for authorized clients

src/feature/hs/hs_config.c

@@ -143,6 +143,24 @@ helper_parse_uint64(const char *opt, const char *value, uint64_t min,
   return ret;
 }
 
+/* Return the service version by trying to learn it from the key on disk if
+ * any. If nothing is found, the current service configured version is
+ * returned. */
+static int
+config_learn_service_version(hs_service_t *service)
+{
+  int version;
+
+  tor_assert(service);
+
+  version = hs_service_get_version_from_key(service);
+  if (version < 0) {
+    version = service->config.version;
+  }
+
+  return version;
+}
+
 /* Return true iff the given options starting at line_ for a hidden service
  * contains at least one invalid option. Each hidden service option don't
  * apply to all versions so this function can find out. The line_ MUST start
@@ -490,6 +508,12 @@ config_service(const config_line_t *line, const or_options_t *options,
                                    0) < 0) {
     goto err;
   }
+  /* We'll try to learn the service version here by loading the key(s) if
+   * present. Depending on the key format, we can figure out the service
+   * version. If we can't find a key, the configuration version will be used
+   * which has been set previously. */
+  service->config.version = config_learn_service_version(service);
+
   /* Different functions are in charge of specific options for a version. We
    * start just after the service directory line so once we hit another
    * directory line, the function knows that it has to stop parsing. */

src/feature/hs/hs_service.c

@@ -1037,7 +1037,7 @@ load_service_keys(hs_service_t *service)
   /* Don't ask for key creation, we want to know if we were able to load it or
    * we had to generate it. Better logging! */
   kp = ed_key_init_from_file(fname, INIT_ED_KEY_SPLIT, LOG_INFO, NULL, 0, 0,
-                             0, NULL);
+                             0, NULL, NULL);
   if (!kp) {
     log_info(LD_REND, "Unable to load keys from %s. Generating it...", fname);
     /* We'll now try to generate the keys and for it we want the strongest
@@ -1045,7 +1045,7 @@ load_service_keys(hs_service_t *service)
     uint32_t key_flags = INIT_ED_KEY_CREATE | INIT_ED_KEY_EXTRA_STRONG |
                          INIT_ED_KEY_SPLIT;
     kp = ed_key_init_from_file(fname, key_flags, LOG_WARN, NULL, 0, 0, 0,
-                               NULL);
+                               NULL, NULL);
     if (!kp) {
       log_warn(LD_REND, "Unable to generate keys and save in %s.", fname);
       goto end;
@@ -2884,6 +2884,29 @@ service_add_fnames_to_list(const hs_service_t *service, smartlist_t *list)
   smartlist_add(list, hs_path_from_filename(s_dir, fname));
 }
 
+/* Return true iff the given service identity key is present on disk. */
+static int
+service_key_on_disk(const char *directory_path)
+{
+  int ret = 0;
+  char *fname;
+  ed25519_keypair_t *kp = NULL;
+
+  tor_assert(directory_path);
+
+  /* Build the v3 key path name and then try to load it. */
+  fname = hs_path_from_filename(directory_path, fname_keyfile_prefix);
+  kp = ed_key_init_from_file(fname, INIT_ED_KEY_SPLIT,
+                             LOG_DEBUG, NULL, 0, 0, 0, NULL, NULL);
+  if (kp) {
+    ret = 1;
+  }
+
+  ed25519_keypair_free(kp);
+  tor_free(fname);
+  return ret;
+}
+
 /* ========== */
 /* Public API */
 /* ========== */
@@ -3375,6 +3398,36 @@ hs_service_circuit_has_opened(origin_circuit_t *circ)
   }
 }
 
+/* Return the service version by looking at the key in the service directory.
+ * If the key is not found or unrecognized, -1 is returned. Else, the service
+ * version is returned. */
+int
+hs_service_get_version_from_key(const hs_service_t *service)
+{
+  int version = -1; /* Unknown version. */
+  const char *directory_path;
+
+  tor_assert(service);
+
+  /* We'll try to load the key for version 3. If not found, we'll try version
+   * 2 and if not found, we'll send back an unknown version (-1). */
+  directory_path = service->config.directory_path;
+
+  /* Version 3 check. */
+  if (service_key_on_disk(directory_path)) {
+    version = HS_VERSION_THREE;
+    goto end;
+  }
+  /* Version 2 check. */
+  if (rend_service_key_on_disk(directory_path)) {
+    version = HS_VERSION_TWO;
+    goto end;
+  }
+
+ end:
+  return version;
+}
+
 /* Load and/or generate keys for all onion services including the client
  * authorization if any. Return 0 on success, -1 on failure. */
 int

src/feature/hs/hs_service.h

@@ -24,7 +24,7 @@
 /* When loading and configuring a service, this is the default version it will
  * be configured for as it is possible that no HiddenServiceVersion is
  * present. */
-#define HS_SERVICE_DEFAULT_VERSION HS_VERSION_TWO
+#define HS_SERVICE_DEFAULT_VERSION HS_VERSION_THREE
 
 /* As described in the specification, service publishes their next descriptor
  * at a random time between those two values (in seconds). */
@@ -262,6 +262,7 @@ void hs_service_free_(hs_service_t *service);
 unsigned int hs_service_get_num_services(void);
 void hs_service_stage_services(const smartlist_t *service_list);
 int hs_service_load_all_keys(void);
+int hs_service_get_version_from_key(const hs_service_t *service);
 void hs_service_lists_fnames_for_sandbox(smartlist_t *file_list,
                                          smartlist_t *dir_list);
 int hs_service_set_conn_addr_port(const origin_circuit_t *circ,

src/feature/relay/routerkeys.c

@@ -248,6 +248,9 @@ write_secret_key(const ed25519_secret_key_t *key, int encrypted,
  * <b>fname</b>, with certificate type <b>cert_type</b>.  On failure, return
  * NULL; on success return the keypair.
  *
+ * The <b>options</b> is used to look at the change_key_passphrase value when
+ * writing to disk a secret key. It is safe to be NULL even in that case.
+ *
  * If INIT_ED_KEY_CREATE is set in <b>flags</b>, then create the key (and
  * certificate if requested) if it doesn't exist, and save it to disk.
  *
@@ -276,9 +279,6 @@ write_secret_key(const ed25519_secret_key_t *key, int encrypted,
  * secret key unless no public key is found.  Do not return a secret key. (but
  * create and save one if needed).
  *
- * If INIT_ED_KEY_NO_LOAD_SECRET is set in <b>flags</b>, don't try to load
- * a secret key, no matter what.
- *
  * If INIT_ED_KEY_TRY_ENCRYPTED is set, we look for an encrypted secret key
  * and consider encrypting any new secret key.
  *
@@ -291,6 +291,9 @@ write_secret_key(const ed25519_secret_key_t *key, int encrypted,
  *
  * If INIT_ED_KEY_EXPLICIT_FNAME is set, use the provided file name for the
  * secret key file, encrypted or not.
+ *
+ * If INIT_ED_KEY_OFFLINE_SECRET is set, we won't try to load the master
+ * secret key and we log a message at <b>severity</b> that we've done so.
  */
 ed25519_keypair_t *
 ed_key_init_from_file(const char *fname, uint32_t flags,
@@ -299,7 +302,8 @@ ed_key_init_from_file(const char *fname, uint32_t flags,
                       time_t now,
                       time_t lifetime,
                       uint8_t cert_type,
-                      struct tor_cert_st **cert_out)
+                      struct tor_cert_st **cert_out,
+                      const or_options_t *options)
 {
   char *secret_fname = NULL;
   char *encrypted_secret_fname = NULL;
@@ -503,7 +507,8 @@ ed_key_init_from_file(const char *fname, uint32_t flags,
 
   /* Write it to disk if we're supposed to do with a new passphrase, or if
    * we just created it. */
-  if (created_sk || (have_secret && get_options()->change_key_passphrase)) {
+  if (created_sk || (have_secret && options != NULL &&
+                     options->change_key_passphrase)) {
     if (write_secret_key(&keypair->seckey,
                          encrypt_key,
                          secret_fname, tag, encrypted_secret_fname) < 0
@@ -734,7 +739,7 @@ load_ed_keys(const or_options_t *options, time_t now)
                INIT_ED_KEY_NEEDCERT|
                INIT_ED_KEY_INCLUDE_SIGNING_KEY_IN_CERT,
                LOG_INFO,
-               NULL, 0, 0, CERT_TYPE_ID_SIGNING, &sign_cert);
+               NULL, 0, 0, CERT_TYPE_ID_SIGNING, &sign_cert, options);
     tor_free(fname);
     check_signing_cert = sign_cert;
     use_signing = sign;
@@ -836,7 +841,7 @@ load_ed_keys(const or_options_t *options, time_t now)
     id = ed_key_init_from_file(
              fname,
              flags,
-             LOG_WARN, NULL, 0, 0, 0, NULL);
+             LOG_WARN, NULL, 0, 0, 0, NULL, options);
     tor_free(fname);
     if (!id) {
       if (need_new_signing_key) {
@@ -904,7 +909,7 @@ load_ed_keys(const or_options_t *options, time_t now)
                                  flags, LOG_WARN,
                                  sign_signing_key_with_id, now,
                                  options->SigningKeyLifetime,
-                                 CERT_TYPE_ID_SIGNING, &sign_cert);
+                                 CERT_TYPE_ID_SIGNING, &sign_cert, options);
     tor_free(fname);
     if (!sign)
       FAIL("Missing signing key");

src/feature/relay/routerkeys.h

@@ -27,7 +27,8 @@ ed25519_keypair_t *ed_key_init_from_file(const char *fname, uint32_t flags,
                                          time_t now,
                                          time_t lifetime,
                                          uint8_t cert_type,
-                                         struct tor_cert_st **cert_out);
+                                         struct tor_cert_st **cert_out,
+                                         const or_options_t *options);
 ed25519_keypair_t *ed_key_new(const ed25519_keypair_t *signing_key,
                               uint32_t flags,
                               time_t now,

src/feature/rend/rendservice.c

@@ -1341,6 +1341,29 @@ rend_service_poison_new_single_onion_dir(const rend_service_t *s,
   return 0;
 }
 
+/* Return true iff the given service identity key is present on disk. This is
+ * used to try to learn the service version during configuration time. */
+int
+rend_service_key_on_disk(const char *directory_path)
+{
+  int ret = 0;
+  char *fname;
+  crypto_pk_t *pk = NULL;
+
+  tor_assert(directory_path);
+
+  /* Load key */
+  fname = hs_path_from_filename(directory_path, private_key_fname);
+  pk = init_key_from_file(fname, 0, LOG_DEBUG, 0);
+  if (pk) {
+    ret = 1;
+  }
+
+  crypto_pk_free(pk);
+  tor_free(fname);
+  return ret;
+}
+
 /** Load and/or generate private keys for all hidden services, possibly
  * including keys for client authorization.
  * If a <b>service_list</b> is provided, treat it as the list of hidden

src/feature/rend/rendservice.h

@@ -145,6 +145,7 @@ int rend_config_service(const struct config_line_t *line_,
 void rend_service_prune_list(void);
 void rend_service_free_staging_list(void);
 int rend_service_load_all_keys(const smartlist_t *service_list);
+int rend_service_key_on_disk(const char *directory_path);
 void rend_services_add_filenames_to_lists(smartlist_t *open_lst,
                                           smartlist_t *stat_lst);
 void rend_consider_services_intro_points(time_t now);

src/lib/log/log.c

@@ -101,7 +101,7 @@ sev_to_string(int severity)
     case LOG_NOTICE:  return "notice";
     case LOG_WARN:    return "warn";
     case LOG_ERR:     return "err";
-    default:     /* Call assert, not tor_assert, since tor_assert
+    default:     /* Call raw_assert, not tor_assert, since tor_assert
                   * calls log on failure. */
                  raw_assert_unreached(); return "UNKNOWN"; // LCOV_EXCL_LINE
   }
@@ -122,7 +122,8 @@ should_log_function_name(log_domain_mask_t domain, int severity)
       /* We care about places where bugs occur. */
       return (domain & (LD_BUG|LD_NOFUNCNAME)) == LD_BUG;
     default:
-      /* Call assert, not tor_assert, since tor_assert calls log on failure. */
+      /* Call raw_assert, not tor_assert, since tor_assert calls
+       * log on failure. */
       raw_assert(0); return 0; // LCOV_EXCL_LINE
   }
 }
@@ -579,7 +580,7 @@ logv,(int severity, log_domain_mask_t domain, const char *funcname,
   char *end_of_prefix=NULL;
   int callbacks_deferred = 0;
 
-  /* Call assert, not raw_assert, since raw_assert calls log on failure. */
+  /* Call raw_assert, not tor_assert, since tor_assert calls log on failure. */
   raw_assert(format);
   /* check that severity is sane.  Overrunning the masks array leads to
    * interesting and hard to diagnose effects */
@@ -694,7 +695,7 @@ tor_log_update_sigsafe_err_fds(void)
   if (!found_real_stderr &&
       int_array_contains(fds, n_fds, STDOUT_FILENO)) {
     /* Don't use a virtual stderr when we're also logging to stdout. */
-    raw_assert(n_fds >= 2); /* Don't raw_assert inside log fns */
+    raw_assert(n_fds >= 2); /* Don't tor_assert inside log fns */
     fds[0] = fds[--n_fds];
   }
 

src/test/test_routerkeys.c

@@ -262,13 +262,14 @@ test_routerkeys_ed_key_init_basic(void *arg)
   unlink(fname2);
 
   /* Fail to load a key that isn't there. */
-  kp1 = ed_key_init_from_file(fname1, 0, LOG_INFO, NULL, now, 0, 7, &cert);
+  kp1 = ed_key_init_from_file(fname1, 0, LOG_INFO, NULL, now, 0, 7, &cert,
+                              NULL);
   tt_assert(kp1 == NULL);
   tt_assert(cert == NULL);
 
   /* Create the key if requested to do so. */
   kp1 = ed_key_init_from_file(fname1, INIT_ED_KEY_CREATE, LOG_INFO,
-                              NULL, now, 0, 7, &cert);
+                              NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp1 != NULL);
   tt_assert(cert == NULL);
   tt_int_op(stat(get_fname("test_ed_key_1_cert"), &st), OP_LT, 0);
@@ -276,24 +277,24 @@ test_routerkeys_ed_key_init_basic(void *arg)
 
   /* Fail to load if we say we need a cert */
   kp2 = ed_key_init_from_file(fname1, INIT_ED_KEY_NEEDCERT, LOG_INFO,
-                              NULL, now, 0, 7, &cert);
+                              NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp2 == NULL);
 
   /* Fail to load if we say the wrong key type */
   kp2 = ed_key_init_from_file(fname1, 0, LOG_INFO,
-                              NULL, now, 0, 6, &cert);
+                              NULL, now, 0, 6, &cert, NULL);
   tt_assert(kp2 == NULL);
 
   /* Load successfully if we're not picky, whether we say "create" or not. */
   kp2 = ed_key_init_from_file(fname1, INIT_ED_KEY_CREATE, LOG_INFO,
-                              NULL, now, 0, 7, &cert);
+                              NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp2 != NULL);
   tt_assert(cert == NULL);
   tt_mem_op(kp1, OP_EQ, kp2, sizeof(*kp1));
   ed25519_keypair_free(kp2); kp2 = NULL;
 
   kp2 = ed_key_init_from_file(fname1, 0, LOG_INFO,
-                              NULL, now, 0, 7, &cert);
+                              NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp2 != NULL);
   tt_assert(cert == NULL);
   tt_mem_op(kp1, OP_EQ, kp2, sizeof(*kp1));
@@ -302,7 +303,7 @@ test_routerkeys_ed_key_init_basic(void *arg)
   /* Now create a key with a cert. */
   kp2 = ed_key_init_from_file(fname2, (INIT_ED_KEY_CREATE|
                                        INIT_ED_KEY_NEEDCERT),
-                              LOG_INFO, kp1, now, 7200, 7, &cert);
+                              LOG_INFO, kp1, now, 7200, 7, &cert, NULL);
   tt_assert(kp2 != NULL);
   tt_assert(cert != NULL);
   tt_mem_op(kp1, OP_NE, kp2, sizeof(*kp1));
@@ -315,7 +316,7 @@ test_routerkeys_ed_key_init_basic(void *arg)
   /* Now verify we can load the cert... */
   kp3 = ed_key_init_from_file(fname2, (INIT_ED_KEY_CREATE|
                                        INIT_ED_KEY_NEEDCERT),
-                              LOG_INFO, kp1, now, 7200, 7, &cert2);
+                              LOG_INFO, kp1, now, 7200, 7, &cert2, NULL);
   tt_mem_op(kp2, OP_EQ, kp3, sizeof(*kp2));
   tt_mem_op(cert2->encoded, OP_EQ, cert->encoded, cert->encoded_len);
   ed25519_keypair_free(kp3); kp3 = NULL;
@@ -323,7 +324,7 @@ test_routerkeys_ed_key_init_basic(void *arg)
 
   /* ... even without create... */
   kp3 = ed_key_init_from_file(fname2, INIT_ED_KEY_NEEDCERT,
-                              LOG_INFO, kp1, now, 7200, 7, &cert2);
+                              LOG_INFO, kp1, now, 7200, 7, &cert2, NULL);
   tt_mem_op(kp2, OP_EQ, kp3, sizeof(*kp2));
   tt_mem_op(cert2->encoded, OP_EQ, cert->encoded, cert->encoded_len);
   ed25519_keypair_free(kp3); kp3 = NULL;
@@ -331,13 +332,13 @@ test_routerkeys_ed_key_init_basic(void *arg)
 
   /* ... but that we don't crash or anything if we say we don't want it. */
   kp3 = ed_key_init_from_file(fname2, INIT_ED_KEY_NEEDCERT,
-                              LOG_INFO, kp1, now, 7200, 7, NULL);
+                              LOG_INFO, kp1, now, 7200, 7, NULL, NULL);
   tt_mem_op(kp2, OP_EQ, kp3, sizeof(*kp2));
   ed25519_keypair_free(kp3); kp3 = NULL;
 
   /* Fail if we're told the wrong signing key */
   kp3 = ed_key_init_from_file(fname2, INIT_ED_KEY_NEEDCERT,
-                              LOG_INFO, kp2, now, 7200, 7, &cert2);
+                              LOG_INFO, kp2, now, 7200, 7, &cert2, NULL);
   tt_assert(kp3 == NULL);
   tt_assert(cert2 == NULL);
 
@@ -368,13 +369,14 @@ test_routerkeys_ed_key_init_split(void *arg)
   unlink(fname2);
 
   /* Can't load key that isn't there. */
-  kp1 = ed_key_init_from_file(fname1, flags, LOG_INFO, NULL, now, 0, 7, &cert);
+  kp1 = ed_key_init_from_file(fname1, flags, LOG_INFO, NULL, now, 0, 7, &cert,
+                              NULL);
   tt_assert(kp1 == NULL);
   tt_assert(cert == NULL);
 
   /* Create a split key */
   kp1 = ed_key_init_from_file(fname1, flags|INIT_ED_KEY_CREATE,
-                              LOG_INFO, NULL, now, 0, 7, &cert);
+                              LOG_INFO, NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp1 != NULL);
   tt_assert(cert == NULL);
   tt_int_op(stat(get_fname("test_ed_key_3_cert"), &st), OP_LT, 0);
@@ -383,7 +385,7 @@ test_routerkeys_ed_key_init_split(void *arg)
 
   /* Load it. */
   kp2 = ed_key_init_from_file(fname1, flags|INIT_ED_KEY_CREATE,
-                              LOG_INFO, NULL, now, 0, 7, &cert);
+                              LOG_INFO, NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp2 != NULL);
   tt_assert(cert == NULL);
   tt_mem_op(kp1, OP_EQ, kp2, sizeof(*kp2));
@@ -392,7 +394,7 @@ test_routerkeys_ed_key_init_split(void *arg)
   /* Okay, try killing the secret key and loading it. */
   unlink(get_fname("test_ed_key_3_secret_key"));
   kp2 = ed_key_init_from_file(fname1, flags,
-                              LOG_INFO, NULL, now, 0, 7, &cert);
+                              LOG_INFO, NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp2 != NULL);
   tt_assert(cert == NULL);
   tt_mem_op(&kp1->pubkey, OP_EQ, &kp2->pubkey, sizeof(kp2->pubkey));
@@ -402,7 +404,7 @@ test_routerkeys_ed_key_init_split(void *arg)
 
   /* Even when we're told to "create", don't create if there's a public key */
   kp2 = ed_key_init_from_file(fname1, flags|INIT_ED_KEY_CREATE,
-                              LOG_INFO, NULL, now, 0, 7, &cert);
+                              LOG_INFO, NULL, now, 0, 7, &cert, NULL);
   tt_assert(kp2 != NULL);
   tt_assert(cert == NULL);
   tt_mem_op(&kp1->pubkey, OP_EQ, &kp2->pubkey, sizeof(kp2->pubkey));
@@ -412,7 +414,7 @@ test_routerkeys_ed_key_init_split(void *arg)
 
   /* Make sure we fail on a tag mismatch, though */
   kp2 = ed_key_init_from_file(fname1, flags,
-                              LOG_INFO, NULL, now, 0, 99, &cert);
+                              LOG_INFO, NULL, now, 0, 99, &cert, NULL);
   tt_assert(kp2 == NULL);
 
  done: