Skip to content
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

Add view menu option to allow screenshots for temporary period of time #7580

Closed
sascha224 opened this issue Mar 22, 2022 · 36 comments · Fixed by #8841
Closed

Add view menu option to allow screenshots for temporary period of time #7580

sascha224 opened this issue Mar 22, 2022 · 36 comments · Fixed by #8841
Assignees
Milestone

Comments

@sascha224
Copy link

Overview

KeePass 2.7.0 main window is invisible in a remote session with TeamViewer or AnyDesk --> problem with new feature "Prevent screen capture"?

Steps to Reproduce

  1. Connect to your remote pc with TeamViewer or AnyDesk
  2. Open KeePassXC on the remote machine
  3. The program opens in tray, but the window is not visible. On screen of the remote machine, the window is visible normally, but not in remote software window.

Expected Behavior

The program should be usable with a remote software like TeamViewer or AnyDesk. Optionally, if this conflicts with the screen capture prevention feature, it should be possible to disable this feature.

Actual Behavior

The main window keeps invisible.

KeePassXC - Version 2.7.0
Revision: d7a9ef4

Operating System: Windows 10 and 11

@sascha224 sascha224 added the bug label Mar 22, 2022
@droidmonkey
Copy link
Member

https://keepassxc.org/docs/KeePassXC_UserGuide.html#_screenshot_security

@octylFractal
Copy link

Is there a chance of making this a configurable checkbox in the settings? I'm unsure of how to pass the flag to the automated start-up of KeePassXC, and I really do need to disable this all the time as I am using Windows in a VM and it is always considered "screen capture", so it's impossible for me to use KeePassXC now.

@sascha224
Copy link
Author

Is there a chance of making this a configurable checkbox in the settings? I'm unsure of how to pass the flag to the automated start-up of KeePassXC, and I really do need to disable this all the time as I am using Windows in a VM and it is always considered "screen capture", so it's impossible for me to use KeePassXC now.

+1

@droidmonkey droidmonkey reopened this Mar 23, 2022
@droidmonkey droidmonkey changed the title KeePass 2.7.0 main window is invisible in remote software like TeamViewer or AnyDesk Make screenshot protection a configuration option Mar 23, 2022
@droidmonkey
Copy link
Member

We specifically did not make this a configuration option because you can "forget" it is allowed and spill information. You can easily add the command line parameter to the auto-start definition in the registry: HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Run look for the KeePassXC entry

@tunbridgep
Copy link

Most other "dangerous" settings (like running a beta version) tend to create banners at the top of the window, so a "Screenshots are enabled!" banner could probably fix the "you forgot to turn screenshot protection back on" problem

@phoerious
Copy link
Member

A "Screenshots are enabled!" banner is exactly what you don't want if you intend to make screenshots of KeePassXC.

@michaelk83
Copy link

michaelk83 commented Mar 29, 2022

A "Screenshots are enabled!" banner is exactly what you don't want if you intend to make screenshots of KeePassXC.

Unless you can close it. Then the banner reminds, user closes it and continues as usual. I'd also argue that it's far easier to forget a modified registry entry or modified shortcut than a setting. The setting is more visible (though not by much).

@phoerious
Copy link
Member

I think users who have the need to disable screenshot protection can also be trusted to be knowledgeable enough to disable it again if they don't need it anymore.

@douglasg14b
Copy link

douglasg14b commented Apr 21, 2022

@droidmonkey If you're worried about it being forgotten, then have a timer since it's probably temporary anyways?

Pulling control out of the user's hands isn't exactly a great option here, and guaranteed the registry key will be much more easily forgotten than a checkbox as googlers end up here.

Security at the cost of convenience comes at the cost of security and all

I just want to take a one-off screenshot -_-

@droidmonkey
Copy link
Member

droidmonkey commented Apr 21, 2022

Pulling control out of the user's hands isn't exactly a great option here, and guaranteed the registry key will be much more easily forgotten than a checkbox as googlers end up here.

