| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
CWE-573: Missing varargs init or cleanup (VARARGS)
missing_va_end: va_end was not called for ap.
|
|
CWE-569: Wrong sizeof argument (SIZEOF_MISMATCH)
suspicious_sizeof: Passing argument '8UL /* sizeof (char *) */ * *j'
to function malloc and then casting the return value to 'char *' is
suspicious.
|
|
|
|
|
|
|
|
|
|
Presently we have CodeQL and Coverity Scan. CodeQL is run on every
push, but Coverity is only run on a schedule since it’s aggressively
rate limited, the results are hidden in their private webapp, and it
doesn’t give us very high value feedback.
|
|
This test is run only with GCC, as on Travis.
Use the ‘setup-perl’ action (shogo82148/actions-setup-perl@v1) to run
the distcheck build with Perl 5.14, thus consolidating the distcheck
build with the “oldest supported perl” build that we had on Travis.
Read the set of critic-related CPAN distributions that are expected to
be installed from .perlcriticrc and install them with cpanm. (Some
but not all of them are available via apt, but this way we can pin
exact versions.) Download CPAN packages over HTTPS; this might mean
we’re hitting one overloaded server on someone’s desk somewhere, but
CPAN packages don’t necessarily have any built-in cryptographic
integrity verification, and even when they do you have to manually set
up PGP keys for cpanm --verify to work. Transport security is at
least *something*.
|
|
This tests four configurations: (GCC, Clang) x (Valgrind, ASan+UBSan),
all with everything enabled.
|
|
Use the ‘cache’ action to save all the files created by autogen.sh,
keyed on a hash of the autotools source files, plus the version
numbers of autoconf, automake, libtool, and pkg-config. This shaves a
few seconds off all our builds, and since we’re about to go from ten
builds to more than 50, it’s worth it.
Move log dumping on test failure to an independent script that
captures config.log as well as all testsuite logs no matter where they
are found. Use “action commands” to make nice foldable sections for
each. Also add foldable sections to the output of
ci-log-dependency-versions.
|
|
ASan’s “interceptors” for crypt and crypt_r have a semantic conflict
with libxcrypt, requiring a few tests to be disabled for builds with
-fsanitize-address. See commentary in test/crypt-badargs.c for an
explanation of the conflict, and commentary in build-aux/zw_detect_asan.m4
for why a configure test is required.
This does not work around https://github.com/google/sanitizers/issues/1365
which is an unrelated problem affecting test/special-char-salt.c and
which doesn’t currently break CI.
|
|
codecov-action@v1 is deprecated due to its inextricable coupling with
Codecov’s shell-script uploader, which is also deprecated, due to its
being an unmaintainable pile of shell script. See
https://github.com/codecov/codecov-action/blob/635d4e88ad8c1f75b56277efd9ec186c3359727f/README.md#%EF%B8%8F--deprecration-of-v1 [sic]
and https://about.codecov.io/blog/introducing-codecovs-new-uploader/
for gory details.
The new uploader does not support the ‘gcov_executable‘ and
‘gcov_path_exclude‘ parameters yet, so for the time being we go back
to preprocessing the coverage data with lcov (as we were doing on
Travis).
|
|
The test spec was only correct when both or neither of bigcrypt and
descrypt were enabled. This sort of thing is why we need the giant CI
configuration matrix we used to have on Travis.
|
|
None of the matrix configure options were getting passed to configure
because the intermediate environment variable name was wrong.
This may explain some of the coverage loss relative to Travis [
https://app.codecov.io/gh/besser82/libxcrypt/compare/1e793892f46196458a63ffe8bde909f248165725...ff1bfae58b5fa9bfd53f6acdaffcd09274ad6fc6/changes
].
|
|
The four variants of bcrypt are independently configurable, but the
badsalt tests for them were all being toggled by INCLUDE_bcrypt, which
is only the macro for the $2b$ variant.
|
|
.perlcriticrc now explicitly mentions every single policy that is
available from the policy packages we use, as either enabled or
disabled. Furthermore .perlcriticrc lists, in comments, all of the
expected policy sets and related distributions that should be
available, with version pins; this is now the canonical home for this
information.
A new ancillary script, build-aux/check-perlcritic-config, validates
that the list of policies in .perlcriticrc is 1:1 with the set of
available policies. It is run by ‘make distcheck.’ (Todo: convert to
a local perlcritic policy so that every run of perlcritic does this
validation.)
These changes will hopefully prevent future problems along the lines
of 5ad98a9dca84f064e7dc19049afbc478d7320f97 (reverted by
55880f9d2e057b83ccaf29132f01a16d7285a7a7), where two developers did
not have the same set of perlcritic policies installed and therefore
disagreed on whether a ‘no critic’ annotation was necessary. They
will also ensure that the set of perlcritic policies being tested in
CI is exactly what we expect.
The downside of this change is that .perlcriticrc is now harder to
read through and find all the policies with configuration. Better
ideas for organizing it are welcomed.
|
|
These 'no critic' markers are necessary when Perl::Critic::TooMuchCode
is installed.
This reverts commit 5ad98a9dca84f064e7dc19049afbc478d7320f97.
|
|
|
|
It is sometimes useful to compile all the test programs but not run
them. Add a Makefile target that does this.
|
|
Both of the things deepcode.ai flagged were harmless in context, but
a clean lint baseline is valuable and the revised code is cleaner anyway.
* test/crypt-badargs.c: Don’t use clever tricks with strcat to
formulate test labels, use a straightforward snprintf for each.
* test/ka-table-gen.py: Remove duplicate `import os`.
|
|
Future readers of this file might wonder why we need to install the
‘llvm’ package as well as the ‘clang’ package; add a few words of
explanation.
|
|
Separate the script that logs the versions of _all_ our build
dependencies from build-aux/travis-before. Run it from the Github
Actions configuration.
|
|
|
|
This not-yet-released feature for fkirc/skip-duplicate-actions
(see https://github.com/fkirc/skip-duplicate-actions/pull/112) avoids
a failure mode in which each concurrent duplicate job cancels itself
in favor of the other one, and thus no builds are run at all.
In order to use the feature, we must pin the action to
98d1dc89f43a47f8e4fba8e1c1fb8d6c5fc515ee for now.
|
|
This is the second of two patches being landed on trunk ahead of the
complete migration from Travis CI to Github Actions; see the commit
message for 407a546a1a8477dd18d396cb6e0533032062381e for more
explanation.
This workflow may not yet achieve code coverage matching what we had
with Travis. We are only building on Linux for now, and only with a
handful of configure-time variations.
|
|
We are migrating from Travis CI to Github Actions because the Travis build
farm has become too unreliable. As of spring 2021 any given build was
likely to have at least one job fail due to problems within the build
infrastructure (e.g. timeouts downloading dependencies). This was at the
same time the company was failing to communicate honestly about the
withdrawal of their free-to-FLOSS-projects tier of service: see for example
- https://blog.travis-ci.com/2021-05-07-orgshutdown
- https://travis-ci.community/t/org-com-migration-unexpectedly-comes-with-a-plan-change-for-oss-what-exactly-is-the-new-deal/10567
- https://travis-ci.community/t/extremely-poor-official-communication-of-the-org-shutdown/10568
We appreciate that “free to FLOSS projects” is becoming untenable for many
organizations due to abuse: see for example
- https://github.blog/2021-04-22-github-actions-update-helping-maintainers-combat-bad-actors/
- https://man.sr.ht/ops/builds.sr.ht-migration.md
- https://about.gitlab.com/blog/2021/05/17/prevent-crypto-mining-abuse/
where three competing CI providers (including Github) announce,
without mincing any words, that their free CI service is being abused
to mine cryptocurrency and, as a countermeasure, they are requiring at
least the _ability_ to charge all users for their CPU time costs.
If Travis had been this transparent we might have been willing to work
with their new model, but they weren’t.
The Travis configuration and build scripts are only disabled, not removed,
with this commit; they will be removed when the migration is complete.
Our original plan for this migration was to do all the work in a topic
branch (see pull request #131) and land it only when complete, but due to
major gaps in our code coverage logs on trunk (in turn because of Travis’
unreliability) it has proved impossible to tell whether changes on the
branch are having the desired effects. We are therefore making a clean
break on trunk, with this commit and the next one, ahead of the rest of the
changes.
|
|
|
|
Test patterns are now mostly generated rather than manually coded into
a big table. Not reading past the end of the “setting” part of the
string is tested more thoroughly (this would have caught the sunmd5 $$
bug if it had been available at the time). Test logs are tidier.
This change exposed a lacuna in handling of text beyond the end of
the setting: we were not rejecting e.g. ‘$3$__not_used_:c809a450df09a3’
because nthash ignores everything after ‘$3$’ and nothing else detects
the colon. Now setting strings containing ‘:’ or a newline are
rejected in generic code. (I would like to expand this to match the
rule laid out in crypt(5),
Hashed passphrases are always entirely printable ASCII, and do not
contain any whitespace or the characters ‘:’, ‘;’, ‘*’, ‘!’, or
‘\’. (These characters are used as delimiters and special markers
in the passwd(5) and shadow(5) files.)
but I am worried that that will trip up people with stored passphrases
with weird salt, like in <https://github.com/besser82/libxcrypt/issues/105>,
so I’m not doing it yet.
|
|
Debian has switched to use the yescrypt hashing algorithm as
the default for new user passwords, so we should add a group
for this distribution.
|
|
|
|
|
|
|
|
|
|
Packit provides tooling and automation to integrate upstream
open source projects into Fedora or CentOS Stream.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
In the previous commits we implemented the return value for indicating
the use of weak hashing methonds in crypt_checksalt(). It turned out,
the implementation has a bug, that that erroneously reported settings,
which are representing an empty passphrase or an uncomputed setting for
descrypt without any salt characters to be weak instead of invalid.
|
|
The jump label will be useful, as soon as we start implementing
the other not yet used return values of crypt_checksalt(), since
it would not make much sense, e.g. to check for a too weak salt,
if the hashing method is already disabled.
|
|
|
|
|
|
|
|
As the crypt_checksalt() function now indicates whether a hashing method
selected in the setting is considered to be weak, we need to adjust the
expected return values.
|
|
|
