prop250 - review 1
Merge request reports
Activity
Added 41 commits:
- 5d0656ed...a38dfc7f - 4 commits from branch
master
- 694f1fe8 - write v3-status-votes file earlier in consensus voting
- 5a831229 - Authorities now sort the "package" lines in their votes
- 2fa7a3af - Make advisory-warnings on by default.
- 771ca7c5 - Stop recommending --enable-gcc-warnings in doc/HACKING
- d875101e - Functionify code that writes votes to disk.
- 02383ea7 - Forward-port the 0.2.8.3-alpha changelog
- f2580640 - Bump to 0.2.8.3-alpha-dev
- a93b6bbf - Merge branch 'maint-0.2.8'
- 3934e78b - Make format_changelog.py add links to bugs
- 476714e1 - Merge remote-tracking branch 'arma/bug18840'
- 437cbb17 - Merge remote-tracking branch 'asn/feature19036'
- a0dd8360 - Merge remote-tracking branch 'public/ticket19044'
- e6c15166 - Add tor_htonll/ntohll functions
- 06edc359 - prop250: Add memory and disk state in new files
- 5f315079 - prop250: Add commit and SR values generation code
- 393f52d7 - prop250: Put commits and SRVs in votes/consensus
- d0cc8ebe - prop250: Parse votes and consensus
- 6dcf46b9 - prop250: Initialize the SR subsystem and us it!
- c8c870f2 - prop250: Add unit tests
- 33dbcf24 - prop250: Add changes file
- c3d39e1d - prop250: change time_t to uint64_t
- 08462ac0 - prop250: Use RSA identity digest instead of fingerprint
- ac5b3012 - prop250: Add a valid flag to sr_commit_t
- b455a8c0 - prop250: Add version to Commit line in vote and state
- 447daf9c - prop250: Only trust known authority when computing SRV
- de7f6886 - prop250: Sort smartlist before you get most frequent SRV.
- 7425fb0d - prop250: Improve log messages
- 7260fafc - prop250: Sort commits in lexicographical order in votes
- 770c15ef - prop250: Fix unit tests about the RSA fingerprint check
- 4180cacd - prop250: Pass the dst length to sr_srv_encode()
- fb8feae4 - prop250: Don't reject votes containing commits of unknown dirauths.
- ef9edd8d - prop250: Silence a logging message.
- 59783303 - prop250: Don't use {0} to init static struct -- causes warning on clang.
- fc0fb182 - prop250: Change reveal_num to uint64_t and version to uint32_t
- 0798646a - prop250: Fix format string encoding in log statement
- 43cd82ac - Refactor parameter computation and add a helper function
- f7ceeedc - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit list- 5d0656ed...a38dfc7f - 4 commits from branch
Added 36 commits:
- d9080f5d - Check linking of hardening options, give better warnings if it fails.
- 11d52a44 - Disable GET /tor/bytes.txt and GETINFO dir-usage
- 500c4bf8 - remove an unneeded layer of indentation
- ae4889ac - remove sentence about tor-ops from manpage: #19185
- ab8f1a9e - Do not warn on missing headers
- c4049058 - Fix indentation and quotation of the headers
- 1ce1214d - get rid of one more piece of --enable-instrument-downloads
- 3b83da10 - remove a now-unused section of or.h
- 617b9205 - Merge remote-tracking branch 'public/hardening_flags_must_link'
- 1e5ad156 - Merge remote-tracking branch 'arma/task19035-fixedup'
- 87593702 - roger says this url is better
- fb9975bd - Add tor_htonll/ntohll functions
- 8c622f62 - prop250: Add memory and disk state in new files
- ba9305bc - prop250: Add commit and SR values generation code
- 761c6a20 - prop250: Put commits and SRVs in votes/consensus
- 2a63c13e - prop250: Parse votes and consensus
- ec6aa3db - prop250: Initialize the SR subsystem and us it!
- c2aee71e - prop250: Add unit tests
- 768ad8b2 - prop250: Add changes file
- cd7fcf1b - prop250: change time_t to uint64_t
- 3cca08a0 - prop250: Use RSA identity digest instead of fingerprint
- 40fe1a33 - prop250: Add a valid flag to sr_commit_t
- 3d15f446 - prop250: Add version to Commit line in vote and state
- a511cbdd - prop250: Only trust known authority when computing SRV
- 7327a7b0 - prop250: Sort smartlist before you get most frequent SRV.
- baddfe4f - prop250: Improve log messages
- 59bbc609 - prop250: Sort commits in lexicographical order in votes
- 568c67c2 - prop250: Fix unit tests about the RSA fingerprint check
- 1e8f9f5b - prop250: Pass the dst length to sr_srv_encode()
- d4e63910 - prop250: Don't reject votes containing commits of unknown dirauths.
- 64727034 - prop250: Silence a logging message.
- 7521cb5c - prop250: Don't use {0} to init static struct -- causes warning on clang.
- c00ba6dd - prop250: Change reveal_num to uint64_t and version to uint32_t
- 58db0e50 - prop250: Fix format string encoding in log statement
- ed96b568 - Refactor parameter computation and add a helper function
- 13342477 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit listAdded 4 commits:
- b688be18 - prop250: Change reveal_num to uint64_t and version to uint32_t
- 4b4db3c4 - prop250: Fix format string encoding in log statement
- cfd961c2 - Refactor parameter computation and add a helper function
- 426b5f85 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit listAdded 24 commits:
- 7ea7ff41 - prop250: Add memory and disk state in new files
- c8d4f7fe - prop250: Add commit and SR values generation code
- 1637a647 - prop250: Put commits and SRVs in votes/consensus
- 5effbebc - prop250: Parse votes and consensus
- 76ddc732 - prop250: Initialize the SR subsystem and us it!
- 08dd9f53 - prop250: Add unit tests
- 07347d92 - prop250: Add changes file
- 89a42dfa - prop250: change time_t to uint64_t
- c1d5d260 - prop250: Use RSA identity digest instead of fingerprint
- 042bfd70 - prop250: Add a valid flag to sr_commit_t
- 4937cdee - prop250: Add version to Commit line in vote and state
- 383a5d0b - prop250: Only trust known authority when computing SRV
- 10c2ad91 - prop250: Sort smartlist before you get most frequent SRV.
- 68c63142 - prop250: Improve log messages
- 2cb39791 - prop250: Sort commits in lexicographical order in votes
- 3a409ce1 - prop250: Fix unit tests about the RSA fingerprint check
- e3c7c0ca - prop250: Pass the dst length to sr_srv_encode()
- 3ebf8945 - prop250: Don't reject votes containing commits of unknown dirauths.
- cce0f66e - prop250: Silence a logging message.
- c73e2689 - prop250: Don't use {0} to init static struct -- causes warning on clang.
- 09b6be5a - prop250: Change reveal_num to uint64_t and version to uint32_t
- 59f38813 - prop250: Fix format string encoding in log statement
- 41d6e481 - Refactor parameter computation and add a helper function
- bc678f80 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit listAdded 24 commits:
- 164a720b - prop250: Add memory and disk state in new files
- ec7b8611 - prop250: Add commit and SR values generation code
- d26b0c27 - prop250: Put commits and SRVs in votes/consensus
- b7bef6bb - prop250: Parse votes and consensus
- 8708c5c9 - prop250: Initialize the SR subsystem and us it!
- 5876abb5 - prop250: Add unit tests
- 99afdc44 - prop250: Add changes file
- e0cdceea - prop250: change time_t to uint64_t
- d36f83ce - prop250: Use RSA identity digest instead of fingerprint
- a01ecd4e - prop250: Add a valid flag to sr_commit_t
- 9f04560f - prop250: Add version to Commit line in vote and state
- 820c5bc5 - prop250: Only trust known authority when computing SRV
- 08831b9f - prop250: Sort smartlist before you get most frequent SRV.
- 10b9fee6 - prop250: Improve log messages
- a3d5c9cc - prop250: Sort commits in lexicographical order in votes
- d6ceb85b - prop250: Fix unit tests about the RSA fingerprint check
- 239f0eff - prop250: Pass the dst length to sr_srv_encode()
- c5c430cf - prop250: Don't reject votes containing commits of unknown dirauths.
- 24e06526 - prop250: Silence a logging message.
- 976e7c43 - prop250: Don't use {0} to init static struct -- causes warning on clang.
- 5724059c - prop250: Change reveal_num to uint64_t and version to uint32_t
- 4e9bfdab - prop250: Fix format string encoding in log statement
- 8137e429 - Refactor parameter computation and add a helper function
- d0b20185 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit listAdded 30 commits:
- 57bf8bb2 - remove now-irrelevant XXX020 comments in configure.ac
- ce31db43 - We no longer generate v0 directories. Remove the code to do so
- 4f1a04ff - Replace nearly all XXX0vv comments with smarter ones
- 97f2c1c5 - Wait, we had sprintf() in our unit tests?? FOR SHAME!
- 3cdc8bfa - Let's not even talk about those errors, ok?
- a94e9970 - Add tor_htonll/ntohll functions
- e94db0a7 - prop250: Add memory and disk state in new files
- 17077155 - prop250: Add commit and SR values generation code
- 145ad2dc - prop250: Put commits and SRVs in votes/consensus
- 3ba523c3 - prop250: Parse votes and consensus
- bd42caae - prop250: Initialize the SR subsystem and us it!
- 51e57aee - prop250: Add unit tests
- f375d47e - prop250: Add changes file
- 93232c15 - prop250: change time_t to uint64_t
- 2e283ee2 - prop250: Use RSA identity digest instead of fingerprint
- 15c2310f - prop250: Add a valid flag to sr_commit_t
- 323fffd4 - prop250: Add version to Commit line in vote and state
- 6a44818a - prop250: Only trust known authority when computing SRV
- 7d56587b - prop250: Sort smartlist before you get most frequent SRV.
- 126542bb - prop250: Improve log messages
- deb24c0a - prop250: Sort commits in lexicographical order in votes
- ef79e6e3 - prop250: Fix unit tests about the RSA fingerprint check
- 8f61ddbe - prop250: Pass the dst length to sr_srv_encode()
- 4021b3fa - prop250: Don't reject votes containing commits of unknown dirauths.
- 4e96c5b8 - prop250: Silence a logging message.
- 736f17cb - prop250: Don't use {0} to init static struct -- causes warning on clang.
- 1a65bb3d - prop250: Change reveal_num to uint64_t and version to uint32_t
- 14a0001a - prop250: Fix format string encoding in log statement
- f604166f - Refactor parameter computation and add a helper function
- ca8757f5 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit listAdded 111 commits:
- 0279e484 - Add support for temporarily suppressing a warning
- 0df2c567 - Use ENABLE_GCC_WARNING and DISABLE_GCC_WARNING in tortls.c
- ce1dbbc4 - Enable the -Waggregate-return warning
- 55b5e007 - Add another 22 or so GCC warnings. None currently triggers for me.
- bdc59e33 - Fix a warning on unnamed nodes in node_get_by_nickname().
- 5854b198 - Use tor_sscanf, not sscanf, in test_util.c.
- a32ca313 - Merge branch 'maint-0.2.7' into maint-0.2.8
- b458a81c - Merge branch 'maint-0.2.8'
- ed0ecd9f - Use tor_sscanf, not sscanf, in test_crypto.c
- 6eeedc02 - Use directory_must_use_begindir to predict we'll surely use begindir
- 83513a93 - Check tor_sscanf return value in test_crypto.c
- c19a3d1b - Merge branch 'maint-0.2.8'
- 1e330e19 - Repair test_crypto_openssl_version with LibreSSL
- 36dd9538 - Don't rely on consensus parameter to use a single guard.
- 9eeaeddb - Reduce make check-spaces noise
- c274f825 - Merge remote-tracking branch 'asn/bug17688'
- 4f8086fb - Enable -Wnull-dereference (GCC >=6.1), and fix the easy cases
- b14c1f40 - Merge remote-tracking branch 'public/bug19203_027' into HEAD
- 12517c73 - Add -Wduplicated-cond on GCC 6
- 2ff20c93 - Add -Wunused-const-variable=2 on GCC >=6.1
- 493499a3 - Add -Wfloat-conversion for GCC >= 4.9
- 8f2d2933 - Use -Wdouble-promotion in GCC >= 4.6
- 4caed242 - Enable -Woverlength-strings for GCC>=4.6 on MOST of the code.
- ad16c552 - Use -Wstrict-overflow=2 on gcc5+.
- 9bbd6502 - Use autoconf, not gcc version, to decide which warnings we have
- 15533c88 - Set our autoconf-breaking options last, not before we check for others
- 26e979b3 - Add all the clang-only warnings that do not trigger now
- c3adbf75 - Resolve some warnings from OSX clang.
- 80f1a2cb - Add the -Wextra-semi warning from clang, and fix the cases where it triggers
- 53a3b39d - Add -Wmissing-variable-declarations, with attendant fixes
- e80a032b - Add clang's -Wstring-conversion, and fix the one place it hits
- d6b2af7a - Merge branch 'bug19180_easy_squashed'
- ada5668c - Merge remote-tracking branch 'public/bug19203_027' into maint-0.2.8
- 841d1b25 - Merge branch 'maint-0.2.8'
- cb71c5dd - Whoops -- this got lost in the merge.
- 47edbd4f - Fix build on 32-bit systems.
- 80f2c355 - Remove -Wc11-extensions
- c14c6627 - Update geoip and geoip6 to the June 7 2016 database.
- 0616fd6f - typo/comment/log fixes i found in my sandbox from montreal
- 925f76b4 - Keep make check-spaces happy
- f25f7b75 - Merge branch 'maint-0.2.4' into maint-0.2.5
- b4bb8860 - Merge branch 'maint-0.2.5' into maint-0.2.6
- 80089c9e - Merge branch 'maint-0.2.6' into maint-0.2.7
- 2ee3dbe8 - Merge branch 'maint-0.2.7' into maint-0.2.8
- 6a7d11f3 - Merge branch 'maint-0.2.8'
- 4c90cdc0 - Coverity dislikes (double) (int/int).
- 60b8aaef - lintChanges fixes
- a427a7c4 - Merge branch 'maint-0.2.8'
- 86f0b806 - Bug 19406: OpenSSL changed the Thread API in 1.1.0 again.
- b563a3a0 - Bug 19406: OpenSSL made RSA and DH opaque in 1.1.0.
- 6ddef1f7 - Bug 19406: OpenSSL removed SSL_R_RECORD_TOO_LARGE in 1.1.0.
- c5e2f7b9 - Bug 19406: Fix the unit tests to work with OpenSSL 1.1.x
- b217e4ac - Bug 19406: Add a changes file.
- 71aacbe4 - Suppress the Wredundant-decls warning in another set of openssl headers
- df4fa92a - Merge branch 'maint-0.2.8'
- 3bffdf05 - use new-form macros to disable -Wredundant-decls
- 99a7ddd6 - Disable -Wc99-c11-compat
- e54f8e34 - Remove some duplicated warnings from the big list
- d6b01211 - Resolve the remaining openssl "-Wredundant-decls" warnings.
- 8486dea8 - Merge branch 'maint-0.2.8'
- 227d3b3d - Use ENABLE/DISABLE_GCC_WARNING in masater.
- 05e2750e - whoops; blank line
- 3a0d42fb - bump version to 0.2.8.4-rc
- 657067ef - Merge branch 'maint-0.2.8'
- c7f1b46a - Perform cache lookup when FetchHidServDescriptors is set
- e718a582 - Bump to 0.2.8.4-rc-dev
- 2042080b - ondrej is no longer making rpms
- 4bcec3ba - Merge branch 'maint-0.2.8'
- 327d4c11 - forward-port changelog
- 7b54d7eb - Mark src/common tor_assert(0)/tor_fragile_assert() unreached for coverage
- f986e268 - Reach 100% line coverage on memarea.c
- d1ab295d - add LCOV_EXCL for unreachable exit() blocks in src/common
- 6dc2b605 - Improve coverage on esc_for_log, esc_for_log_len
- f05a213f - Improve coverage on tv_udiff, and tv_mdiff.
- 41cb26c1 - Correct the rounding behavior on tv_mdiff.
- 79370914 - tests for size_mul_check__()
- 9b0bd65f - I believe I found some dead code in our time parsing functions
- 5c596cdb - Tests for message rate-limiting
- 6ceb3797 - Try to fix memarea test on 32-bit systems
- ab35f9de - Correctly close intro circuit when deleting ephemeral HS
- dd737871 - On Windows, tv_sec is long, not time_t.
- 128ab31c - Mark code unreachable in unescape_string()
- c9ea9de8 - Remove parse_config_line_from_str alias
- a4189049 - Coverage on parse_config_line_from_str_verbose.
- a8c76622 - Mark an abort() as unreachable.
- 9a63f059 - Merge remote-tracking branch 'dgoulet/bug18604_029_01'
- e400e135 - Add tor_htonll/ntohll functions
- 8eba1137 - prop250: Add memory and disk state in new files
- a6ff6bc5 - prop250: Add commit and SR values generation code
- 77c2ca3f - prop250: Put commits and SRVs in votes/consensus
- c6ddac8b - prop250: Parse votes and consensus
- 3a8e4674 - prop250: Initialize the SR subsystem and us it!
- bed851a1 - prop250: Add unit tests
- fd75e0c6 - prop250: Add changes file
- a4cb1337 - prop250: change time_t to uint64_t
- c780dc90 - prop250: Use RSA identity digest instead of fingerprint
- 04100c60 - prop250: Add a valid flag to sr_commit_t
- 1f5a0c7f - prop250: Add version to Commit line in vote and state
- 5a394a93 - prop250: Only trust known authority when computing SRV
- 95fa859d - prop250: Sort smartlist before you get most frequent SRV.
- 1ccd5844 - prop250: Improve log messages
- f0964593 - prop250: Sort commits in lexicographical order in votes
- d1c04333 - prop250: Fix unit tests about the RSA fingerprint check
- 564e85dc - prop250: Pass the dst length to sr_srv_encode()
- f9f2e415 - prop250: Don't reject votes containing commits of unknown dirauths.
- 9c715ae8 - prop250: Silence a logging message.
- 9441a834 - prop250: Don't use {0} to init static struct -- causes warning on clang.
- 67f9c643 - prop250: Change reveal_num to uint64_t and version to uint32_t
- 8eb1da18 - prop250: Fix format string encoding in log statement
- 3371dd3b - Refactor parameter computation and add a helper function
- 81652726 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit listAdded 54 commits:
- 3cc37445 - Add several test scripts wrapping test_workqueue
- 429d15c5 - Mark the unreachable lines in compat_{,p}threads and workqueue
- f016213f - Unit tests for our zlib code to test and reject compression bombs.
- 94762e37 - Use the Autoconf macro AC_USE_SYSTEM_EXTENSIONS
- 4e4a7d2b - Fix base32 API to take any source length in bytes
- f4f9a9be - test: Add base32_encode/decode unit tests
- 48b25e68 - Merge branch 'bug18280_029_03_nm_squashed'
- 033cf30b - Keep make check-spaces happy
- 2c96d95c - Fix spelling of --enable-tor2web-mode in manpage
- b421648d - Merge remote-tracking branch 'public/thread_coverage'
- 80801531 - Remove support for zlib <= 1.1
- 358fc026 - Remove a ridiculous realloc call from torgzip.c
- 5a725dab - Mark some torgzip lines as unreachable/untestable.
- d937b866 - Unindent block
- 81cfd5c9 - Merge branch 'zlib_coverage_squashed'
- 58e6a6aa - Fix #19063: Add check in utility macro
- 1160ac12 - Changes file for 19063; use the BUG macro
- 568dc27a - Make base16_decodes return number of decoded bytes
- 6cedd493 - Merge branch 'bug14013_029_01_squashed'
- 85edef27 - test: Increase offset to rendcache descriptor time
- ba88d781 - Fix unit test crash on 32-bit.
- c1f0ec30 - Merge remote-tracking branch 'dgoulet/bug19465_029_01'
- 2b74e13a - More coverage in backtrace.c
- ba28da8d - compat.c coverage: simplify under-tested alloc_getcwd.
- 603cb712 - Small coverage improvements on compat.c
- 5fbd1959 - Coverage hack for test_switch_id.sh
- 2f75b34d - Patch from dgoulet: fix a base16 problem that manifested w stem
- 400b4250 - mark sanitize_blacklist.txt as obsolete
- 49e8f475 - util: zero target buffer of base*_encode/decode
- 9744a40f - Add tor_htonll/ntohll functions
- b3b4ffce - prop250: Add memory and disk state in new files
- f0711641 - prop250: Add commit and SR values generation code
- b1397015 - prop250: Put commits and SRVs in votes/consensus
- 9cce7fbe - prop250: Parse votes and consensus
- d6f33429 - prop250: Initialize the SR subsystem and us it!
- 94d4a086 - prop250: Add unit tests
- beedf362 - prop250: Add changes file
- ee5398a1 - prop250: change time_t to uint64_t
- ac6ad8ba - prop250: Use RSA identity digest instead of fingerprint
- 55795965 - prop250: Add a valid flag to sr_commit_t
- c14f77c4 - prop250: Add version to Commit line in vote and state
- 75b5710d - prop250: Only trust known authority when computing SRV
- 5f455974 - prop250: Sort smartlist before you get most frequent SRV.
- 2a7dafe9 - prop250: Improve log messages
- 1b4d7f63 - prop250: Sort commits in lexicographical order in votes
- aff5b810 - prop250: Fix unit tests about the RSA fingerprint check
- 267b065b - prop250: Pass the dst length to sr_srv_encode()
- 371a62bb - prop250: Don't reject votes containing commits of unknown dirauths.
- 14fe6808 - prop250: Silence a logging message.
- e3797451 - prop250: Don't use {0} to init static struct -- causes warning on clang.
- 93ec0608 - prop250: Change reveal_num to uint64_t and version to uint32_t
- ad5a9be9 - prop250: Fix format string encoding in log statement
- bddbbb8b - Refactor parameter computation and add a helper function
- 483a8126 - prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements
Toggle commit list- src/or/shared_random_state.c 0 → 100644
892 { 893 state_query(SR_STATE_ACTION_PUT, SR_STATE_OBJ_VALID_AFTER, 894 (void *) &valid_after, NULL); 895 } 896 897 /* Return the phase we are currently in according to our state. */ 898 sr_phase_t 899 sr_state_get_phase(void) 900 { 901 void *ptr; 902 state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_PHASE, NULL, &ptr); 903 return *(sr_phase_t *) ptr; 904 } 905 906 /* Return the previous SRV value from our state. Value CAN be NULL. */ 907 sr_srv_t * I wanted this to return const, because if it doesn't, we are totally violating the supposed guarantee that these structures can only change. When you return a mutable view of the interior of a structure, people can change it out from under you.
I guess we can give this a pass for now (since the code depends on it) but it is totally an abstraction violation and we should fix it some time.
Commit
6a29928
.I'm not entirely sure we had in mind guarantees of mutability for those structures because for instance we fetch a saved commit from our state during the reveal phase and we update it with the reveal value. So we kind of need a "get_commit()" that doesn't return a
const
unless we had a specific function that only updates a field in the commit object through the state API.Anyway, the get previous/current SRV are now const and a delete function has been added.
1048 1052 char *value, digest[DIGEST_LEN]; 1049 1053 digest_algorithm_t alg; 1050 1054 const char *rsa_identity_fpr; 1051 1055 sr_commit_t *commit = NULL; 1052 1056 1053 if (smartlist_len(args) < 3) { 1057 if (smartlist_len(args) < 4) { 1054 1058 goto error; 1055 1059 } 1056 1060 1057 /* First argument is the algorithm. */ 1061 /* First is the version number of the SR protocol which indicates at which 1062 * version that commit was created. */ 1058 1063 value = smartlist_get(args, 0); 1064 version = (uint32_t) tor_parse_ulong(value, 10, 1, UINT32_MAX, NULL, NULL); 1065 if (version < SR_PROTO_VERSION) { 1108 1118 escaped(rsa_identity_fpr)); 1109 1119 goto error; 1110 1120 } 1111 /* Let's make sure, for extra safety, that this fingerprint is known to 1112 * us. Even though this comes from a vote, doesn't hurt to be 1113 * extracareful. */ 1114 if (trusteddirserver_get_by_v3_auth_digest(digest) == NULL) { 1115 log_warn(LD_DIR, "SR: Fingerprint %s is not from a recognized " 1116 "authority. Discarding commit.", 1117 escaped(rsa_identity_fpr)); 1118 goto error; David, based on Nick's comment and for the purposes of forwards-compatibility, it might make sense to reconsider whether we want to
goto error
in cases like the following:if (version < SR_PROTO_VERSION) { log_info(LD_DIR, "SR: Commit version %" PRIu32 " (%s) is not supported.", version, escaped(value)); goto error; }
if (alg != SR_DIGEST_ALG) { log_warn(LD_BUG, "SR: Commit algorithm %s is not recognized.", escaped(value)); goto error; }
if (strlen(encoded) > SR_REVEAL_BASE64_LEN) { /* This means that if we base64 decode successfully the received reveal * value, we'll end up with a bigger decoded value thus unusable. */ goto error; }
Indeed, in this case we have to
goto error
because it's the parsing commit function which also parses the state from disk. Ultimately, when handling the votes, this is called byextract_shared_random_commits()
. So if we want forward compat, being unable to parse the commit from the vote would be exactly like ignoring an unknown line thus we shouldn't reject the vote. This below shouldn'tgoto err
then:if (extract_shared_random_commits(ns, tokens) < 0) { log_warn(LD_DIR, "SR: Unable to parse commits in vote."); goto err; }
... and probably this also:
if (extract_shared_random_srvs(ns, tokens) < 0) { log_warn(LD_DIR, "SR: Unable to parse SRV(s)"); goto err; }
Furthermore, we shouldn't restrict the "extract commit function" to a hardcoded number of arguments in
extract_shared_random_commits()
.if (tok->n_args < 4 || tok->n_args > 5) { goto err; }
What we want is to extract all tokens, put them in a list and send it to the parse commit function which will decide if it's a good line or not. We've added the version number as the very first arg to the commit so having this hard check on number of args. defies this forward compat purpose with a version in front!
I'll cook a patch to address this, I think it's going to be much better and cleaner. I'll let you know which fixup commit ID to look at.
418 418 /* Add the invariant token. */ 419 419 memcpy(msg, SR_SRV_TOKEN, SR_SRV_TOKEN_LEN); 420 420 offset += SR_SRV_TOKEN_LEN; 421 set_uint8(msg + offset, reveal_num); 421 set_uint64(msg + offset, reveal_num); 422 422 offset += 1; Oh crap... GREAT catch with the offset... a value bigger than
uint8_t
would have be super problematic... Fortunately,reveal_num
here is never above 9 (nine dirauth) and the version is1
This is for the construction of the SRV which looks like this so no we don't need htonll() since it's not going on the wire:
* SRV = SHA3-256("shared-random" | INT_8(reveal_num) | * INT_4(version) | HASHED_REVEALS | previous_SRV)
Fixed in
b3a4d6e
418 418 /* Add the invariant token. */ 419 419 memcpy(msg, SR_SRV_TOKEN, SR_SRV_TOKEN_LEN); 420 420 offset += SR_SRV_TOKEN_LEN; 421 set_uint8(msg + offset, reveal_num); 421 set_uint64(msg + offset, reveal_num); 422 422 offset += 1; 423 set_uint8(msg + offset, SR_PROTO_VERSION); 423 set_uint32(msg + offset, SR_PROTO_VERSION); 424 424 offset += 1; Added 6 commits:
- 6a29928f - prop250: Add a DEL state action and return const SRVs
- aaf6bacd - fixup! prop250: Add a valid flag to sr_commit_t
- 017d21fb - fixup! prop250: Add version to Commit line in vote and state
- 4bb7d843 - fixup! prop250: Add commit and SR values generation code
- b3a4d6e8 - fixup! prop250: Change reveal_num to uint64_t and version to uint32_t
- a1d5378b - fixup! prop250: Add unit tests
Toggle commit list49 49 (((DIGEST256_LEN - 1) / 3) * 4 + 4) 50 50 51 51 /* Assert if commit valid flag is not set. */ 52 #define ASSERT_COMMIT_VALID(c) tor_assert(c->valid) 52 #define ASSERT_COMMIT_VALID(c) (tor_assert(c->valid)) 783 783 const sr_srv_t *current_srv = NULL; 784 784 785 785 #define SRV_TEST_VECTOR \ 786 "C3E2E6F48E541F8CB47DAC097050D09ED0AFE0CAC52AF129FCD3D20166B70463" 786 "F3946A6AED72BA529527A52D65C9E8A2C2E34C2E59974F3BD4F9DC409ACC5905" Ehm, currently this test indeed assumes that whatever the code does now must be correct... Not good, I agree.
When we started the project, I implemented the SRV calculation in Python for this test, and that's how the test vector was initially produced. However when we switched from SHA2 to SHA3 I did not update the Python version. It can still be found here: https://gitweb.torproject.org/user/asn/tor.git/tree/src/test/sr_srv_calc_ref.py?h=prop250_final_v4 and it might be worth revising.
I can try to do this at some point next week if you guys think it's worthwhile.
Added 1 commit:
- 14b027c8 - fixup! prop250: Parse votes and consensus