The region tag is hand-checked for its special values in
headerVerifyRegion() and now that v3 packages get the first tag
checked (commit 1c25d27895), there's
little point in calling headerVerifyInfo() on the the region tag.
The region tag data (aka region trailer) violates the regular ordering
of tag data and can't be checked with headerVerifyInfo() in one go,
but for v3 packages without the region tag start again from tag 0
to get data overlap check on the whole thing, for both headers.
As a side bonus makes the signature code again that little bit
closer to the other header...
This used to be there before commit 89dce2b91d
sort of made it unnecessary, but since then we've started looking
at the actual data in case of string tags, so there's new opportunities
for trouble as well - witness commit
1b93f23579. So this is kind of a
belt-and-suspenders thing...
We only have a disabler for %ghost files ATM but that doesn't mean
the actual code can't be generic, the logic is actually much clearer
this way (especially compared to the to-negate-or-not-to-negate
fun on verify side)
One might envision a generic way to set filters via a cli argument to
go with this, eg --noattr=cd to skip documentation and configs,
or --noattr=g as an alias to --noghost.
Verify has "always" accepted --noghost as an option but it's always
been broken too, the --noghost option used to *clear* the bit on
qva_fflags, bit which nothing set in the first place. When you
test that bit for enabling verify on ghosts, chances are the ghosts
remain ethereal...
That was until commit efd696d32d fixed
the bit clear to set to make --noghost work on queries, but hardwired
the verify code to no ghosts. Wtf?
So... after 15 years of existence (commit
60977b6c27) and somewhat colorful
history, lets make it work and document it too. qva_fflags is supposed
to be a "filter out if matching attribute in file" bitfield but its been
kinda hard to tell when there was one example left which had it the wrong
way around. The original commit has two more examples where the bit setting
is right but disabled with "ifdef NEVER" ... since removed.
Now that we're verifying the entire header (as of
commit 0d36fc4cba) there's little point
in revalidating the same data here again. Just call ei2h() for endianess
correction instead.
To begin with, rpm v3 packages don't HAVE an immutable region. Which
means that for v3 packages whose signatures we cannot verify on normal
paths, we'd also miss even the most basic of sanity checks because
ril on v3 packages is zero.
For installed packages this also enables data overlap checks for
tags outside the region, which up to now have been checked but only
one by one in headerSigVerify().
Return zero for zuccess (haha) like most of the world does, -1
for success is so bizarre it's like violating the principle of
least surprise on purpose. Callers only ever cared about the
non-success value for error reporting but since that is taken
care of by headerVerifyInfo() it doesn't matter at all. Return
the ordinal of failing tag achieves the same should anybody care.
Leaving the error message to actual index number to keep the message
compatible, not that its likely to matter.
For whatever historical reason rpmReadSignature() had its own loop
for calling headerVerifyInfo() on each tag one by one. Not only is
this redundant since headerVerifyInfo() can loop on its own and a
wholly unnecessary difference between the signature and regular header,
it also misses the opportunity to perform data overlap checks on the
signature data.
Happens with eg pattern "~0//0", easily visible if alloca() is changed
to malloc(). Reported as a security concern, dunno about that but a bug
it is anyway.
For this we need to pass dataStart into headerVerifyInfo(), and
then we can actually just call dataLength() to do the hard work.
There's a non-trivial cost involved of course, especially since we
now do this twice, but better slow than sorry. We can always
make things smarter later on, and this closes down a major hole
in how rpm deals with header data.
We're ultimately copying the data to the sigtd so why bother with
intermediate info and siglen variables? Use a helper function to
to convert the entry into into td instead, eliminating a redundant
memset + copy on the info struct. Plus the resulting code is that
wee bit more readable.
No one is quite sure why there's a redundant '-python' suffix,
but the module isn't named that, and typically we want the name
in the metadata to be the same as the name of the module.
This has no effect on Python code itself, as it doesn't change the
name of the installed module used in import statements, and since
we've never published to PyPi, it's not something that can be sanely
referenced for 'pip' and other similar tools in a useful manner.
Rearrange the message to "linenum: line: message" which is a more
logical order of things (I think), and in particular, include the
actual actual package name in the message. In order to do that,
consolidate the error logging inside lookupPackage() where we
now get a slightly hysterical logic around the error messages but what
the hey, its not like this is an exported API.
All this time rpm has merrily accepted a trigger with no condition
at all, eg "%triggerin --". Doh. Check and error out on missing trigger.
This is all ridiculously subtle and fragile - pay attention to splitting
the line only after all error messages have been done for full error
message, but since we're now moving s further on whitespace we need
to store the separation point into another helper to avoid breaking
the other %trigger line semantics.
Make the missing separator check more obvious (check s, not reqargs),
jump over a potential separator when found, and eliminate a leftover early
return causing memleaks. Apart from memleak, shouldn't actually change it.
"--" in filenames and such might be rare but it can exist, requiring
whitespace surrounding the separator is the only sane thing to do.
Would be easier to strstr() on " -- ", but inevitably that would
break somebodys tab-aligned spec so...
For anybody suspiciously looking at the pointer arithmetics: "--" cannot
be at the beginning of a line which gets us here so s-1 has to be safe,
and on the other side there has to be at least the trailing \0 at s+2.
This is a sort of too-little-too-late bandaid for the issue
in RhBug:547997 / commit 507f21f6bb.
Unexpanded macros here are almost certainly packaging bugs,
except if they happen on a parse for build-requires when its
more beneficial to just let the unexpanded stuff through. We dont
have a means to separate between build-dep parse and build but
lets at least warn about these things. Refactor the logging into
a helper function to avoid umptheen cases dealing with warnings
and errors and spec or no spec etc.
Rpm has always been a bit dazed and confused when it comes to specs
with sub-packages having different version etc from the main package.
Many things work fine in that case .. except .. when they dont. Debuginfo
picking up wrong versions (RhBug:1051407) is just one example, there
are countless more in bugzilla wrt buildroot paths and whatnot.
The simple and sane solution would be not piling on them macros
from sub-packages, but that would surely break somebodys precious
spec tricks.
The ugly but brutally simple and compatible solution to this all is to
create separate set of macros when on the main package, this lets users
in and out of rpm pick which one (latest or main) they want. To hopefully
avoid stomping on anybodys toes, use uppercasing for the macro name (other
variants like %pkg_release are awfully commonly used). Pile 'em on, yay!
Building with -Og reveals m being possibly ununitialized here,
and on closer inspection there's an actual bug here: on linear
search cmpPoolFn() needs the increasing index, not trying the
same (in theory possibly uninitialized) value of mid.
Fixes regression introduced by yours truly in commit
9bf578376d. Oops.
Prior to commit f16a522ca4, failing
as early as possible was the only way to catch errors. However
now that rpm actually checks for the files by itself, this behavior
can lead to really confusing error messages. For example if the
first %doc of, say, twenty files is missing, you'll get error messages
for all twenty.
So don't stop copying on errors but copy as much as possible and
check for errors last, this way the actually missing files will
stand out.
Keys from rpmdbIndexIteratorNext() are not necessarily \0-terminated,
buyer beware.
Sometimes you get lucky, but in particular when built as PIE (such as
by default in Fedora) this falls over consistently.
In Fedora this has been hidden by the fact that test suite has been
disabled because its been so broken with fakechroot until recently,
and without PIE the testsuite regularly passes. Valgrind does
complain though.
Let rpm go into $(bindir) along with everything else, it just isn't
that special. Many distros are nowadays symlinkingbin to usr/bin anyway
which makes the special casing of /bin/rpm even more silly.
Somebody wants to put it someplace else, 'mv' is your friend and
I'm not going to stop you.
This isn't 100% correct but more so than the former version:
"make distcheck" now completes, whee! The test-suite fails left and
right with path issues during it so there's more work to do however.
During distcheck the test data is copied from a read-only tree, so without
this chmod, clean-local will fail due to permissions on the data files.
The other data in testing/ gets copied from other sources or gets
permissions explicitly set (eg make install)
ChangeLog is listed as EXTRA_DIST but there's no rule to create it
so dist target is broken except when invoked with Makefile.maint. Which
nobody finds because its such a strange thing to have.
Move back ChangeLog generation into main Makefile.am but do not
require git to create it. Instead have a rule to create an empty
file to appease EXTRA_DIST no matter what, and only create the
real thing if we're in a git checkout and git command is present.
Replace manually maintained yet-another-return-code with an
"all failures" variable and base the actual return code on that.
It's tempting to use just one verify result variable but that'd be
wrong because of the shared file mtime filtering.
No functional changes intended.
Note that this is for rpmfile only, intentionally NOT adding this
to the rpmfi python representation which is deprecated.
omitMask is optional, otherwise identical to its C counterpiece.
Untested but hey, it works in C - what could possibly go wrong.
Turn rpmVerifyFile() into indexed rpmfilesVerify() method but
drop the unnecessary double return code which also "fixes" the
oddball argument order where return value is not last, add
thin wrapper for rpmfi iteration. Mark rpmVerifyFile() deprecated
due to the strange calling conventions and unused arguments.
Leaving the code in verify.c to make it clearer what changed, looks
a bit out of place but doesn't matter as it doesn't need access
to rpmfi(les) internals.
Verification functionality as such is supposed to be unchanged by this.
Use a local helper variable for collecting verification results,
arrange a single exit point for the function and base the return
value on the verify result to make it clear what the return code
actually means. As a sort of side-effect its now legal to call
rpmVerifyFile() with NULL res, not that it makes a whole lot of
sense to do so since all the interesting stuff is there. But useful
for next refactoring steps...
The size is likely to be wrong anyhow but if we can't read the file
then any correction is likely to be more wrong than the original value.
While at it, eliminate an unnecessary function-level temporary
variable used for this one purpose only.
Now that we can, lets use the actual data types instead of void
pointers and be const-correct about it as well. Which immediately
points out a missing const in ei2h() first arg...
Since rpmLeadWrite() takes a header there's no more need for
rpmLeadFromHeader() to be "public" and since nothing returns
leads there's no need to free them either, make static + adjust
to eliminate the need for ever allocating a lead so the free func
can go too. Whee. the remaining rpmlead.h is something we could
actually consider making public.
Panu Matilainen