-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Debian No-Feature KeePassXC Package #10725
Comments
@julian-klode this needs to be reverted asap. This is now our fourth bug report because of the decision to neuter the base KeePassXC package in Debian. Put the base package back where it was and create a keepassxc-minimal. |
I'm afraid that's not going to happen. It was a mistake to ship with all plugins built by default. This will be painful for a year as users annoyingly do not read the NEWS files they should be reading but there's little that can be done about that. It is our responsibility to our users to provide them the most secure option possible as the default. All of these features are superfluous and do not really belong in a local password database manager, these developments are all utterly misguided. Users who need this crap can install the crappy version but obviously this increases the risk of drive-by contributor attacks. |
Good luck to you. Really bad decision. We will be sure to let everyone know.
It's the same code base from the same provider. Do you have a documented, validated security issue with ALL of the features we provide? Otherwise, it's your opinion (and likely yours alone) so 🤷🏻 |
The dev is right every plugin should be selected by the user and can be considered a potential security risk either backdoor potential or vulnerability potential. |
You fundamentally misunderstand our program when you use the word plugin. These are built in features, not plugins. The features can be enabled as desired by the user and they come disabled by default. This change to not compile and ship these features in the base keepassxc package does nothing besides create angry (or confused) users. |
My 2 cent as a former KeePassXC maintainer: Compiling without plugin was added as a feature back in 2016 to reduce attack surface and potential vulnerabilites (see #50, #123, #125). This was especially a heated topic back then because KeeHTTP had vulnerability in the past and also the favicon fetching mechanism via HTTP was (is?) also sucettible to SSRF attacks. IMHO KeePassXC should provide a warning UI in place of the usual "Browser integration" page informing the user that the current installation don't have such capabilities and they should install the "full" version instead. |
Im not to familiar with software development, but does this mean Julian is not compiling the same source code in this repo? |
@TheZ3ro I'm all for a keepassxc-minimal package that has no or limited features. To switch the main keepassxc package from full featured to no feature is the problem at hand here. We will bear the brunt of this decision with issues and complaints, not Debian or Julian. @Jordo0G it is the same code just built with feature flags turned off. So not all the code is present in the final build. |
I don't know how to reply but that's difficult to stay calm while users are insulted that way. You can't expect all users to read NEWS/CHANGELOGs from the hundreds of packages installed on their machine. There's an implicit trust contract maintainers should follow that packages don't change behaviour that way, unless explicit requests are made from the upstream project. |
Is this change expected to be rolled out in default installs on other platforms? Browser integration is a bullet point feature (along with TOTP and SSH agent), and I might move to another fork if they are discontinued or not enabled by default, or this change makes its way to Debian stable. |
As @droidmonkey said, none of these features are plugins. All of them are built-in functionality that belong to the main product. If anything, we will reduce the number of such compile-time flags in the future, so these things cannot be disabled anymore. For example, one that will definitely go is WITH_XC_YUBIKEY. It has served its purpose, but it is becoming a burden. More flags mean more possible configurations in which bugs can occur and it is impossible to test them all. The safest KeePassXC is not the one with all flags disabled, but the one which is tested best by us and by the majority of our users. The only real safe way to reduce the code surface is to actually remove code, not to ifguard sections of it and hope that disabling them doesn't introduce new bugs. We do run a minimal build with all flags disabled on our CI, but that is all we do to test it (that said, if you start enabling flags selectively, you are basically on our own). |
I've empathy for all parties in this situation, I think the proper solution would probably be to package both a "-full" and "-minimal" version of the software and utilize Debian package meta-data fields to define a It is generally a violation of Debian policy for an upgrade to break existing functionality it is also generally encouraged to provide builds that allow for choice and self determination for system's operators. addendum to the above: I've not been actively maintaining/packaging any deb stuff in a number of years, the actual interaction of the various package relationship metadata might be slightly different than I've described above, but there should (IIRC) be a combination of metadata that will result in the behavior I've described. |
So in the future every installation of KeePassXC will depend on libyubikey-* (thus increasing the code, the attack surface, the possibility of supply chain attacks, etc)
Disabling those compile time flags actually removes code and reduce attack surface so yes, the safest KeePassXC is the one with less "enabled flags".
Distro maintainers run build tests as well, and (contrary to KeePassXC maintainers) have actual telemetry data and crash reports affecting the end users. Having said this, I expect that the average user does want some (or all) of the so-called "plugins" and the full keepassxc experience out-of-the-box. I think the best solution would be to provide a keepassxc-minimal package on the distro side (as suggested by @droidmonkey #10725 (comment)). I also totally agree with @drawks proposal #10725 (comment) |
+1 for keepass-minimal |
We don't depend on libyuibkey anymore since it is basically an abandoned package. The code necessary to interface with yubikey is shipped with keepassxc source. Various good reasons to do this, that code was thoroughly audited by me and is rather minimal. |
if you upstream developers will agree on suggesting Linux distros package maintainers to provide also a minimalistic KXC version, I will make sure that it will be shipped in Fedora / EPEL |
Perhaps you should create your own .deb repository and publicize it? A large number of projects distribute out of their own repository and don't seem any the worse for it. Time to join them? |
or just acknowledge that most users use a small subset of all the possibilities of the program, and that the obvious most secure solution is to provide it ? Of course, the most flexible solution would be to make those less used possibilities dynamically loadable, like plugins, so that distribution can package them separately and offer a true customized solution to users. But in the meantime, a default minimal package that meets the requirements of most users, and another one for users who need more possibilities, seems the way to go. |
@julian-klode Thank you for the change. I have been using KeePassXC for a long time, and I was not aware that all these features were present, and I strongly prefer them not to be included by default. A package EDIT: Update: #10725 (comment) |
Browser auto-fill is now "crap" that doesn't belong in a password manager? That's absurd. Browser auto-fill is a required feature of a modern password manager, to prevent users from doing all the things they resort to without it. This doesn't make KeePassXC a more secure password manager, it makes it an encrypted spreadsheet. Speaking as someone that uses Debian and KeePassXC all day long every day and will be irritated by this decision at some point in the future. |
The easiest solution is for KeepassXC to have the flags also function as switches that enable notifications to users about features that are missing and inventory them on the about page, etc. Preferences related to the disabled features could be grayed out or replaced with stub notices that the features were disabled at compile time and not to file bug reports. |
What fantasy world do you live in?
Kind of the point, because they are literally disabled by default. Every feature is opt in for KeePassXC. If you don't enable it, the code never gets executed. We think long and hard before introducing a new feature. We have an extensive code review process. We aren't perfect, but we certainly put a ton of effort into delivering a secure and feature rich (if you desire) password manager. |
If the reasons for the new stripped Debian variant called If so then why would they the Debian packagers be offering the If the Debian packagers have good reason to believe the It is one thing to talk about a broader attack surface to developers or power users who have some grasp of such issues, but speaking of such issues to end users is rather snide, when there doesn't seem to be any evidence of these problems in the wild. |
Unfortunately it won't be the same subset. For example, I use my yubikey but I don't use browser autotype. My elderly relatives, the other way around. |
Where would the user learn about those news files? I know with Gentoo they will actually show a message that there is news when syncing with the package repository.
I don't understand why you would be using a GUI password manager in that case. Security is very important, but must still be balanced with usability.
As a person who uses KeepassXC because of said 'crap', I'm not excited for the day this gets downstream of Debian into other Distros (Ubuntu, KDE Neon, etc). |
@Dio9sys no, this was not communicated to us before hand, nor was there a chance to collaborate on an effective solution for both parties. There also seems to be "no going back" per Julian above. So this one way door decision is the new reality I guess. All I can say is, use Flatpak and get away from distro lock in altogether. |
yep, that's why the real solution would be to make those features truly dynamic. In the absence of that, the next best thing would be to provide several versions of the package with different « most used » subsets. In the absent of that, provide the minimal most secure version, and another with all features. That doesn't seem so drastic to me: anyone can keep using the full version if they so choose, and most users get the minimal most secure one. |
@Kwpolska If the experience with xscreensaver is any indication, I would not be surprised if Debian would patch out any such UI messages directed at Debian users, even if upstream were to somehow implement those. |
@nekohayo If Debian does so, then all issues mentioning Debian should be auto-closed. |
nope, they create more potential for exploitable security issues. That's the very definition of an attack surface: it does not need to be exploited, just exploitable. It is good practice to reduce it, even in the absence of proved exploits. |
Adding support for dynamically loadable modules is not renowned for reducing attack surface. |
From a governance and transparency perspective, it's a bit concerning that there's not really any traceability for why such a major decision was made. Was there a specific incident that prompted this change in packaging policy? Was this part of a larger audit? Is there a list of criteria that's used to determine which parts of a project are "features" and which are "crap"? How are those principles applied to (for example) browsers, arguably the largest security surface that a user actively engages with? Why is discussion about this change largely happening on a GitHub issue for the upstream project affected, and not the project that made the change? |
How about not breaking existing userbase and preparing keepassxc-minimal package instead of ego-powertriping? |
it depends on the feature set. Take a cms: would you want to use a wordpress or drupal installation with each and every single module out there activated ? I don't think it would actually even be possible. But anyway, in this case, the possibility for loading functionality obviously reduces the attack surface. I don't know where keepassxc stands on that feature scale (obviously lower than a cms :-) ), but there are functionalities that are obviously used by a very small subset of users. Some of them do expose the network. So yes, despite what many seem to think here, they are obvious attack vectors (and no, I don't know of any actual attack, that's not the point). So instead of launching flames at one another, maybe a better solution would be to acknowledge the problem, start a discussion to determine what a minimal, standard, and full package should contain., and how to migrate existing users to that new scenario. But maybe that's too much to ask... |
Answering my own comment: #10725 (comment) Although I think that Given this disagreement, replacing the existing |
Would have been nice to have about a month ago when this was unilaterally put into action. Alas, here we are. So yah flaming arrows will absolutely be thrown because there was no chance at proper discourse. This thread should be a lesson to downstream folks who think they know what's "best" for the user. FWIW, all the maintainers of KeePassXC use all the features present in the software. So by that alone, you would think we care a lot about their security and functionality. We only ship code that we care about. Take one look at some of our open PRs and the amount of discourse applied to every new feature PR (remote database support is a standout). In the lead up to this thread I received three reports of this new package method crippling people's workflow. One report was a user who couldn't open their database anymore because the yubikey feature was removed. Let that sink in for a second. People who lose access to their most important secrets can sometimes do irrational things in the moment of panic. |
The arrogance here says it all. @julian-klode doesn't just want to remove the options, he insists on insulting all the users who have used them and depended on them for years. If that is the case the proper thing to do is to remove it
Since when did these features get to be considered superfluous? I've only been using Is Debian now suckless.org? If so why not go the whole hog and take out systemd as well, after all you don't need all that crap running under Trust is an essential part of life, and we generally trust others to do what they say they will do and reliably and safely so, and as IT users we put our trust in others knowing that there is a risk involved in that trust. If that was the case @julian-klode would never even use computers, or fly in a Boeing plane, or even run a checking account because of flaws in the systems we use that are well known to hackers in those fields. Let people decide for themselves whether the risks are worth taking and don't restrict or frustrate their choices in your bid to make them safe. |
Your main responsibility is to ship the package to users. If you're not happy with the features of a program because reasons, then fork it. Oh wait, that's what you essentially did. But you stole the name "keepassxc". Not very nice.
When I installed the keepassxc package, (like many other users I suppose), I explicitly wanted these features. Now you're going to tell me you don't care about breaking functionality? That goes against everything I'm looking for in a distribution. I want stability. This is anything but. BTW I wonder how much of Debian's code of conduct you're breaking just now, in this thread? Or does that simply not apply to external contributors? |
Did you do some kind of survey to determine how small this subset supposedly is? I didn't receive a copy if you did; I'd be happy to fill it out.
That is absolutely the point. You state they are "obvious", but... they're so obvious that you can't describe them? Whenever the topic of security comes up in software, there are people that wave their hands around terms like "attack surface" and "threat model" and "vector", but struggle when pressed to provide any specifics. People with more expertise in software security likewise don't have as much trouble with this; they have reasons behind their recommendations. Security doesn't exist in a vacuum. All manner of really bad ideas have been popular over time "for security" (like forced password rotation), and have resulted in overall weakened security. There are "obvious" security benefits to HIBP integration and browser auto-fill. Users should be automatically notified when one of their accounts has been breached and their password for that account has been found floating in a db dump. Users should rely on their password manager to handle logins for them, so they're less likely to get tricked into a phishing page. These are actual threats that these features actually address. There needs to be stronger justification for removing them from the default package (against upstream's recommendations) than "because security".
Indeed. Let's set up a |
more like two weeks ago, but hey... right now it's only in debian sid, the development distribution, used for... well, development, and only by people who are supposed to know what they are doing; and a few days ago it migrated to testing, the distribution that noone should be using except people who really know what they are doing. All these people are supposed to know where to look when something breaks, and most certainly to file bugs against debian instead of the upstream project. That's obviously a packaging issue, not an upstream one, it's even documented in the changelog, the NEWS file (which is shown during upgrades), and the bugtracker. It couldn't be made more obvious and/or documented. People complain here ? Redirect them to debian's bug tracker without any second thought. They use development tools, they should know how to deal with them. And in the meantime, please drop the pressure a bit :-) |
This comment has been minimized.
This comment has been minimized.
Indeed, discussion on this should happen on Debian's turf. One bug has already been opened, but is way too soft IMO: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069743 BTW a heated discussion is also happening on HN: |
no casual user can be exposed to the current package. Absolutely none. |
Ok I misunderstood you, will retract my comment |
This assumes that all casual users are on Debian stable. I do not think this is a valid assumption, some people who are not ultra-casual but also not Debian experts may have chosen testing or unstable due to more recent packages (and still reasonable stability). |
I'm going to make another attempt to high-road this discussion a bit.... If you are here to contribute to suggesting a solution to the issue that hasn't yet been advocated before, by all means this is the place to have that discussion. If you are here to "me too" or worse to degrade the the dialogue with various ad-hominems rest assured that such "contributions" are not actually likely to result in a positive or expedient resolution. It is unfortunate that @julian-klode has taken the tactic of using demeaning language and and doubling down on their decision instead of engaging in a meaningful discussion. It is also unfortunate that consumers of the It is probably past time for this issue to have public comments disabled by the maintainers and for everyone to have a less heated and more promising discussion. |
If keepassxc provides these compile-time flags it seems a bit off-base to complain when they are actually used? The issue of upstream receiving bug reports related to packaging seems like it's the only thing that should be relevant to upstream. Which is why I will repeat that the best option here is just to increase visibility to end-users that optional features were disabled during compile. People who don't want network features should be able to quickly see that they're not present. People who want them should be able to quickly see that they're enabled. And when someone asks "hey why isn't this working" quick checks of compile options make triage fast and it never even bubbles into upstream. |
agreed, but if they are on unstable or testing, they have to know a bit about the processes, docs, and tools of debian. Even more so if they are capable of finding KeepassXC's github project and report an issue. I'm dead serious when I say « just point them to bugs.debian.org ». The only issue for keepassxc could be that it doesn't show anything when a feature has been « compile time » disabled. It could show something. (could, not should or must). |
Gonna lock this up, thank you for the healthy discourse. |
Perhaps to add some final remarks on the "less code means fewer bugs" stance:
I think this is a fundamental misunderstanding of the relationship between lines and code and attack surface. Bugs are not directly CAUSED by more code, though there is a strong correlation between more code and more bugs. So far I do agree. However, this is not something that can be rectified by more ifdef guards. Bugs are correlated with lines of source code, not with binary size. More ifdef guards mean MORE source code, therefore MORE bugs. Ifdefs in particular (due to the abysmal nature of how the preprocessor works) also add more (basically untestable) compilation unit configurations, hence I think the following proposal is the absolute worst thing we could do:
|
Overview
I'm using the Brave and Firefox browsers under Ubuntu testing using keepassxc version 2.7.7, suddenly the browser integration doesn't work anymore. So I went into the settings menu to enabled it again. But the "Browser Integration" from the listbox selection is gone. Not only that one but there are only two items left:
See the picture in the attachment. All the other options are missing, so I can't enable the browser integration anymore.
Steps to Reproduce
Expected Behavior
Working browser options and working integration
Actual Behavior
Browser integration is not working and the options to enable them are not available anymore
Context
KeePassXC - Version 2.7.7
Revision: 68e2dd8
Qt 5.15.10
Debugging mode is disabled.
Operating system: Debian GNU/Linux trixie/sid
CPU architecture: x86_64
Kernel: linux 6.7.12-amd64
Enabled extensions:
81db-edfbe67abc5f)
Cryptographic libraries:
Desktop Env: Gnome
Windowing System: X11
The text was updated successfully, but these errors were encountered: