adds functional test coverage for SAFECOOKIE authentication, including
successful authentication, AUTHCHALLENGE failure, an invalid server
nonce, a mismatched server hash, an invalid-length cookie file, and a
missing cookie file
while working on #35292, i realised we did not have coverage for
SAFECOOKIE authentication either, so i added it here
Test successful authentication and failures caused by an unsuccessful
`AUTHCHALLENGE`, an invalid server nonce or hash, and a missing or
invalid-length cookie file.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
HASHEDPASSWORD authentication by winterrdog)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
| Scope | Metric | Before (merge-base at cf80493e2367786ce9946c522eb8313a52c90557) | After (PR HEAD) |
Change |
|---|---|---|---|---|
src/torcontrol\.cpp$ |
Lines | 71.7% (281/392) | 81.9% (321/392) | +10.2 pp (+40 lines) |
src/torcontrol\.cpp$ |
Functions | 88.9% (24/27) | 96.3% (26/27) | +7.4 pp (+2 funcs) |
src/torcontrol\.cpp$ |
Branches | 39.7% (265/668) | 47.9% (320/668) | +8.2 pp (+55 branches) |
to get these results, i only ran feature_torcontrol.py before and after the
change on the same build, using GCC's coverage instrumentation and lcov
2.0
<details><summary>bash script used to get test coverage locally</summary>
# 1. compile once with coverage baked in
cmake -B build -DCMAKE_BUILD_TYPE=Coverage
cmake --build build -j$(nproc)
# 2. BEFORE: run the merge-base version of the test file
git checkout $(git merge-base HEAD master) -- test/functional/feature_torcontrol.py
find build -name '*.gcda' -delete
python3 test/functional/feature_torcontrol.py --configfile=build/test/config.ini
lcov --capture --directory build --gcov-tool gcov-$(gcc -dumpversion | cut -d. -f1) \
--rc branch_coverage=1 --ignore-errors inconsistent,mismatch,unused,negative \
--output-file base.info
# 3. AFTER: run the PR version of the test file
git checkout HEAD -- test/functional/feature_torcontrol.py
find build -name '*.gcda' -delete
python3 test/functional/feature_torcontrol.py --configfile=build/test/config.ini
lcov --capture --directory build --gcov-tool gcov-$(gcc -dumpversion | cut -d. -f1) \
--rc branch_coverage=1 --ignore-errors inconsistent,mismatch,unused,negative \
--output-file after.info
# 4. keep only the file under test, then build the HTML report
lcov --extract base.info '*/src/torcontrol.cpp' --rc branch_coverage=1 -o base_tor.info
lcov --extract after.info '*/src/torcontrol.cpp' --rc branch_coverage=1 -o after_tor.info
genhtml after_tor.info --baseline-file base_tor.info --branch-coverage -o cov_diff
## DONE...
# opens the system's default browser on linux. on other systems, just open the
# differential coverage report (./cov_diff/index.html) in your browser of choice
xdg-open cov_diff/index.html
</details>
ACK 6827006f4f0d144248244731e75191bf203e1dd6
Concept ACK 6827006f4f
I introduced a few changes in torcontrol.cpp skipping the ServerHash comparison, dropping the ServerNonce length check, using the wrong key for the client hash and accepting any cookie length, all of this pass on master and fail on this branch, so the new tests catch them.