You have full control with the command line flag. Actually happy we didn't just have a config option because those using remote desktop and such would have to edit the ini config file to set the option.

If you want to take screenshots just make a shortcut that has the command line flag and open keepassxc using that shortcut.

@svogt
Copy link

svogt commented Apr 24, 2022

Is it possible to change this setting at runtime of the app? In that case a "hotkey" might be a good compromise. I have other software which toggles the screen capture by hitting "print screen" with a dialog if the app should allow screen capture and shift-print-screen disables it again.

Also helps a lot when trying to help someone via TeamViewer if I can just tell them to hit print screen...

@droidmonkey
Copy link
Member

droidmonkey commented Apr 24, 2022

Yikes. That is precisely what we are trying to prevent with this. Remote support inadvertently or purposefully exposing the secrets of the end user through remote desktop or screensharing software. If your users are conditioned to let you see their password database then they won't hesitate to do so when social engineered.

Tell them to read the user guide for assistance. If it's not clear, tell us so we can make edits.

@svogt
Copy link

svogt commented Apr 25, 2022

Well users in that case are my parents and grand parents which I'm trying to get away from birthdays and six letter passwords for dozens of websites. So no, telling them to RTFM won't help here.

@Wallhallaaaaa
Copy link

I'm reading this discussion right now and am puzzled by the reasoning. For example I have the problem that until now I have not been able to start KeePassXC on the Mac with the --allow-screencapture parameter. Maybe I'm just too stupid. If yes, please help. I also help many users via Teamviewer and it was me who informed the users about the meaning of good passwords and password managers. Still, many find it difficult to deal with. That hasn't been a problem so far because I was always able to help. I support the users with it whenever they have a problem. Unfortunately, this is no longer possible. I personally think that everyone should decide for themselves whether or not to allow screencapturing. And I can't understand the argument that the users I help and you let me into your password manager don't bother about their passwords or become careless, exactly the opposite is the case. If there is no option to bypass or switch off this lock on the Mac in the foreseeable future, even if only temporarily and / and with a clear warning, then the program will no longer be useful for me and many of my supervised users. If there is this option on the Mac, I would be grateful for a hint how I can implement it without forcing the unsuspecting user to start the program with parameters that don't seem to work via the shell. Please reconsider adding it as an option. It would be a pity if users had to save their access data in their browser again in the future due to the lack of an alternative. I'm curious if you can offer a solution.

@droidmonkey
Copy link
Member

Again I reiterate that if our documentation is not sufficient then let us know. Until we make a change to allow this from the view menu, you'll have to have them start with the command line switch.

@JohnLGalt
Copy link

JohnLGalt commented May 9, 2022

I think that most people should be able to sufficiently do it if they follow a few easy, simple steps. (The following is aimed primarily at Windows users, but similar functionality should be applicable across all OSs, even if the methodology is different).

  1. Make a copy of the existing shortcut.
  2. Rename the new shortcut to name of choice, indicating SS enabled.
  3. Edit the path / target to include the argument.

Et voila - 2 shortcuts, one for no SS mode and one for SS mode.

@droidmonkey
Copy link
Member

Even better if you manage enterprise systems you can push the screenshot enabled shortcut via GPO.

@igpit
Copy link

igpit commented May 10, 2022

I do understand the security considerations but NOT revealing some options through the UI seems to be a bad design decision, making life hard for people who need it.
Another suggestion: In the options "for Brower integration" there is a separate tab "Advanced" hiding the dangerous options, there is even a warning banner permanantly shown.
How about a tab "expert/dangerous options" with permanent warning banner, that should enable a good compromise.

@fathermaurer
Copy link

fathermaurer commented May 16, 2022

Just a note to say thank you for this thread - I hadn't thought to look in HKEY_CURRENT_USER and was running in circles trying to find the KeePassXC entry in 'Run' (I kept looking in HKEY_LOCAL_MACHINE ¯\_(ツ)_/¯)

The KeePassXC documentation IS very complete. I don't know if it there might be unintended consequences, but having the registry path shared there, but it might be helpful to do so.

In any case, thank you for your time here.

