-
-
Notifications
You must be signed in to change notification settings - Fork 13.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shipping Security Updates #10851
Shipping Security Updates #10851
Conversation
Thank you! This is much needed. I'm excited to see how this will turn out :) |
I saw it, and this is exactly why I am not using it, and why I am making this work. I will explain more next Sunday in my talk.
No, in fact, this is exactly the opposite. The hack is to rely on runtime dependencies, as runtime dependencies are not known statically (without building packages). The current implementation is a conservative approximation of runtime dependencies. There is still one issue with the current function, as it does not consider the closure of dependencies, but only explicit dependencies. |
This extra patches are changing the design to fit better with the ideal model shown in the slide of my presentation. [1][2] So far, while evaluating packages with
[1] http://nbp.github.io/slides/NixCon/2015.ShippingSecurityUpdates/ |
abiSec; | ||
|
||
|
||
# The package compositions. Yes, this isn't properly indented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, no it is. :)
It won't render |
|
@nbp I've only glanced at your slides, so can't yet speak to that. But even in terms of the refactoring alone, this is a HUGE improvement. Thanks so much! (and @Mathnerd314 too!) |
Oh and @Mathnerd314 got a good portion of my comments too, hah, but they didn't show up on the diff. |
# packageOverrides attributes to refer to the original attributes | ||
# (e.g. "foo = ... pkgs.foo ..."). | ||
configOverrides = | ||
if config ? packageOverrides && bootStdenv == null # don't apply config overrides in stdenv boot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 4 space indent
This is because nix-env uses pointer equality for skipping duplicate derivations, rather than output path or derivation path: https://github.com/NixOS/nix/blob/master/src/libexpr/get-drvs.cc#L193. Personally, I would remove that check; outputting all possible aliases is consistent with the behavior of |
I rebased the patches; still testing it. |
from @Ericson2314
I intended to do something similar to ensure that we do not add such aliases to nixpkgs. I do not think this is a good choice to do this automatically as this effectively means that for installing any sotfware you will have to evaluate all of them. from @Mathnerd314
That's what I thought at first, but then I realized that these differences are related to aliased packages, and that for each newly reported entry, we have a potential security issue (as described in #10851 (comment)) So even if this behaviour of |
Also Ericson2314@03486c8 here is a commit just with the improved documentation for |
I got codetriaged here. What's the status of this? |
I'm not sure about the status of the main security part, but the improved fix-point part has been good to go for a while, hense my advocacy to split this in two. |
Wait, so if the improved fix-point is already finished, what do we have to do to make it work and patch security updates? I'd really like to help on this, but I have no idea how. |
@nbp what sort of fixes are holding up a working demo? Anything that actually breaks the mechanism? Or is it that you're trying to fix all of nixpkgs of static analysis errors? |
There is no mandatory fixes but this would be a nicer experience to demo a larger set of fixable packages. From the infrastructure, the main blocker for getting a working demo is to get the Then, we need some task-force to get some fixes (or dummy fixes) applied in the The static analysis is only a tool made to highlight the parts which would not work properly. I have not yet measured, but I would expect that we have already have a large number of packages with no issues. Among the prerequisite are that I should write some tests to ensure that we can successfully update real packages such as openssl.
The mechanism can already be tested locally. For that you need to check out the Currently, we could merge the security-updates branch as-is, as the patching mechanism is disabled unless the
That is something that I would hardly be able to complete alone. Writing documentation to explain how to fix them would be hard as people tend to innovate every-time they write new packages. One of the restriction of the security-updates branch is that all packages should use |
Closing because this seems to be stalled (for 2 years now), feel free to reopen when work is being done again. |
This pull request has been mentioned on Nix community. There might be relevant details there: https://discourse.nixos.org/t/in-overlays-when-to-use-self-vs-super/2968/5 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2022-10-19-nixpkgs-architecture-team-meeting-14/22578/3 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/timely-updates-for-nixos/29231/7 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Motivations
The way of handling security updates today involves a lot of humans efforts, going from reading the mailing list, to adding lines to the configuration files.
More over, this implies having control over the top-level derivation, which is not always possible with
nix-env
, andnix-shell
. This has to be patched inside the current pipeline, as what got made for NixOS.Also, as the current method for applying security updates is complex, we are only restricting them to a few packages, such as
glibc
,bash
andopenssl
which are used a lot, but we do not such mechanism for all other packages which have minor security / usability updates such asfirefox
.As a work-around, today, we offer the option of having a small channel, to get updates from, but this is not a good solution for end-users, as the small channel basically involves recompiling any large project.
More over the current mechanism cannot safely be composed as explained in #4336.
Goals
This work aims at making it transparent, such as security updates are cannot be distinguished from any other updates by the end user.
This work aims at making it easy, such that packagers only have to cherry-pick a commit made on master into a security/master branch to let the buildfarm compile and distribute the package against the latest stable branch.
This work aims at making it fast, such that the buildfarm in charge of producing pre-compiled versions of the packages will only recompile the packages which are being fixed, against the latest stable branch. When a user updates, Nix will download from the latest stable channel, with the addition of the recompiled packages, and Nix will patch all the packages depending on the fixed packages.
This new approach is sane, as it constructs the patches which have to be apply, without any knowledge of the runtime dependency. Thus, all the packages patches can be build up-front without having to realize any of the derivations. This is a big plus to guarantee reproducible builds of the patching mechanism.
Approach
Nixpkgs as a pure function
This approach aims at constructing patches based on the Nix expressions of the packages. Thus we have to change the way we evaluate Nixpkgs to reflect the patching mechanism.
This approach is based on the recent modification of #14000 (#9400), which make Nixpkgs behave as a single function with no external inputs. As the Nix language is pure, the following property holds for Nixpkgs.
This property means that we can call
f
one extra time on the result of the fix-point without changing the result of the fix-point. As a consequence, all the packages will keep the same hashes.Recompile only fixed packages
This patching mechanism relies on one hypothesis. This hypothesis is that all packages derivations are expressed in the last evaluation of Nixpkgs, and that dependencies are taken from the previous evaluation of Nixpkgs.
This hypothesis does not hold, as of today, but with the help of a static analysis, we can identify how many times we jump through the Nixpkgs argument before we settle on a derivation, and before we settle on its inputs.
Once this hypothesis is guaranteed, we can change the above expression to
Where
g
is a slightly different version off
. With the above hypothesis, this means that all derivations of packages are taken fromg
, but the dependencies ofg
are taken fromf
.If
f
is the stable version of Nixpkgs, andg
is a stable version of Nixpkgs with extra patches. This implies we will recompile any patched packages ofg
, while keeping the same hashes asf
for non-patched packages.At this stage, we have what we expect to let Hydra compile. Still, this stage does not apply any patches to any of the packages.
Building patches
To be able to apply patches to the dependencies, we need the equivalent of a fix-point, to evaluate the dependencies, and the dependencies of the dependencies, and so on … In addition to a fix-point, we want to iterate from the maybe-recompiled packages of
g (fix f)
, and iterate overg
to settle on the patched version of the packages. We would name this patched version of packagesh
.To construct patches, we need to know the dependencies of the packages, and which one got updated.
To list the dependencies of a package, we have to rely on some form of introspection, encoded as part of the derivations. As a first approach I relied on
callPackage
, which would give us a super set of the dependencies, but this has a lot of issues, and left-over packages. As a second approach, I now rely on thebuildInputs
which are exposed by the derivation, but this has the issue of being a sub-set of the dependencies. Still, I am confident that we can instrument the default builder to ensure that buildInputs are a super set of the runtime dependencies.To check for updates, we can compare the
outPath
of 2 derivations. If the derivations have a differentoutPath
, this means that the derivations has a different inputs. (Note: hash collisions are unlikely, but if this happens we can easily add a dummy environment variable to get a different hash)Patches are composed of the hashes of the old versions, and the hashes of the new versions. Making a patch for one derivation, is being able to list the old hashes and associate them with the new hashes. The hashes are the hashes of the dependencies.
Based on the last hypothesis, looking for the dependencies of
g (fix f)
, will gives us hashes of derivations fromfix f
. These are our old hashes.Looking for the new hashes, implies that we have to build packages based on patched versions of the packages, i-e
h
. Thus if we look for the dependencies ofg h
, we would have our new hashes.Therefore
h
definition should look like:Where
patch
is a function which on a derivation, will take the value ofg (fix f)
, and apply a patch for any of its dependencies which got updated, by substituting the dependencies fromfix f
by dependencies fromh
.In practice, we also need to feed
fix f
to thispatch
function, as we have to make sure that patches can be applied safely, by ensuring that theoutPath
of the derivation still have the same length.Status
nix-env -f ./. -qaP --drv-path
works whenquickfixPackages
is equal todefaultPackages
, and does not cause recompilation of existing packages. (Note: nox-review fails to see that we have no recompilation because of violation of the one-application hypothesis)nix-env -f ./. -qaP --drv-path
always works, whenquickfixPackages
is equal todefaultPackages
.This mechanism is disabled by default, as no
quickfixPackages
are provided yet. Also, when disabled, this work has no impact.To avoid to bloat and delay these changes, I would recommend that we merge them as soon as possible, even if the one-application hypothesis does not hold yet, and address any issues reported by the static analysis as follow-up issues.
References
NixCon slides: http://nbp.github.io/slides/NixCon/2015.ShippingSecurityUpdates/
NixCon video: https://www.youtube.com/watch?v=RhcKXS00zEE