Detect inline keys config

[pierre-borckmans]

Currently, the plugin only considers sops files if a .sops.yaml is found in one of the parent directories.

However, sops also works with standalone files inlining their keys config, like:

sops_gcp_kms__list_0__map_created_at=2022-02-16T19:19:15Z
sops_gcp_kms__list_0__map_enc=...
sops_gcp_kms__list_0__map_resource_id=projects/xyz/locations/global/keyRings/abcd

This PR allows for the 2 scenarios detection:

• as before, a simple sops file + a sops config yaml in any of the parent dirs
• [new]: a standalone sops file with any of the supported (taken from here: https://github.com/getsops/sops/blob/d8e8809bf92a47e0b2f742b9a43c3ab713acfd6a/stores/stores.go#L64)

[pierre-borckmans]

Note that I didn't find where the README of the plugin page is to update it, so I guess this is part of the plugin release process with JetBrains.

Ideally, if this gets merged, this blob should be updated to add the 2 scenarios:

[DaPutzy]

Hi, thanks for the contribution! I have a full weekend but i will have closer look at the beginning of next week.

But something from the top of my head: Have you tested this? Because from memory the issue is not that you cant derypt the file, but that the plugin doesnt know how to encrypt it again since all this information is lost during decryption.

I have two ideas spinning in my head:

• we could add an option just two decrypt i.e. view instead of edit
• we could extract the information before decrypting and "store" it so that we can tell sops how to encrypt once the user finished editing the file.

Note that I didn't find where the README of the plugin page is to update it, so I guess this is part of the plugin release process with JetBrains.

Yes this is maintained via the plugin marketplace.

[pierre-borckmans]

Hi there, thanks for your feedback!

You are correct I didn't try the edit functionality, only the decryption.
I'll have a look at the options you propose:

• Read-only (decrypt)
• Edit with some kind of store for the inline keys config

[pierre-borckmans]

I found a way to do the replace using the inline metadata.

Admittedly a little hacky but it works nicely:

• I'm leveraging the fact that when you call sops file... it runs in edit mode, and you can pass the editor you want via the EDITOR env var:
• I'm writing the new unencrypted content to a temp file
• writing a adhoc temp script that replaces the content of the temp file that SOPS passes to the editor with the new content

The reason this is nice is that I don't need to handle all the possible keys schemes that sops manages, we let sops use the normal edit flow. We simply replace the editor with a script that replaces the content.

This way, sops takes care of the decryption/encryption using either the yaml config or the inline metadata.

Note that it only works for mac/linux so far as I'm writing a temp bash script, but easily extendable for windows.

Curious to see what you think 😄

[DaPutzy]

The original version of this plugin was using IntelliJ as the sops editor, which was working fine for IntelliJ on macOS but neither with a different OS nor with a different Jetbrains product 😄

This solution does not care about which jetbrains product you use and only has the operating system to consider.

I will have good think about what implications this may have 🤔

[pierre-borckmans]

Interesting to hear about the history of the plugin 😄

The only os-dependent bit here is to have an EDITOR script that simply copies a file.
That should probably be doable?

I'm not versed into windows stuff for a long time now, but the current solution should work for mac/linux.

In any case, let me know if I can help

[DaPutzy]

After attacking the other PR today i could not let go and after hours of pain and misery i got it working under windows 🥳

Not sure if this is its final form because i can no longer think straight. Will have another look tomorrow if i can make sense of what i wrote today.

[pierre-borckmans]

Fantastic!

I can't test the windows part, but it looks like you got it nicely covered.

I tested with your change, and my only nit is that the original file doesn't always refresh after re-encrypting.
I changed the file.refresh to be synchronous and then it works for me.

Let me know if that's ok.

[DaPutzy]

It always worked for me asynchronously but i have no strong feelings either way.

Ill probably adjust the exception handling a bit when i get the time, ill release the other PR first.

[DaPutzy]

TODOs:

• Replace should not use this new way of encryption because it is supposed to replace the entire file in case the private key got lost! ✔️
• finally add an actual README.md ✔️
• pwsh vs. powershell? ✔️
• rethink file detection, the new keys are always there but could all be null anyway ✔️
  • only check for .sops.yaml for replace action?
• always use UTF-8 for script files ✔️

[ProbstDJakob]

I for myself thought about fixing the problem of #11 with the EDITOR variable, but I didn't got time to do so and thus I am very happy that you had time to rewrite it. I only have some thoughts without looking deeply into your PR.

1. set -o pipefail is not available in posix shells, though /usr/bin/env sh might link to bash it is not guaranteed, but since you do not use any pipes, your fine with only set -eu.
2. It may be good to not copy the content of the decrypted file to prevent data from leaking. I do not know how sops deletes the temporary decrypted content, but I hope they delete it in a save way. This plugin at least does not. I do not mean to be rude or anything. This plugin made it easier to work with sops files and I am glad for it to exists. So back to the topic of why it is not as safe as it could be. Files being deleted the way you do it are only unlinked but the content of the file will stay on the hard drive until another file allocates exactly this block(s). So opening the file multiple times (sequentially), will create multiple temporary files which will be unlinked, but not erased. Since now there are multiple decrypted copies of the file on the hard drive, the probability of overwriting them by creating new files will decline and thus giving an attacker the possibility to (more or less) easily read the files. Yes this might sound unrealistic, but it is possible and if the effort is justified for the attacker it might be the way to go.
   To cut a long story short, it might be good to use the file created by sops without copying to not have to deal with deleting sensible files correctly. The ways I could think of to solve this are mainly possible on Linux, maybe on macOS too, but I am uncertain if they are possible on Windows at all.
   1. Keep the sops process open and create an "editor" which will pass the path of the decrypted file to JetBrains via an unix-socket (or for windows an tcp-socket) created by the IDE and when finished editing, a simple ok-message (or just closing the connection) will be sent back to the "editor". The problem with this solution is, that the created socket could be used by a malicious process to send a fake path. But I think it is unrealistic to be attacked this way, since the process could more easily search for the file in some temp directories instead. Furthermore depending on the implementation any second access to the socket may immediately receive a EOF or some other form of rejection, thus stoping the "editor" and therefore also sops. The only potential successful attack would be a newly created sops encrypted "empty" file which then will be populated via the IDE thus leaving the user unaware that the file they see is not managed by sops since it is also empty.
   2. Keep the sops process open and create an editor which uses the stdin/stdout as sockets similar to the first solution. The problem with this solution, the sockets are also used by sops and may be polluted with warnings or similar of sops. But the benefit would be, that there are no sockets to attack (unless using something like reptyr but then again there is no benefit over instead searching for the file).
   3. Decrypt the file and only use the content with in-memory IDE virtual files (if such exist - I do not have experience with the IDE plugin API). The problem with this solution is, how do you get the new content into the decrypted file provided by sops. Maybe by a solution similar to the ones above or by instead of passing the path to the IDE, passing the content to the stdin and the "editor" passes it to the file (something like cat /dev/stdin >"$1"). The benefits of this solution would be to not have a long running process and keeping the communication to the "editor" simple (and noise free). The downside is a possible race condition between sending the content and the "editor" being ready. But this could also be solved by passing a random string to the generated editor script like simple-sops-is-ready-hash and waiting for this string to appear on stdout/stderr as a whole line. The probability that such a line will be printed by sops as a warning is near zero I guess.
3. Printing out the decrypted content to the logs, even though it is only with the trace level, does not seem to be a good idea since the logs, at least if I am correct, will be saved to a file and therefore giving an attacker easy access to the content. Yes trace will most of the times not be enabled, but it is also not that unlikely. If it is crucial for debugging, that the content can be identified, a sha256 sum (or similar) could fulfil the same goal. Otherwise leaving it out might be the best solution.

I hope my thoughts are not to overwhelming and I do not want to say, that anyone here does a bad job, I am just sharing my two cents. It is up to you if any of those points are getting into this PR, end up in a separate issue or being thrown into the garbage.

Thank all of you for your work and have a nice day,
Jakob

[DaPutzy]

Quick answers:

1. good find, i just copied it from one of our shell scripts

2. security:

Decrypt the file and only use the content with in-memory IDE virtual files

Thats what we are already doing :)

The problem with this solution is, how do you get the new content into the decrypted file provided by sops

Exactly. Currently we write the in memory file into a temp file.
I will take another close look at our options when i have the time.

3. logging was leftover from yesterdays debugging session :)

Thanks for your insight! Thats exactly why i commented in all the old issues so that more people take a look and we end up with something worthwhile.

[DaPutzy]

FYI: getsops/sops#696

[DaPutzy]

I took another crack at it and got it working on macOS and Windows with environment variables instead of temp files, what do you think @ProbstDJakob and @pierre-borckmans?

[pierre-borckmans]

It looks nice, I guess the only caveat would be the size limit on env vars on windows/linux.

A quick google search seems to indicate:

• windows -> max 32,760 characters
• linux -> 128KB

[ProbstDJakob]

Besides having a content limit, there are other problems with this solution:

1. Even though environment variables on linux (and probably macOS too) support arbitrary data, I am not sure if this also applies to windows.
2. Passing secret data via command line arguments (echo "$SOPS_CONTENT" will be expanded and contain the secret) will result in the data being visible to other processes of the same user (or root) since they can be accessed via the procfs /proc/<pid>/cmdline (I do not know how they are handled on macOS and Windows). But in this case they wont show up since the echo being called in your script is a shell built-in and does not spawn a separate process (no entry in the procfs), so it is more or less just a nice fact to know.
3. When working with arbitrary data where you do not know what exactly is contained echo is a bad idea. echo, depending on the implementation, has the side effect of expanding things like \n, \t, and so on. This means that the data passed to echo may not be the same as written to the file and may result in a syntax regarding the data notation (json, yaml, env, ini, binary). A safer solution would be to use printf. In the case of your script it would be printf '%s' "SOPS_CONTENT". This also has the benefit of not adding a trailing newline, which is important for binary data (do not forget to add a \n (printf '%s\n' "$SOME_DATA") if you need it within other scripts).
4. Environment variables of a process are as visible as command line arguments via /proc/<pid>/environ (same as with number 2).

For a more detailed explanation see also https://stackoverflow.com/a/55804985.

A solution to those problems would be to use the stdin as proposed earlier. The following pseudo code demonstrates the process:

String identifier = "simple-sosp-edit-ready-" + Random.hash();
// It is up to you to find an equivalent for Windows ^^
String script = [
  "#!/usr/bin/env sh",
  "set -eu",
  "printf '%s\n' " + identifier, // will tell us that the script is ready; since the identifier does not contain any special characters, we do not need to escape the string (neither within java, nor the script)
  "cat - >\"$1\""
].join("\n");

try (Script script = writeTempScript(script)) { // The Script class implements Closeable which will deleted the file on close - see try-with-resources statement if the syntax is unknown
  Command sopsCommand = Command.prepareCommand("sops", /* some args */);
  sopsCommand.setEnvVar("EDITOR", script.path);
  Process sopsProcess = sopsCommand.run();

  for (String line : sopsProcess.stdout.stream()) { // a live stream of the stdout line per line without trailing new lines
    if (line.equals(identifier)) {
      sopsProcess.stdin.write(unencryptedFileContent);
      return;
    }
  }
  Logger.warning("Something went wrong and the content has not been written.");
}

It is a very long time since I last wrote something in Java, thus the code might look weird, but I think you will get what I am trying to "say". Also there may be some other flaws with this solution, therefore it should also be discussed.

[DaPutzy]

@ProbstDJakob got it working on Windows and macOS with stdin! Needs some clean up and refactoring still, but im gonna make use of the nice weather for once. Maybe ill have time to finish it this evening.

[DaPutzy]

There is still work to be done (and a lot of testing), but it is working!

[ProbstDJakob]

The current version looks fine to me. Only two questions. Have you tried it on Linux/macOS? printf "%s" does not include a new line (printf "%s\n") which could cause problems. Also does the escaping made for Windows scriptFiles.script().toAbsolutePath().toString().replace("\\", "\\\\") work on Linux/macOS too?

BTW: In your first version 5bb400d you had the following:

final String startIdentifier = "simple-sops-edit-ready" + RandomStringUtils.randomAlphanumeric(32);
// ...
"printf '%s'".formatted(startIdentifier),
// ...

This could randomly fail to work in cases were the RandomStringUtils.randomAlphanumeric(32) contains sequences printf would expand like \n, \t, %s, %d, and so on. This could be solved in two ways (possibly more):
Escaping the string:

import org.apache.commons.text.StringEscapeUtils;
// ...
"printf \"%s\n\"".formatted(StringEscapeUtils.escapeXSI(startIdentifier)), // note the double quotes around the printf argument, single quotes would cause other problems
// ...

Or even better by passing the string as an argument to printf:

import org.apache.commons.text.StringEscapeUtils;
// ...
"printf '%s\n' " + StringEscapeUtils.escapeXSI(startIdentifier),
// ...

Downside of this approach is the use of apache commons-text. This library drops the newline (source code - at least the code looks like, I have not tried it) instead of properly escaping it. So it may be reasonable to look for an alternative library, because this would cause also random breaks, but I just want to give an example.

The current version does not suffer from those problems, since there are no unknown characters and the string does not contain any sequences which could cause problems.

[DaPutzy]

Have you tried it on Linux/macOS? printf "%s" does not include a new line (printf "%s\n") which could cause problems.

Worked on Windows and macOS, have not yet tested on Linux.

Also does the escaping made for Windows scriptFiles.script().toAbsolutePath().toString().replace("\", "\\") work on Linux/macOS too?

Works on macOS, have not tested linux. They use a different path separator though so it should be fine? Im unhappy with the current solution as well, since there could be an edge case. Ill check if i can find a more universal solution.

This could randomly fail to work in cases were the RandomStringUtils.randomAlphanumeric(32) contains sequences printf would expand like \n, \t, %s, %d, and so on.

It should only contain alphanumerics though?

[DaPutzy]

Released as a pre-release on github and as a beta via jetbrains marketplace.

Ill ask friends and colleagues to test, but same as before: your input is highly appreciated.

Please let me know if the README.md makes any sense to you.

PS: You can add https://plugins.jetbrains.com/plugins/beta/list as a plugin repository or install via file.

[ProbstDJakob]

Works on macOS, have not tested linux. They use a different path separator though so it should be fine? Im unhappy with the current solution as well, since there could be an edge case. Ill check if i can find a more universal solution.

Maybe something like:

String editorPath = scriptFiles.script().toAbsolutePath().toString();
if (SystemUtils.IS_OS_WINDOWS) {
	// escape twice for windows because of ENV variable parsing
	editorPath = editorPath.replace("\\", "\\\\")
}
// escape whitespaces
editorPath = editorPath.replace(" ", "\\ ");

It should only contain alphanumerics though?

Reading correctly seems to be very hard ^^

Please let me know if the README.md makes any sense to you.

It makes sense, though I might not be the right person to check if the grammar and syntax is correct (I am a non-native). But there is a typo in line 24 stating "sops will use the file ending of the existing file to figure our the file type" which should be out. Furthermore I do not think the <br>s are need since GitHub itself inserts line breaks where necessary, but you probably had a reason :)

Last think to note, but that has not much to do with the README itself, is that the Replace option may be better named with something like Encrypt with sops (unless I misunderstood the feature - have not tested it yet).

Released as a pre-release on github and as a beta via jetbrains marketplace.

I will look into it, but maybe not before next week.

[mwtrew]

I've just tried it too (also on macOS), works like a charm and solves the problem from #11. Thank you both very much!

[pierre-borckmans]

Just wanted to give a quick update.

I've been using the 2.0.0 beta for a while on a daily basis and it works like a charm for my needs.
Thanks again for this... quite a journey from my original PR 😛

[DaPutzy]

Im currently swamped, ill finalize the release as soon as i find the time.

[DaPutzy]

Github release is no longer a pre-release and I published to Jetbrains Marketplace (approval pending) 🥳