Merge remote-tracking branch 'dgoulet/bug20991_030_01'

5 files changed, 20 insertions, 15 deletions

diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c
index bfc7ec3876..b37c9a1b9f 100644
--- a/src/or/hs_intropoint.c
+++ b/src/or/hs_intropoint.c
@@ -70,7 +70,11 @@ verify_establish_intro_cell(const hs_cell_establish_intro_t *cell,
     ed25519_signature_t sig_struct;
     const uint8_t *sig_array = hs_cell_establish_intro_getconstarray_sig(cell);
 
-    if (hs_cell_establish_intro_getlen_sig(cell) != sizeof(sig_struct.sig)) {
+    /* Make sure the signature length is of the right size. For EXTRA safety,
+     * we check both the size of the array and the length which must be the
+     * same. Safety first!*/
+    if (hs_cell_establish_intro_getlen_sig(cell) != sizeof(sig_struct.sig) ||
+        hs_cell_establish_intro_get_sig_len(cell) != sizeof(sig_struct.sig)) {
       log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
              "ESTABLISH_INTRO sig len is invalid");
       return -1;
@@ -83,7 +87,7 @@ verify_establish_intro_cell(const hs_cell_establish_intro_t *cell,
 
     const size_t sig_msg_len = cell->end_sig_fields - msg;
     int sig_mismatch = ed25519_checksig_prefixed(&sig_struct,
-                                                 (uint8_t*) msg, sig_msg_len,
+                                                 msg, sig_msg_len,
                                                  ESTABLISH_INTRO_SIG_PREFIX,
                                                  &auth_key);
     if (sig_mismatch) {
diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index 3b5a3e7853..7b12b63065 100644
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -152,8 +152,9 @@ generate_establish_intro_cell(const uint8_t *circuit_key_material,
     tor_assert(encoded_len > ED25519_SIG_LEN);
 
     if (ed25519_sign_prefixed(&sig,
-                              (uint8_t*) cell_bytes_tmp,
-                              encoded_len - ED25519_SIG_LEN,
+                              cell_bytes_tmp,
+                              encoded_len -
+                                (ED25519_SIG_LEN + sizeof(cell->sig_len)),
                               ESTABLISH_INTRO_SIG_PREFIX,
                               &key_struct)) {
       log_warn(LD_BUG, "Unable to gen signature for ESTABLISH_INTRO cell.");
diff --git a/src/trunnel/hs/cell_establish_intro.c b/src/trunnel/hs/cell_establish_intro.c
index 02094fe2bf..633bd7c214 100644
--- a/src/trunnel/hs/cell_establish_intro.c
+++ b/src/trunnel/hs/cell_establish_intro.c
@@ -231,6 +231,11 @@ hs_cell_establish_intro_getconstarray_handshake_mac(const hs_cell_establish_intr
 {
   return (const uint8_t  *)hs_cell_establish_intro_getarray_handshake_mac((hs_cell_establish_intro_t*)inp);
 }
+const uint8_t *
+hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp)
+{
+  return inp->end_sig_fields;
+}
 uint16_t
 hs_cell_establish_intro_get_sig_len(const hs_cell_establish_intro_t *inp)
 {
@@ -242,11 +247,6 @@ hs_cell_establish_intro_set_sig_len(hs_cell_establish_intro_t *inp, uint16_t val
   inp->sig_len = val;
   return 0;
 }
-const uint8_t *
-hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp)
-{
-  return inp->end_sig_fields;
-}
 size_t
 hs_cell_establish_intro_getlen_sig(const hs_cell_establish_intro_t *inp)
 {
@@ -518,12 +518,12 @@ hs_cell_establish_intro_parse_into(hs_cell_establish_intro_t *obj, const uint8_t
   CHECK_REMAINING(TRUNNEL_SHA3_256_LEN, truncated);
   memcpy(obj->handshake_mac, ptr, TRUNNEL_SHA3_256_LEN);
   remaining -= TRUNNEL_SHA3_256_LEN; ptr += TRUNNEL_SHA3_256_LEN;
+  obj->end_sig_fields = ptr;
 
   /* Parse u16 sig_len */
   CHECK_REMAINING(2, truncated);
   obj->sig_len = trunnel_ntohs(trunnel_get_uint16(ptr));
   remaining -= 2; ptr += 2;
-  obj->end_sig_fields = ptr;
 
   /* Parse u8 sig[sig_len] */
   CHECK_REMAINING(obj->sig_len, truncated);
diff --git a/src/trunnel/hs/cell_establish_intro.h b/src/trunnel/hs/cell_establish_intro.h
index a9166f10d8..725d47cd85 100644
--- a/src/trunnel/hs/cell_establish_intro.h
+++ b/src/trunnel/hs/cell_establish_intro.h
@@ -19,8 +19,8 @@ struct hs_cell_establish_intro_st {
   struct cell_extension_st *extensions;
   const uint8_t *end_mac_fields;
   uint8_t handshake_mac[TRUNNEL_SHA3_256_LEN];
-  uint16_t sig_len;
   const uint8_t *end_sig_fields;
+  uint16_t sig_len;
   TRUNNEL_DYNARRAY_HEAD(, uint8_t) sig;
   uint8_t trunnel_error_code_;
 };
@@ -170,6 +170,9 @@ uint8_t * hs_cell_establish_intro_getarray_handshake_mac(hs_cell_establish_intro
  * a const pointer
  */
 const uint8_t  * hs_cell_establish_intro_getconstarray_handshake_mac(const hs_cell_establish_intro_t *inp);
+/** Return the position for end_sig_fields when we parsed this object
+ */
+const uint8_t * hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp);
 /** Return the value of the sig_len field of the
  * hs_cell_establish_intro_t in 'inp'
  */
@@ -179,9 +182,6 @@ uint16_t hs_cell_establish_intro_get_sig_len(const hs_cell_establish_intro_t *in
  * return -1 and set the error code on 'inp' on failure.
  */
 int hs_cell_establish_intro_set_sig_len(hs_cell_establish_intro_t *inp, uint16_t val);
-/** Return the position for end_sig_fields when we parsed this object
- */
-const uint8_t * hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp);
 /** Return the length of the dynamic array holding the sig field of
  * the hs_cell_establish_intro_t in 'inp'.
  */
diff --git a/src/trunnel/hs/cell_establish_intro.trunnel b/src/trunnel/hs/cell_establish_intro.trunnel
index 4f9e8f7e08..33a133bf67 100644
--- a/src/trunnel/hs/cell_establish_intro.trunnel
+++ b/src/trunnel/hs/cell_establish_intro.trunnel
@@ -26,9 +26,9 @@ struct hs_cell_establish_intro {
   u8 handshake_mac[TRUNNEL_SHA3_256_LEN];
 
   /* Signature */
-  u16 sig_len;
   /* Indicate the end of the handshake authentication data. */
   @ptr end_sig_fields;
+  u16 sig_len;
   u8 sig[sig_len];
 };