We want to forbid this pattern since, unlike the other log_*()
macros, log_debug() conditionally evaluates its arguments only if
debug-level logging is enabled. Thus, a call to
log_debug("%d", x++);
will only increment x if debugging logs are enabled, which is
probably not what the programmer intended.
One bug caused by this pattern was #30628.
This script detects log_debug( ) calls with any of E++, E--, ++E,
or --E in their arguments, where E is an arbitrary expression.
Closes ticket 30743.
The purpose of tracking whether an exception is used is so that we
can tell whether it is overbroad. This means that an _underbroad_
exception is still a used one. Fixes bug 31338.
Note that this header file behaves a bit strangely. It is used by
coccinelle just for the purpose of knowing how to parse
difficult-to-parse stuff. It doesn't need to produce good C -- just
grammatical C.
spatch can let us know whether a file has parsed "perfectly" or
not. The more perfect it parses, the likelier any semantic patches
are to apply. I've used this script to identify problem areas in
our code.
It's a bit tricky to remember the right incantation to get the
proper include paths and incantations for coccinelle, but without
it, coccinelle is less effective at parsing our C.
I was expecting our filter code to work in a way it didn't. I
thought that saying that DependencyViolation applied to "*" would
hit all of the files -- but actually, "*" wasn't implemented. I had
to say "*.c" and "*.h"
* Move the shellcheck script from the Makefile to its own script file
* Reformat the shellcheck script so it's easier to read and modify
* Call the shellcheck script from the pre-commit hook
Fixes bug 30967; not in any released version of Tor.
Most shell arguments should be quoted to avoid mistakes.
But since all branch names are hard-coded, or supplied by the script user,
we don't need to be too concerned about command injection.
Quoting all shell arguments would take a major refactor.
(Probably using arrays.)
Part of 31314.
Skip test branches that are the same as remote maint/release/master
branches.
Add a TOR_PUSH_SAME and -s argument to git-push-all.sh to change this
default.
Part of 31314.
TOR_GIT_PUSH provides the git push command and default arguments.
Also fix handling of git-push-all.sh script arguments and arguments that
are passed through to $TOR_GIT_PUSH, using a "--" argument as a separator.
Fix on 29879.
Add a -t <test-branch-prefix> argument to git-merge-forward.sh and
git-push-all.sh, which makes these scripts create, merge forward, and
push test branches.
Add a -r <remote-name> argument to git-push-all.sh, so the script can
push test branches to a personal remote.
Closes ticket 31314.
(I've chosen to suppress some instances rather than 'fix' them,
since the fix would require arrays or major refactoring.)
Fixes bug 31519; bug not in any released Tor.
The pre-push hook already calls the pre-commit hook, which calls
practracker.
Also update the script comments to avoid similar issues in future.
Fixes bug 31462; bugfix on 0.4.1.1-alpha.
Add a TOR_PUSH_DELAY variable to git-push-all.sh, which makes the script
push master and maint branches with a delay between each branch. These
delays trigger the CI jobs in a set order, which should show the most
likely failures first.
Also:
* make pushes atomic by default, and
* make the script pass any command-line arguments to git push.
Closes ticket 29879.
A .may_includes file can be "advisory", which means that some
violations of the rules are expected. We will track these
violations with practracker, not as automatic errors.
Our topological sort code really deserves a function of its own.
Additionally, don't print from inside the topological sort code:
instead, return a result, and let the caller print it.
I'll want to make this block into a series of functions in a
subsequent commit, but I'm doing this separately to get the
indentation change out of the way.
This branch will end up with making checkIncludes.py an integrated
part of practracker, for ticket 31176.
Padding circuits were regular cells that got closed before their padding
machine could finish. This means that they can still receive regular cells from
their past life, but they have no way or reason to answer them anymore. Hence
let's ignore them before they even get to the proper subsystems.
I'm using 500 as a file size limit, and 15 as an include limit.
This affects comparatively few files, but I think they are the worst
ones.
Closes ticket 31175.
This test runs practracker with a set of 0 thresholds, to make sure
that it enumerates all its values right. It tries running with an
empty exceptions file, and with an exceptions file that covers
_some_ of the data, and it makes sure that the outputs are as expected.
Now that there is only one toplevel place where we print problems,
we can redirect just that one print to a file when we are
regenerating the exceptions.txt file. Previously we redirected
sys.stdout, which is naughty, and forced us to send warnings (and
warnings alone) to stderr.
These flags let you suppress the message about the number of
problems and warnings, and let you control the thresholds above
which something counts as a problem.
I need this for testing.
Instead of having "consider" functions that have to call a global
ProblemVault, we can now generate all the metrics for the code
separately from the decision about what to do for them.
I'm about to refactor the code into a set of iterators that yield
*all* the metrics for the code, and then add a filter on top of that
to return the problems.
Previously, when TestingTorNetwork was set, we would manually adjust
the initvalue members of a bunch of other config_var_t, and then
re-run the early parts or parsing the options.
Now we treat the initvalue fields as immutable, but instead assign
to them in options_init(), as early as possible. Rather than
re-running the early parts of options, we just re-call the
options_init_from_string() function.
This patch de-kludges some of our code pretty handily. I think it
could later handle authorities and fallbacks, but for now I think we
should leave those alone.
When an exception is present, we can now violate the limit by a little
bit and only produce a warning. The strict flag overrides this
behavior.
I've given file sizes a 2% tolerances and function sizes/include
counts a 10% tolerance.
Part of 30752
Instead of excluding directories at the last minute if they happen
to appear in our filenames, we exclude them early, before recursing
into all their subdirectories.
Part of 29746.
The practracker_tests.py unit test file called a function by its old
name.
Also, practracker counted functions as starting one line after the
function name, and ending with the closing brace. Now they start
with the open brace and end with the closing brace.
Coverity doesn't understand that if begin_cell_parse() returns 0 and
sets is_begindir to 0, its address field will always be set.
Fixes bug 30126; bugfix on 0.2.4.7-alpha; Fixes CID 1447296.
Right now, this has been done at a high level by confparse.c, but it
makes more sense to lower it.
This API is radically un-typesafe as it stands; we'll be wrapping it
in a safer API as we do #30914 and lower the struct manipulation
code as well.
Closes ticket 30864.
Set the env vars:
* TOR_MASTER_NAME to override the tor master branch name, and
* TOR_WKT_NAME to override the worktree path
Fixes bug 30841; bugfix on 0.4.0.1-alpha.
Proposal 289 prevents SENDME-flooding by requiring the other side to
authenticate the data it has received. But this data won't actually
be random if they are downloading a known resource. "No problem",
we said, "let's fell the empty parts of our cells with some
randomness!" and we did that in #26871.
Unfortunately, if the relay data payloads are all completely full,
there won't be any empty parts for us to randomize.
Therefore, we now pick random "randomness windows" between
CIRCWINDOW_INCREMENT/2 and CIRCWINDOW_INCREMENT. We remember whether we have
sent a cell containing at least 16 bytes of randomness in that window. If we
haven't, then when the window is exhausted, we send one. (This window approach
is designed to lower the number of rng checks we have to do. The number 16 is
pulled out of a hat to change the attacker's guessing difficulty to
"impossible".)
Implements 28646.
We added this facility so that we could get deterministic PRNG
behavior for coverage testing on tests that use a replaced PRNG.
We need to have our coverage determinism tool test for this as well.
Turns out that we were only recording the "b_digest" but to have
bidirectionnal authenticated SENDMEs, we need to use the "f_digest" in the
forward cell situation.
Because of the cpath refactoring, this commit plays with the crypt_path_ and
relay_crypto_t API a little bit in order to respect the abstractions.
Previously, we would record the cell digest as the SENDME digest in the
decrypt cell function but to avoid code duplication (both directions needs to
record), we now do that right after iff the cell is recognized (at the edge).
It is now done in circuit_receive_relay_cell() instead.
We now also record the cell digest as the SENDME digest in both relay cell
encryption functions since they are split depending on the direction.
relay_encrypt_cell_outbound() and relay_encrypt_cell_inbound() need to
consider recording the cell digest depending on their direction (f vs b
digest).
Fixes#30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
It turns out that only the exit side is validating the authenticated SENDME v1
logic and never the client side. Which means that if a client ever uploaded
data towards an exit, the authenticated SENDME logic wouldn't apply.
For this to work, we have to record the cell digest client side as well which
introduced a new function that supports both type of edges.
This also removes a test that is not valid anymore which was that we didn't
allow cell recording on an origin circuit (client).
Part of #30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because github PRs choose the most recent origin/master at the time of the PR
(and for any fixups pushed to a PR later to send to CI), there are tons of
conflicts and unexpected practracker issues.
This means CI can suddenly fail after fixups to a branch that pass locally.
Then CI fails and we have to close and re-open the PR.
Split the core reply formatting code out of control_fmt.c into
control_proto.c. The remaining code in control_format.c deals with
specific subsystems and will eventually move to join those subsystems.
(This should be all of the command that work nicely with positional
arguments only.)
Some of these commands should probably treat extra arguments as
incorrect, but for now I'm trying to be careful not to break
any existing users.
The first line break in particular was mishandled: it was discarded
if no arguments came before it, which made it impossible to
distinguish arguments from the first line of the body.
To solve this, we need to allocate a copy of the command rather than
using NUL to separate it, since we might have "COMMAND\n" as our input.
Fixes ticket 29984.