@meagle22
Copy link

Hello all. When adding the flag and trying to run from command line, I am getting an error: Unknown option 'allow-screenshare'. Do I have the flag wrong? I am on Windows and following instructions that go like this:

"Create a desktop shortcut for KeePassXC.exe, and open its Properties window. Add the following argument at the end of the Target field, --allow-screencapture. Hit OK, and the program will let you capture screenshots of the interface."

The command not recognized.

Any ideas?

@fathermaurer
Copy link

fathermaurer commented May 19, 2022

The flag should be outside the program path - which itself should be in quotes:

"C:\installpathtoexecuteable\KeePassXC.exe" --allow-screencapture

@meagle22
Copy link

Thanks so much for responding. This doesn't seem to solve my problem. Here is what I have in the Target field of the shortcut:
"C:\Program Files\KeePassXC\KeePassXC.exe" --allow-screencapture (This is a correct path for sure)

Error when trying to run the app by double clicking on the shortcut is:
Unknown option --allow-screencapture.

When I remove the flag, the application runs from that short cut. Any other suggestions? Thanks again for your help,

@meagle22
Copy link

So, it finally worked out. I just had to update to the last version of KeePassXC and follow the instruction that fathermaurer provided. Thank you for your help fathermaurer.

@h-h-h-h
Copy link

h-h-h-h commented May 20, 2022

@fathermaurer: I recommend the software Autoruns, which lists autostart apps froms every possible location.

@contextnerror
Copy link

Again I reiterate that if our documentation is not sufficient then let us know. Until we make a change to allow this from the view menu, you'll have to have them start with the command line switch.

The documentation is not sufficient. I struggled for ages trying to figure out how to do it for a bug report and ultimately failed since none of the instructions are applicable for a Mac. When the only available documentation tells you to run "keepassxc.exe" in the command line you're already off to a bad start.

@yi0n
Copy link

yi0n commented Aug 31, 2022

Today I discovered that if you have KeepassXC running in a RDP session, resizing the RDP-Window on the host and than take a screenshot on the host, makes screenshots from opened Keepass window possible. Even if keyboard and clipboard are shared with the RDP session. Keepass is unable to detect that.

@phoerious
Copy link
Member

We won't make this a configuration option, since it kind of defeats the purpose. We might install a secondary launcher by default, but generally, this is an expert option.

@phoerious phoerious closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2022
@phoerious phoerious removed this from the v2.7.2 milestone Oct 18, 2022
@droidmonkey
Copy link
Member

Actually I was thinking of making a "Allow screenshots for 5 minutes" or something in the view menu so people can screenshot their database for bug reports.

@droidmonkey droidmonkey reopened this Oct 18, 2022
@droidmonkey droidmonkey changed the title Make screenshot protection a configuration option Add view menu option to allow screenshots for temporary period of time Oct 18, 2022
@droidmonkey droidmonkey added this to the v2.8.0 milestone Oct 18, 2022
@phoerious
Copy link
Member

Would that actually be so useful, though? From what I can tell, most people having issues with this seem to be looking more for a permanent solution for their RDP setup. Making screenshots is probably more of a niche use case.

@igpit
Copy link

igpit commented Oct 18, 2022

We won't make this a configuration option, since it kind of defeats the purpose.

How so? Exposing the option via UI is no more or less secure than having a command line paramter. Besides, there are already "expert" options for potentially unsafe configuration. It is always up to the user how to use the product given.

droidmonkey added a commit that referenced this issue Nov 25, 2022
* Fix #7580
* Also refactor the code to move everything into MainWindow
droidmonkey added a commit that referenced this issue Nov 25, 2022
* Fix #7580
* Also refactor the code to move everything into MainWindow
@gpetrina
Copy link

gpetrina commented Dec 9, 2022

The command line argument option doesn't exist in the current build? Also the documentation for running from command line is not comprehensive enough. keepassXC.exe is not a recognized command, using "C:\Program Files\KeePassXC\KeepassXC.exe" --Allow-screencapture gives unrecognized option. Putting the option in the registry entries only results in an immediate error.

