Hi,
first off, sorry if my write-up seemed a bit harsh, this was the last
time I am trying to respond to a change proposal late at night :).
On 12/6/22 14:14, Siddhesh Poyarekar wrote:
On Mon, Dec 5, 2022 at 7:35 PM Jaroslav Prokop<jar.pro...@volny.cz> wrote:
Default C and C++ compiler flags to build packages in Fedora currently
includes `-Wp,-D_FORTIFY_SOURCE=2`, which enables fortification of
some functions in glibc, thus providing some mitigation against buffer
overflows. Since glibc 2.34 and GCC 12, there has been a new
fortification level (`_FORTIFY_SOURCE=3`) which improves the coverage
of this mitigation.
The core change to bring in this mitigation is to change the default
build flags in `redhat-rpm-config` so that packages build by default
with `-Wp,-D_FORTIFY_SOURCE=3`. There are packages (e.g. `systemd`)
that do not interact well with `_FORTIFY_SOURCE` and will also need a
workaround to downgrade fortification to level 2. The change will also
include this override.
How come systemd gets an exception? If it is a security option, it should be
enabled everywhere.
It's an unfortunate (although hopefully temporary) exclusion, see this
issue for more context:
https://github.com/systemd/systemd/issues/22801
In summary, systemd uses malloc_usable_size in a way that it's
discouraged, which results in behaviour that's undefined by the
standard, which is now caught out by the compiler. We're hoping to
convince the systemd community to fix this so that we can actually
build it with fortification.
I do not see benefit in a security change that ignores PID 1 process,
Do you mean that factually or rhetorically? I've shared statistics of
improvement in the proposal, surely improvements to packages like
bash, sudo and coreutils would count for something :)
If the feature, on the GCC side, is not 100% done.
How do I tell a difference of a bug with the _FORTIFY_SOURCE which I will
ignore and a bug with my package?
_FORTIFY_SOURCE=3 has been enabled on OpenSUSE for nearly a year. If
it causes a crash in your package, it's extremely likely that your
package has a bug.
This is interesting to know. I was not aware of this as I do not follow
other distributions closely.
I would have appreciated seeing this noted on the proposal.
I then know I can compare spec files / build results with the relevant
counterparts if it works for them but not for us.
I do not have the knowledge or the time to be able to say that GCC generated
the wrong machine code and therefore it is not a bug with my package.
If my program was not complaining before the change and is now complaining with
the change, I am opting out of the change, and filing a bug against GCC on
Fedora.
I assume that by the package providing the exception, packagers get to choose
themselves and we do not need to go through FESCO
to disable a security feature?
I think they should have to, but that's something for FESCO to decide.
_FORTIFY_SOURCE is not a new feature, the new level is just better at
catching bugs. Significantly better.
To this point, including and opt-out mechanism is IMO reasonable.
Even including a "reference" to the opt out mechanism of the current
_FORTIFY_SOURCE level,
should it be the same, is valuable.
== Benefit to Fedora ==
[https://docs.google.com/spreadsheets/d/1nPSmbEf3HVB91zI8yBraMqVry3_ILmlV2Z5K7FZeHZg/edit?usp=sharing
Analysis of packages] in Fedora rawhide indicate that the improvement
of mitigation coverage is on average over 2.4x, in some cases
protecting more than half of the fortified glibc calls in the target
application.
This change will thus harden Fedora to a significant extent, thus
making it a more secure distribution out of the box.
1) Is there some complete source for all these findings? If google sheets cannot handle
all the "raw data" as noted in the comment,
maybe it is the wrong medium.
I've literally posted the raw data in the sheets, please take a look.
Ah, I was starting to get mixed up the sheet pages. The "Raw data - All
x86_64" include the sentence "Note: Summary data since Google sheets
didn't like the raw data".
Other raw data variants seem to be correctly "raw".
2) What does *anything* on that google sheet mean. I have managed to figure
out, from the article, that bos and bdos correspond to level 2 and 3 of
_FORTIFY_SOURCE.
However, total of /.*/? Violated accesses? Segfaults? Then followed by "Sum of total".
For rubygem-ffi, this reaches into hundreds while "bdos" is 2. That is the only sum I can
make, with the data provided.
I am no wiser from looking at it, what do the data mean?
I've mentioned this in the article, but it's a compile-time statistic
of the number of success of __builtin_object_size vs
__builtin_dynamic_object_size, using the fortify-metrics plugin:
https://github.com/siddhesh/fortify-metrics
The numbers you should be looking at are the "coverage" columns, which
tells you what percentage of the calls were successful. For
rubygem-ffi, the fortification is negligible regardless of whether it
is built at _FORTIFY_SOURCE=2 or _FORTIFY_SOURCE=3. This means that
it is unaffected by this change.
What I was trying to say with that paragraph is that the format is hard
to understand for me and I really did not expect
the description or link to it under "More trapped buffer overflows".
A line next to the link in the Fedora Change along the lines of
"Collected via fortify-metrics, see <fortify-metrics repo>."
provides me with the required context.
3) I cannot speak to much else than Ruby, I do not see ruby in neither the failures or
"All x86_64". Should we attempt to test it ourselves?
That's odd, thanks for pointing out.
Please provide a proper "how to test" section, I cannot fix what I cannot test
or compare results when I have no idea what I am seeing.
Actually, last time I heard about number of packages, it was around 50k (not
source, build result), and as I stated,
Ruby is missing, and so are quite a few dependent packages that should have GCC
involved somewhere:
~~~
$ dnf repoquery --whatrequires "*libruby.so*" --disablerepo="*" --enablerepo="fedora"
2>/dev/null | grep -v "i686" | uniq | wc -l
115
~~~
If we also filter rubygem-* packages that depend on the *libruby.so* (and most
probably contain a binary extension written in C/C++ that links to Ruby), I get
68 packages.
When I search "All x86_64" for "ruby" I get 28 packages. That is... not adding
up.
Yeah, I have missed packages; I had looked at revdeps glibc-devel,
which turned out around 9k packages, of which 6k had any
_FORTIFY_SOURCE use at all. I can redo with *all* packages and
generate a report.
I'll be honest, I do not expect that much of an improvement if the
package using only Ruby provided memory C-API (which is why you probably
searched only for glibc-devel).
== Scope ==
* Proposal owners: Post a merge request to redhat-rpm-config with the
actual change to build flags.
* Other developers:
Resolve bugs filed for build failures, either by fixing the bug
exposed by `_FORTIFY_SOURCE=3` or by disabling `_FORTIFY_SOURCE=3` for
the package if it is a false positive or if the package is unable to
adapt to the change.
* Release engineering: Mass rebuild required
* Policies and guidelines: Guidelines should include workaround for
packages that fail to build with `-Wp,-D_FORTIFY_SOURCE=3` due to a
false positive.
I'll just ask, what is a false positive. How can I tell. What are the steps for
this.
If _FORTIFY_SOURCE causes crashes in your package, it's most likely a
bug in your package. In the rare case of a false positive (or even if
in doubt or if you can't figure out what the bug is), cc me and I'll
help you determine what to do there.
Thank you and Florian for speaking up to this point. Again, I would
appreciate if this would be noted in the change proposal
as that will be the document I consult first if I suspect that
_FORTIFY_SOURCE might be causing problems.
* Trademark approval: N/A (not needed for this Change)
== Upgrade/compatibility impact ==
No ABI change, so there should be no impact on compatibility in a
mixed environment.
== How To Test ==
* Smoke testing of packages to ensure that they continue to work
correctly. Some packages may have overflows exposed at runtime, which
may need to be fixed.
This does not describe how to test, e.g. ahead of time, just that the packagers
are to deal with the fallout...
cc me, I'll be happy to help you out.
What I had in mind here are use cases like Ruby C extensions.
If Ruby's GC goes anywhere close to the related calls and there is a bug
in the API, we cannot validate it without first rebuilding Ruby
and then all the related binary extensions with the option to cover most
ground.
Maybe I am chasing a red herring with this, but I generally err on the
side of caution.
I did a rebuild of nearly 10k
packages and there were few failures (see the failures tab in the
doc). I'll expand that list to make sure that the pain on maintainers
is minimal.
Thank you, I'd appreciate that.
== User Experience ==
No noticeable change to users.
I beg to differ. From what I am able to tell, from the limited provided
sources, packages can have a runtime crash.
A LOT of GUI packages use C or the likes that respond to the flags of "fortify
source".
This can have a serious impact to users by the recompiled packages:
* not suddenly working, package's tests passed, but user triggered just the
right code branch
* performance impacted due to overhead.
I don't think that's the intended definition of "User Experience"
here, but I'll be happy to add to it if needed.
I agree that this one seems more open to interpretation what belongs where.
What I latch on to is the following sentence in the change template
"Describe what Users will see or notice, for example:"
As a user I will definitely notice app crashing :) .
I am not as worried about this since I found out, via the discussion,
that other distributions have this as a default for some time now.
This inspires a bit more confidence in general.
Thanks,
Sid
Thanks for the response,
Jarek
_______________________________________________
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue
