mirror of
https://github.com/dani-garcia/vaultwarden.git
synced 2025-12-10 17:23:04 +03:00
unnecessary privacy issue with icon_cache? #2059
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @jhaar on GitHub.
I was snooping around the sqlite db and noticed there is nothing unencrypted in there - can't tell usernames, etc. I'm thinking "wow - bitwarden_rs is privacy focused". Then I looked in the icon_cache directory and see thousands of website dns names in cleartext. I can see work DNS names as well as Internet - and if I was the sysadmin for a group of people - I'd be able to see what websites people use - even if I couldn't necessarily tell which users goes to what website
Shouldn't you apply the same privacy-first approach to those files? After all - there could be porn sites, banks, cancer treatment sites - all kinds of potentially upsetting detail. I assume you could have a "secret" somewhere that could be used to encrypt the filenames - thus "hiding" them from view? (eg rsa_key.pem).
@mqus commented on GitHub:
Well there is one use-case where hashing the domain names is useful:
Preventing accidental discoveries of such information. If an admin just wants to view/edit the db or snoop around in it, he won't see which domains were used at a glance.
On the other hand, this could imply false safety (which we could try to prevent by documentation) and I agree that this isn't that useful to have because we can't link users to domains anyway.
@fbartels commented on GitHub:
But if you have a server wide secret, the person with access to the icon folder can still use this secret to decrypt filenames again.
If you want to get rid of domains in the filename, then a hashing function is probably the better approach (e.g. store md5 hash of domain name, instead of domain name itself)
@BlackDex commented on GitHub:
We could make it an option, but still, those images contain the logo of the site, the log will also contain the requests (unless this can be disabled or we need to use the hashed values in the logs also.)
All things considered i think it's a bit to much, but if really wanted we can add it.
@jhaar commented on GitHub:
Am I over-thinking it? The documentation (https://help.bitwarden.com/article/website-icons/) which is linked to the Option under the User-profile states that it "leaks" data. So if the "Disable Website Icons" option per user is disabled by default and the user chooses to enable it - then it's absolutely fine? Frankly imports runs much faster with it disabled (all the saved creds I had for work 10.X websites - none of which are reachable from the Internet where the bitwarden server resides - leading to all the icon lookups timing out). It's a more "privacy aware" default - at the cost of having uglier/no icons
@dani-garcia commented on GitHub:
I don't think there is a great way to solve this issue, to be honest. Encrypting the filenames like you say wouldn't help much when the decryption key is also in the data folder, it would only make it a bit more slow.
We could hash the domains with SHA-256 or similar, but that would only really help a little, as you could always get a list of domain names from something like https://zonefiles.io/all-registered-domains/, and hash them all, then compare against the local files. My calculations say it won't take more than a couple of days to hash the entire 300 million domains in that file.
@dani-garcia commented on GitHub:
That would be a good feature request for the clients themselves, as they are the ones controlling whether to download the icons or not. I think at least making the option per user instead of per client would help a lot.
As it is the only thing we can do is either allow everyone to download the icons or to disable them for all.
@BlackDex commented on GitHub:
Not that this will fix the storing of the icon's in plain text on the server it self.
But there now is an option to blacklist non global IP's.
ICON_BLACKLIST_NON_GLOBAL_IPS=trueThis is default true, and will prevent local/internal domain/ip discovery by using this.
Other then letting the users decide for them self, you can disable it server wide as @mprasil posted.
Also, note that bitwarden_rs is downloading the icon's independently from bitwarden.com, and that the issue described on the support page of bitwarden.com which @jhaar posted will only be on the own server, and not via any CDN or bitwarden.com.
@mprasil commented on GitHub:
Just to expand on what @dani-garcia said, you can disable icon fetching server-wide by setting the
DISABLE_ICON_DOWNLOAD=trueenvironment variable. The clients will still make the request, but server won't try to fetch the icon.@mprasil commented on GitHub:
I think we can close this now? For users really concerned about storing the icons, there's already an option to disable icon loading either client side or server-wide. Hashing the icon files makes very little sense, considering the icons themselves contain the logo of the page and the address space is so small that it can be brute forced in couple of days anyway.