Please either add the configuration option your users are asking for or actually make the application behave how it's meant to so that the user can actually use the program how they intend.

@h-h-h-h
Copy link

h-h-h-h commented Dec 10, 2022

--Allow-screencapture gives unrecognized option... actually make the application behave how it's meant to [@gpetrina]

Why do you use a capital A in the option and expect this to work? npm --Version, git --Version, yt-dlp --Version all don't work. The app behaves correctly and as documented.

@gpetrina
Copy link

--Allow-screencapture gives unrecognized option... actually make the application behave how it's meant to [@gpetrina]

Why do you use a capital A in the option and expect this to work? npm --Version, git --Version, yt-dlp --Version all don't work. The app behaves correctly and as documented.

For the most part, CLI is not the same as Powershell which dictates case sensitivity when necessary. Because CLI is usually not case sensitive, I had no reason to expect that a capitalization would affect the use of the option as there's no documentation that explicitly says that this is the case. As I said prior, the documentation needs either updated or there should be a configuration option so that simple issues like this are avoided.
And again, it does not work as documented. In the same link you posted, it shows usage to be "Usage: keepassxc.exe [options] [filename(s)]", simply putting keepassxc.exe does not result in a recognized command unless you navigate to the folder first, so the usage should read "Usage: 'C:\YourProgramFilePath\keepassxc.exe' [options] [filenames(s)]".

@svengreb
Copy link

svengreb commented Dec 12, 2022

For the most part, CLI is not the same as Powershell…

@gpetrina I guess you are not aware what CLI means and what PowerShell is. CLI stands for Command Line Interface which is something defined by every application itself, when supported at all. PowerShell is just a shell, an interpreter for a script DSL, so you can say it interprets a kind of programming language where on functionality is to run executable files.
With that said

Powershell which dictates case sensitivity…
makes not sense at all because everything passed to an executable is interpreted by the application's CLI logic, e.g. flags and arguments. Therefore of course the application defines the way how flags are named, including case sensitivity because these are just simple strings where the a is another character code as A.
So the documentation is correct because the application is the owner of this option and therefore “dictates“ the naming and case-sensitivity.

The documentation is also correct for the usage instructions because the installation path can of course be changed by the user or the application can be used without an installation at all (portable). I could download the application and place it in a downloads folder, change into this directory with ANY shell (because all of the can run executables which is just a syscall, but too much detail here) and make use of the KeePassXC CLI from there.
The documentation is scoped to the CLI, not the (dynamic) environment it is placed in, and simply documents all features the CLI provides.

…the usage should read "Usage: 'C:\YourProgramFilePath\keepassxc.exe' [options] [filenames(s)]".

This is an absolute path to where your executable is placed, but would be totally wrong in a application documentation.

But all of this text is just a quick way to say:
Please don't try to use such features if you're not familiar with such basic concepts at all. This is why features like this are placed behind these kind of barriers to keep the "normal" users from shooting themself in the foot.

And the real reason I wrote this: Please ask such questions in dedicated community places like a forum, GitHub discussion thread or (as a last resort) open an issue, but this problem has nothing to do with the discussion in this issue about providing this option outside the of CLI too. Your post triggers notifications for many devs that subscribe to this issue to stay up-to-date about the discussion while side-track topics like this can be annoying when you receive 50+ notifications per day.

@keepassxreboot keepassxreboot locked and limited conversation to collaborators Dec 12, 2022
droidmonkey added a commit that referenced this issue Feb 14, 2023
* Fix #7580
* Also refactor the code to move everything into MainWindow
droidmonkey added a commit that referenced this issue Feb 18, 2023
* Fix #7580
* Also refactor the code to move everything into MainWindow
droidmonkey added a commit that referenced this issue Feb 18, 2023
* Fix #7580
* Also refactor the code to move everything into MainWindow
droidmonkey added a commit that referenced this issue Feb 25, 2023
* Fix #7580
* Also refactor the code to move everything into MainWindow
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.