Commit 482c4972 authored by David Goulet's avatar David Goulet 🐼 Committed by Nick Mathewson
Browse files

sendme: Clarify how sendme_circuit_cell_is_next() works 



Commit 4ef8470fa5480d3b was actually reverted before because in the end we
needed to do this minus 1 check on the window.

This commit clarifies that in the code, takes the useful comment changes from
4ef8470fa5480d3b and makes sendme_circuit_cell_is_next() private since it
behaves in a very specific way that one external caller might expect.

Part of #30428.

Signed-off-by: David Goulet's avatarDavid Goulet <dgoulet@torproject.org>
parent 3835a3ac

src/core/or/circuit_st.h

+4 −5
Original line number Diff line number Diff line
@@ -115,12 +115,11 @@ struct circuit_t { 
   * list can not contain more than 10 digests of DIGEST_LEN bytes (20).

   *

   * At position i in the list, the digest corresponds to the

   * ((CIRCWINDOW_INCREMENT * i) - 1)-nth cell received since we expect the

   * (CIRCWINDOW_INCREMENT * i)-nth cell to be the SENDME and thus containing

   * the previous cell digest.

   * (CIRCWINDOW_INCREMENT * i)-nth cell received since we expect a SENDME to

   * be received containing that cell digest.

   *

   * For example, position 2 (starting at 0) means that we've received 299

   * cells and the 299th cell digest is kept at index 2.

   * For example, position 2 (starting at 0) means that we've received 300

   * cells so the 300th cell digest is kept at index 2.

   *

   * At maximum, this list contains 200 bytes plus the smartlist overhead. */

  smartlist_t *sendme_last_digests;

src/core/or/sendme.c

+22 −8
Original line number Diff line number Diff line
@@ -312,15 +312,29 @@ record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest) 
 * We are able to know that because the package or deliver window value minus

 * one cell (the possible SENDME cell) should be a multiple of the increment

 * window value. */

bool

sendme_circuit_cell_is_next(int window)

static bool

circuit_sendme_cell_is_next(int window)

{

  /* Is this the last cell before a SENDME? The idea is that if the package or

   * deliver window reaches a multiple of the increment, after this cell, we

   * should expect a SENDME. */

  /* At the start of the window, no SENDME will be expected. */

  if (window == CIRCWINDOW_START) {

    return false;

  }



  /* Are we at the limit of the increment and if not, we don't expect next

   * cell is a SENDME.

   *

   * We test against the window minus 1 because when we are looking if the

   * next cell is a SENDME, the window (either package or deliver) hasn't been

   * decremented just yet so when this is called, we are currently processing

   * the "window - 1" cell.

   *

   * This function is used when recording a cell digest and this is done quite

   * low in the stack when decrypting or encrypting a cell. The window is only

   * updated once the cell is actually put in the outbuf. */

  if (((window - 1) % CIRCWINDOW_INCREMENT) != 0) {

    return false;

  }



  /* Next cell is expected to be a SENDME. */

  return true;

}
@@ -596,7 +610,7 @@ sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath) 
  /* Is this the last cell before a SENDME? The idea is that if the

   * package_window reaches a multiple of the increment, after this cell, we

   * should expect a SENDME. */

  if (!sendme_circuit_cell_is_next(package_window)) {

  if (!circuit_sendme_cell_is_next(package_window)) {

    return;

  }
@@ -621,7 +635,7 @@ sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath) 
  tor_assert(circ);



  /* Only record if the next cell is expected to be a SENDME. */

  if (!sendme_circuit_cell_is_next(cpath ? cpath->deliver_window :

  if (!circuit_sendme_cell_is_next(cpath ? cpath->deliver_window :

                                           circ->deliver_window)) {

    return;

  }
@@ -644,7 +658,7 @@ sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath) 
  tor_assert(circ);



  /* Only record if the next cell is expected to be a SENDME. */

  if (!sendme_circuit_cell_is_next(cpath ? cpath->package_window:

  if (!circuit_sendme_cell_is_next(cpath ? cpath->package_window :

                                           circ->package_window)) {

    goto end;

  }

src/core/or/sendme.h

+0 −3
Original line number Diff line number Diff line
@@ -40,9 +40,6 @@ void sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath); 
void sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath);

void sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath);



/* Circuit level information. */

bool sendme_circuit_cell_is_next(int window);



/* Private section starts. */

#ifdef SENDME_PRIVATE