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

Define in the user preferences a list of file hashes to be trusted when jar files are added to the classpath of jdt.ls #1965

Closed
sunix opened this issue Jun 1, 2021 · 9 comments · Fixed by #2087

Comments

@sunix
Copy link

sunix commented Jun 1, 2021

[provide a description of the issue]

Environment
  • Operating System:
  • JDK version:
  • Visual Studio Code version: Che-theia
  • Java extension version:
Steps To Reproduce

When we add a jar file in the classpath like

      java.jdt.ls.vmargs: '-javaagent:/lombok.jar'

a Security Warning is popping up and this is an issue in Che the option is not persisted in the user preferences (but in the global state which is gone when starting a new workspace).

image

For security reasons, we would like to provide a list of jar files that can be trusted in the preferences.
Would be something similar to https://github.com/redhat-developer/vscode-xml/blob/985705ebd603bd08ee8ca149484d3ea48841007a/docs/Preferences.md#trusted-binary-hashes

[Please attach a sample project reproducing the error]
Please attach logs

Current Result
Expected Result
Additional Informations
@rgrunber
Copy link
Member

rgrunber commented Jun 1, 2021

Just a few workaround to consider before we look at providing some kind of trusted resource store :

  1. Can you provide javaagent argument to java.jdt.ls.vmargs under user settings (not workspace settings) ?

As you've discovered in https://github.com/redhat-developer/vscode-java/blob/master/src/settings.ts#L141-L158 , when the workspace settings contain javaagent in java.jdt.ls.vmargs, we bring up a popup to warn the user. However, if the setting comes from the user settings, we allow it. On most VSCode instances, this would be under ${userDataDir}/User/settings.json, and ${userDataDir} usually defaults to $HOME/.config/Code/. I'm able to get javaagent accepted as an argument without any prompt using this approach. If so, you'd probably just need to figure out what corresponds to ${userDataDir} in your environment.

  1. Can you provision the contents of globalState ?

From https://github.com/redhat-developer/vscode-java/blob/master/src/settings.ts#L141-L158 , the property written to globalState to skip the security check when javaagent is defined in workspace settings, is :

key = redhat.java
value = {"java.ls.isVmargsAllowed::${workspacePath}/jdt_ws::${javaAgentFlag}":true}

where the globalState is usually the sqlite database at ${userDataDir}/User/globalStorage/state.vscdb, ${workspacePath} would be something like /tmp/code-tmp/config/User/workspaceStorage/d7f5a8f370269b38c3449ecf3ba23efb/redhat.java and ${javaAgentFlag} would be something like -javaagent:/tmp/code-tmp/lombok.jar

I'm fairly certain (2) isn't easy to do, but (1) seems possible. If neither work, we can probably look at some kind of trustedHash setting like we do for vscode-xml.

@sunix
Copy link
Author

sunix commented Jun 1, 2021

hi @rgrunber.
About the user preferences workaround: the user could provide it but we can't pre-configure it in Che. In Che we can only preconfigure workspace settings. We could hack it, but the problem with user settings is that it would affect all the workspaces that may not use this jar (and the jar may not even be available). As far as I remember, this is what is done with the Lombok extension and this is why this couldn't be used in Che. In Che, user settings are shared through all the workspaces that belong to a user. Each workspace could have different file system or extensions loaded.

About the global storage workaround, we could hack it, but I doubt my PR would be accepted ;)

@rgrunber rgrunber removed the need info label Jun 1, 2021
@rgrunber
Copy link
Member

rgrunber commented Jun 1, 2021

About the user preferences workaround: the user could provide it but we can't pre-configure it in Che. In Che we can only preconfigure workspace settings. We could hack it, but the problem with user settings is that it would affect all the workspaces that may not use this jar (and the jar may not even be available).

I see a problem with this though (unless I'm missing some piece of info). If you can only configure workspace settings, then we have to allow "trustedHashes" as a workspace setting. But then any workspace could just provide a settings file that self-approves its own content. The purpose of user settings is to prevent this.

Even if you look in https://github.com/redhat-developer/vscode-xml/blob/master/package.json#L147 (for the xml trustedhashes), we don't allow this to be set as a workspace setting (application scope).

@sunix
Copy link
Author

sunix commented Jun 1, 2021

I don't think the workspace scope is about self approving or not. it's would just apply the settings for a workspace which is a collection of folders (aka projects). So basically, user settings apply in all workspaces, but workspace settings is only about the current workspace. I don't see the problem with that. We provide preconfigured workspaces that the user can view, edit and customize.
User settings are like, global settings for 1 user. Workspace settings are the settings for a particular workspace with project1 (quarkus), project2(node) and project3 (java-lombok). We just want to trust and load the jar for these workspaces.

Having that said, for our usecase, maybe it should be done in the application scope. But I am not sure it is implemented on our side.

@rgrunber
Copy link
Member

rgrunber commented Jun 2, 2021

The case we're trying to guard against is a user importing a folder (eg. git cloned) they don't fully trust, that contains a .vscode/settings.json file and sets properties such as java.home, or java.jdt.ls.vmargs. If not guarded, they could execute arbitrary code. This is why we explicitly ask the user to allow such usage.

Is such a case possible in Che if a user requests to create their workspace from a cloned project that contains a settings file at the root ?

@sunix
Copy link
Author

sunix commented Jun 2, 2021

if this is the only use case, could we just show this popup if the jar file is located in the project folders ?
or could we have predefined folders in the file system where the jars to be trusted would be located ?

@sunix
Copy link
Author

sunix commented Jun 2, 2021

In Che we load everything from a devfile.yaml file that is located in the root project anyway. The difference is that all these are running in isolated pod in the cloud and not in your local laptop with all your sensitive personal data.
We also have preconfigured devfiles. There are certified "devfiles" in CodeReady Workspaces and other "community" devfiles from the Che community. It is up to the user to trust them or not. It would be like running container from Dockerhub ... could you trust all of them ?

@rgrunber
Copy link
Member

rgrunber commented Jun 2, 2021

I think a good compromise would be what you suggested, to confirm that the path provided by javaagent corresponds to a file that is not provided by the workspace.

@sunix
Copy link
Author

sunix commented Jun 2, 2021

Thanks Roland, that would work for us

rgrunber added a commit to rgrunber/vscode-java that referenced this issue Aug 27, 2021
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit to rgrunber/vscode-java that referenced this issue Aug 27, 2021
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit to rgrunber/vscode-java that referenced this issue Aug 27, 2021
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit to rgrunber/vscode-java that referenced this issue Aug 31, 2021
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit to rgrunber/vscode-java that referenced this issue Sep 1, 2021
- Do not show prompt if the workspace is already trusted
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit to rgrunber/vscode-java that referenced this issue Sep 1, 2021
- Do not show prompt if the workspace is already trusted
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit to rgrunber/vscode-java that referenced this issue Sep 2, 2021
- Fixes redhat-developer#1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit that referenced this issue Sep 7, 2021
- Fixes #1965

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber rgrunber added this to the Mid September 2021 milestone Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants