diff options
| -rw-r--r-- | changes/ticket27838 | 4 | ||||
| -rw-r--r-- | src/feature/hs/hs_service.c | 134 | ||||
| -rw-r--r-- | src/feature/hs/hs_service.h | 72 | ||||
| -rw-r--r-- | src/test/test_hs_service.c | 4 |
4 files changed, 135 insertions, 79 deletions
diff --git a/changes/ticket27838 b/changes/ticket27838 new file mode 100644 index 0000000000..1699730d7a --- /dev/null +++ b/changes/ticket27838 @@ -0,0 +1,4 @@ + o Minor bugfixes (hidden service v3): + - Build the service descriptor signing key certificate before uploading so + we always have a fresh one leaving no chances for it to expire service + side. Fixes bug 27838; bugfix on 0.3.2.1-alpha. diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index aec2aa4381..7d56c9e2ad 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -1696,6 +1696,32 @@ build_desc_intro_points(const hs_service_t *service, } DIGEST256MAP_FOREACH_END; } +/* Build the descriptor signing key certificate. */ +static void +build_desc_signing_key_cert(hs_service_descriptor_t *desc, time_t now) +{ + hs_desc_plaintext_data_t *plaintext; + + tor_assert(desc); + tor_assert(desc->desc); + + /* Ease our life a bit. */ + plaintext = &desc->desc->plaintext_data; + + /* Get rid of what we have right now. */ + tor_cert_free(plaintext->signing_key_cert); + + /* Fresh certificate for the signing key. */ + plaintext->signing_key_cert = + tor_cert_create(&desc->blinded_kp, CERT_TYPE_SIGNING_HS_DESC, + &desc->signing_kp.pubkey, now, HS_DESC_CERT_LIFETIME, + CERT_FLAG_INCLUDE_SIGNING_KEY); + /* If the cert creation fails, the descriptor encoding will fail and thus + * ultimately won't be uploaded. We'll get a stack trace to help us learn + * where the call came from and the tor_cert_create() will log the error. */ + tor_assert_nonfatal(plaintext->signing_key_cert); +} + /* Populate the descriptor encrypted section from the given service object. * This will generate a valid list of introduction points that can be used * after for circuit creation. Return 0 on success else -1 on error. */ @@ -1811,17 +1837,15 @@ build_service_desc_superencrypted(const hs_service_t *service, /* Populate the descriptor plaintext section from the given service object. * The caller must make sure that the keys in the descriptors are valid that - * is are non-zero. Return 0 on success else -1 on error. */ -static int + * is are non-zero. This can't fail. */ +static void build_service_desc_plaintext(const hs_service_t *service, - hs_service_descriptor_t *desc, time_t now) + hs_service_descriptor_t *desc) { - int ret = -1; hs_desc_plaintext_data_t *plaintext; tor_assert(service); tor_assert(desc); - /* XXX: Use a "assert_desc_ok()" ? */ tor_assert(!tor_mem_is_zero((char *) &desc->blinded_kp, sizeof(desc->blinded_kp))); tor_assert(!tor_mem_is_zero((char *) &desc->signing_kp, @@ -1835,24 +1859,13 @@ build_service_desc_plaintext(const hs_service_t *service, plaintext->version = service->config.version; plaintext->lifetime_sec = HS_DESC_DEFAULT_LIFETIME; - plaintext->signing_key_cert = - tor_cert_create(&desc->blinded_kp, CERT_TYPE_SIGNING_HS_DESC, - &desc->signing_kp.pubkey, now, HS_DESC_CERT_LIFETIME, - CERT_FLAG_INCLUDE_SIGNING_KEY); - if (plaintext->signing_key_cert == NULL) { - log_warn(LD_REND, "Unable to create descriptor signing certificate for " - "service %s", - safe_str_client(service->onion_address)); - goto end; - } /* Copy public key material to go in the descriptor. */ ed25519_pubkey_copy(&plaintext->signing_pubkey, &desc->signing_kp.pubkey); ed25519_pubkey_copy(&plaintext->blinded_pubkey, &desc->blinded_kp.pubkey); - /* Success. */ - ret = 0; - end: - return ret; + /* Create the signing key certificate. This will be updated before each + * upload but we create it here so we don't complexify our unit tests. */ + build_desc_signing_key_cert(desc, approx_time()); } /** Compute the descriptor's OPE cipher for encrypting revision counters. */ @@ -1942,8 +1955,7 @@ build_service_desc_keys(const hs_service_t *service, * * This can error if we are unable to create keys or certificate. */ static void -build_service_descriptor(hs_service_t *service, time_t now, - uint64_t time_period_num, +build_service_descriptor(hs_service_t *service, uint64_t time_period_num, hs_service_descriptor_t **desc_out) { char *encoded_desc; @@ -1962,9 +1974,8 @@ build_service_descriptor(hs_service_t *service, time_t now, goto err; } /* Setup plaintext descriptor content. */ - if (build_service_desc_plaintext(service, desc, now) < 0) { - goto err; - } + build_service_desc_plaintext(service, desc); + /* Setup superencrypted descriptor content. */ if (build_service_desc_superencrypted(service, desc) < 0) { goto err; @@ -2037,10 +2048,8 @@ build_descriptors_for_new_service(hs_service_t *service, time_t now) } /* Build descriptors. */ - build_service_descriptor(service, now, current_desc_tp, - &service->desc_current); - build_service_descriptor(service, now, next_desc_tp, - &service->desc_next); + build_service_descriptor(service, current_desc_tp, &service->desc_current); + build_service_descriptor(service, next_desc_tp, &service->desc_next); log_info(LD_REND, "Hidden service %s has just started. Both descriptors " "built. Now scheduled for upload.", safe_str_client(service->onion_address)); @@ -2070,7 +2079,7 @@ build_all_descriptors(time_t now) } if (service->desc_next == NULL) { - build_service_descriptor(service, now, hs_get_next_time_period_num(0), + build_service_descriptor(service, hs_get_next_time_period_num(0), &service->desc_next); log_info(LD_REND, "Hidden service %s next descriptor successfully " "built. Now scheduled for upload.", @@ -2282,12 +2291,9 @@ service_desc_schedule_upload(hs_service_descriptor_t *desc, } } -/* Update the given descriptor from the given service. The possible update - * actions includes: - * - Picking missing intro points if needed. - */ +/* Pick missing intro points for this descriptor if needed. */ static void -update_service_descriptor(hs_service_t *service, +update_service_descriptor_intro_points(hs_service_t *service, hs_service_descriptor_t *desc, time_t now) { unsigned int num_intro_points; @@ -2326,15 +2332,17 @@ update_service_descriptor(hs_service_t *service, } } -/* Update descriptors for each service if needed. */ +/* Update descriptor intro points for each service if needed. We do this as + * part of the periodic event because we need to establish intro point circuits + * before we publish descriptors. */ STATIC void -update_all_descriptors(time_t now) +update_all_descriptors_intro_points(time_t now) { FOR_EACH_SERVICE_BEGIN(service) { /* We'll try to update each descriptor that is if certain conditions apply * in order for the descriptor to be updated. */ FOR_EACH_DESCRIPTOR_BEGIN(service, desc) { - update_service_descriptor(service, desc, now); + update_service_descriptor_intro_points(service, desc, now); } FOR_EACH_DESCRIPTOR_END; } FOR_EACH_SERVICE_END; } @@ -2619,10 +2627,10 @@ run_build_descriptor_event(time_t now) * been rotated or we just started up. */ build_all_descriptors(now); - /* Finally, we'll check if we should update the descriptors. Missing - * introduction points will be picked in this function which is useful for - * newly built descriptors. */ - update_all_descriptors(now); + /* Finally, we'll check if we should update the descriptors' intro + * points. Missing introduction points will be picked in this function which + * is useful for newly built descriptors. */ + update_all_descriptors_intro_points(now); } /* For the given service, launch any intro point circuits that could be @@ -3083,6 +3091,37 @@ should_service_upload_descriptor(const hs_service_t *service, return 0; } +/* Refresh the given service descriptor meaning this will update every mutable + * field that needs to be updated before we upload. + * + * This should ONLY be called before uploading a descriptor. It assumes that + * the descriptor has been built (desc->desc) and that all intro point + * circuits have been established. */ +static void +refresh_service_descriptor(const hs_service_t *service, + hs_service_descriptor_t *desc, time_t now) +{ + /* There are few fields that we consider "mutable" in the descriptor meaning + * we need to update them regurlarly over the lifetime fo the descriptor. + * The rest are set once and should not be modified. + * + * - Signing key certificate. + * - Revision counter. + * - Introduction points which includes many thing. See + * hs_desc_intro_point_t. and the setup_desc_intro_point() function. + */ + + /* Create the signing key certificate. */ + build_desc_signing_key_cert(desc, now); + + /* Build the intro points descriptor section. The refresh step is just + * before we upload so all circuits have been properly established. */ + build_desc_intro_points(service, desc, now); + + /* Set the desc revision counter right before uploading */ + set_descriptor_revision_counter(desc, now, service->desc_current == desc); +} + /* Scheduled event run from the main loop. Try to upload the descriptor for * each service. */ STATIC void @@ -3118,15 +3157,12 @@ run_upload_descriptor_event(time_t now) service->config.num_intro_points, (desc->missing_intro_points) ? " (couldn't pick more)" : ""); - /* At this point, we have to upload the descriptor so start by building - * the intro points descriptor section which we are now sure to be - * accurate because all circuits have been established. */ - build_desc_intro_points(service, desc, now); - - /* Set the desc revision counter right before uploading */ - set_descriptor_revision_counter(desc, approx_time(), - service->desc_current == desc); + /* We are about to upload so we need to do one last step which is to + * update the service's descriptor mutable fields in order to upload a + * coherent descriptor. */ + refresh_service_descriptor(service, desc, now); + /* Proceed with the upload, the descriptor is ready to be encoded. */ upload_descriptor_to_all(service, desc); } FOR_EACH_DESCRIPTOR_END; } FOR_EACH_SERVICE_END; diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index cc0006f1dc..a8a9faaea9 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -99,49 +99,65 @@ typedef struct hs_service_intropoints_t { digestmap_t *failed_id; } hs_service_intropoints_t; -/* Representation of a service descriptor. */ +/* Representation of a service descriptor. + * + * Some elements of the descriptor are mutable whereas others are immutable: + + * Immutable elements are initialized once when the descriptor is built (when + * service descriptors gets rotated). This means that these elements are + * initialized once and then they don't change for the lifetime of the + * descriptor. See build_service_descriptor(). + * + * Mutable elements are initialized when we build the descriptor but they are + * also altered during the lifetime of the descriptor. They could be + * _refreshed_ everytime we upload the descriptor (which happens multiple times + * over the lifetime of the descriptor), or through periodic events. We do this + * for elements like the descriptor revision counter and various + * certificates. See refresh_service_descriptor() and + * update_service_descriptor_intro_points(). + */ typedef struct hs_service_descriptor_t { - /* Decoded descriptor. This object is used for encoding when the service - * publishes the descriptor. */ - hs_descriptor_t *desc; - - /* Client authorization ephemeral keypair. */ + /* Immutable: Client authorization ephemeral keypair. */ curve25519_keypair_t auth_ephemeral_kp; - /* Descriptor cookie used to encrypt the descriptor, when the client - * authorization is enabled */ + /* Immutable: Descriptor cookie used to encrypt the descriptor, when the + * client authorization is enabled */ uint8_t descriptor_cookie[HS_DESC_DESCRIPTOR_COOKIE_LEN]; - /* Descriptor signing keypair. */ + /* Immutable: Descriptor signing keypair. */ ed25519_keypair_t signing_kp; - /* Blinded keypair derived from the master identity public key. */ + /* Immutable: Blinded keypair derived from the master identity public key. */ ed25519_keypair_t blinded_kp; - /* When is the next time when we should upload the descriptor. */ + /* Immutable: The time period number this descriptor has been created for. */ + uint64_t time_period_num; + + /** Immutable: The OPE cipher for encrypting revision counters for this + * descriptor. Tied to the descriptor blinded key. */ + struct crypto_ope_t *ope_cipher; + + /* Mutable: Decoded descriptor. This object is used for encoding when the + * service publishes the descriptor. */ + hs_descriptor_t *desc; + + /* Mutable: When is the next time when we should upload the descriptor. */ time_t next_upload_time; - /* Introduction points assign to this descriptor which contains - * hs_service_intropoints_t object indexed by authentication key (the RSA - * key if the node is legacy). */ + /* Mutable: Introduction points assign to this descriptor which contains + * hs_service_intropoints_t object indexed by authentication key (the RSA key + * if the node is legacy). */ hs_service_intropoints_t intro_points; - /* The time period number this descriptor has been created for. */ - uint64_t time_period_num; - - /* True iff we have missing intro points for this descriptor because we - * couldn't pick any nodes. */ + /* Mutable: True iff we have missing intro points for this descriptor because + * we couldn't pick any nodes. */ unsigned int missing_intro_points : 1; - /** List of the responsible HSDirs (their b64ed identity digest) last time we - * uploaded this descriptor. If the set of responsible HSDirs is different - * from this list, this means we received new dirinfo and we need to - * reupload our descriptor. */ + /** Mutable: List of the responsible HSDirs (their b64ed identity digest) + * last time we uploaded this descriptor. If the set of responsible HSDirs + * is different from this list, this means we received new dirinfo and we + * need to reupload our descriptor. */ smartlist_t *previous_hsdirs; - - /** The OPE cipher for encrypting revision counters for this descriptor. - * Tied to the descriptor blinded key. */ - struct crypto_ope_t *ope_cipher; } hs_service_descriptor_t; /* Service key material. */ @@ -390,7 +406,7 @@ STATIC int intro_point_should_expire(const hs_service_intro_point_t *ip, STATIC void run_housekeeping_event(time_t now); STATIC void rotate_all_descriptors(time_t now); STATIC void build_all_descriptors(time_t now); -STATIC void update_all_descriptors(time_t now); +STATIC void update_all_descriptors_intro_points(time_t now); STATIC void run_upload_descriptor_event(time_t now); STATIC void service_descriptor_free_(hs_service_descriptor_t *desc); diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index bfe50eb3c6..ee2d71aa75 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -1457,7 +1457,7 @@ test_build_update_descriptors(void *arg) /* Time to test the update of those descriptors. At first, we have no node * in the routerlist so this will find NO suitable node for the IPs. */ setup_full_capture_of_logs(LOG_INFO); - update_all_descriptors(now); + update_all_descriptors_intro_points(now); expect_log_msg_containing("Unable to find a suitable node to be an " "introduction point for service"); teardown_capture_of_logs(); @@ -1508,7 +1508,7 @@ test_build_update_descriptors(void *arg) /* We expect to pick only one intro point from the node above. */ setup_full_capture_of_logs(LOG_INFO); - update_all_descriptors(now); + update_all_descriptors_intro_points(now); tor_free(node->ri->onion_curve25519_pkey); /* Avoid memleak. */ tor_free(node->ri->cache_info.signing_key_cert); tor_free(node->ri->onion_pkey); |