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

storage: improve tooling for decrypting files in situ #89095

Closed
3 tasks done
nicktrav opened this issue Sep 30, 2022 · 2 comments
Closed
3 tasks done

storage: improve tooling for decrypting files in situ #89095

nicktrav opened this issue Sep 30, 2022 · 2 comments
Assignees
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-storage Storage Team

Comments

@nicktrav
Copy link
Collaborator

nicktrav commented Sep 30, 2022

Is your feature request related to a problem? Please describe.

We've had a few scenarios where files we've needed access to (namely Pebble manifest files) have been encrypted. This hampers our ability to debug. Decrypting the file is an involved process that relies on access to both the store key, but also the data keys and the file encryption registry - files that shouldn't leave a deployment environment.

Describe the solution you'd like

Jira issue: CRDB-20113

Epic CRDB-20293

@nicktrav nicktrav added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-storage Relating to our storage engine (Pebble) on-disk storage. T-storage Storage Team labels Sep 30, 2022
@nicktrav
Copy link
Collaborator Author

While I was spelunking Cockroach code, I found enc_utils, which gives a good starting point for the tool outlined above. However, it no longer seems to work - issues unmarshalling the file registry back into its proto form. We've had that issue elsewhere, so I assume it should be a similar fix.

nicktrav added a commit to nicktrav/pebble that referenced this issue Oct 6, 2022
Currently, the tools write directly to `os.{Stdout,Stderr}`, which
complicates testing in the case where the command is run from same
process running the tests (i.e. from a `testing.T` func).

Adapt the tools to make use of the return values from
`(*cobra.Command).{OutOrStdout,OutOrStderr}`. In testing scenarios, the
tools can use `SetOut` and `SetErr` to pass in a writer that can
intercept anything written to stdout / stderr. In productions scenarios,
the tools will emit to the appropriate channel.

Touches cockroachdb/cockroach#89095.
nicktrav added a commit to cockroachdb/pebble that referenced this issue Oct 10, 2022
Currently, the tools write directly to `os.{Stdout,Stderr}`, which
complicates testing in the case where the command is run from same
process running the tests (i.e. from a `testing.T` func).

Adapt the tools to make use of the return values from
`(*cobra.Command).{OutOrStdout,OutOrStderr}`. In testing scenarios, the
tools can use `SetOut` and `SetErr` to pass in a writer that can
intercept anything written to stdout / stderr. In productions scenarios,
the tools will emit to the appropriate channel.

Touches cockroachdb/cockroach#89095.
nicktrav added a commit to nicktrav/cockroach that referenced this issue Oct 10, 2022
During storage-level L2 investigations, files from problematic stores
are often requested (e.g. the MANIFEST file(s), SSTables, etc.). In
cases where the store is using encryption-at-rest, the debug artifacts
are useless unless they have been decrypted.

Add a new debug command that can be used to decrypt a file in-situ,
given the encryption spec for the store, and a path to an encrypted file
in the store. For example:

```bash
$ cockroach debug encryption-at-rest \
  path=/path/to/store,key=/path/to/aes.key,old-key=plain \
  /path/to/encrypted/file \
  /path/to/decrypted/output/file
```

Touches cockroachdb#89095.

Release note (ops change): A new debug tool was added to allow for
decrypting files in a store using encryption-at-rest. This tool is
intended for use while debugging, or for providing debug artifacts to
Cockroach Labs to aid with support investigations. It is intended to be
run "in-situ" (i.e. on site), as it prevents having to move sensitive
key material.
nicktrav added a commit to nicktrav/cockroach that referenced this issue Oct 12, 2022
During storage-level L2 investigations, files from problematic stores
are often requested (e.g. the MANIFEST file(s), SSTables, etc.). In
cases where the store is using encryption-at-rest, the debug artifacts
are useless unless they have been decrypted.

Add a new debug command that can be used to decrypt a file in-situ,
given the encryption spec for the store, and a path to an encrypted file
in the store. For example:

```bash
$ cockroach encryption-decrypt \
  /path/to/store \
  /path/to/encrypted/file \
  /path/to/decrypted/output/file
```

Touches cockroachdb#89095.

Release note (ops change): A new debug tool was added to allow for
decrypting files in a store using encryption-at-rest. This tool is
intended for use while debugging, or for providing debug artifacts to
Cockroach Labs to aid with support investigations. It is intended to be
run "in-situ" (i.e. on site), as it prevents having to move sensitive
key material.
nicktrav added a commit to nicktrav/cockroach that referenced this issue Oct 12, 2022
During storage-level L2 investigations, files from problematic stores
are often requested (e.g. the MANIFEST file(s), SSTables, etc.). In
cases where the store is using encryption-at-rest, the debug artifacts
are useless unless they have been decrypted.

Add a new debug command that can be used to decrypt a file in-situ,
given the encryption spec for the store, and a path to an encrypted file
in the store. For example:

```bash
$ cockroach debug encryption-decrypt \
  --enterprise-encryption=$encryption_spec \
  /path/to/store \
  /path/to/encrypted/file \
  /path/to/decrypted/output/file
```

Touches cockroachdb#89095.

Release note (ops change): A new debug tool was added to allow for
decrypting files in a store using encryption-at-rest. This tool is
intended for use while debugging, or for providing debug artifacts to
Cockroach Labs to aid with support investigations. It is intended to be
run "in-situ" (i.e. on site), as it prevents having to move sensitive
key material.
craig bot pushed a commit that referenced this issue Oct 12, 2022
89668: pkg/cliccl: add `debug encryption-decrypt` command r=jbowens a=nicktrav

During storage-level L2 investigations, files from problematic stores
are often requested (e.g. the MANIFEST file(s), SSTables, etc.). In
cases where the store is using encryption-at-rest, the debug artifacts
are useless unless they have been decrypted.

Add a new debug command that can be used to decrypt a file in-situ,
given the encryption spec for the store, and a path to an encrypted file
in the store. For example:

```bash
$ cockroach debug encryption-decrypt \
  --enterprise-encryption=$encryption_spec \
  /path/to/store \
  /path/to/encrypted/file \
  /path/to/decrypted/output/file
```

Touches #89095.

Epic: None.

Release note (ops change): A new debug tool was added to allow for
decrypting files in a store using encryption-at-rest. This tool is
intended for use while debugging, or for providing debug artifacts to
Cockroach Labs to aid with support investigations. It is intended to be
run "in-situ" (i.e. on site), as it prevents having to move sensitive
key material.

Co-authored-by: Nick Travers <[email protected]>
nicktrav added a commit to nicktrav/cockroach that referenced this issue Oct 12, 2022
The existing `enc_util` package contains a tool that could be used to
dump the files in an encryption registry. This command has been broken
since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of
files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files
contained in the registry of an encrypted store. This is useful for
debugging which store / data key was used to encrypt each file,
replacing the equivalent functionality in `enc_util`.

Touches: cockroachdb#89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an
operator to list the files present in the Encryption-At-Rest file
registry.
nicktrav added a commit to nicktrav/cockroach that referenced this issue Oct 12, 2022
Remove an obsolete, broken command. The functionality has been
superseded by cockroachdb#89095 and cockroachdb#89873.

Touches cockroachdb#89095.
Epic: None.
Release note: None.
craig bot pushed a commit that referenced this issue Oct 13, 2022
89874: cmdccl: remove `enc_utils` r=jbowens a=nicktrav

Remove an obsolete, broken command. The functionality has been superseded by #89095 and #89873.

Touches #89095.
Epic: None.
Release note: None.

89905: multitenant: fix failing distsql test r=cucaroach a=cucaroach

Test was erroneously expecting a bundle in the bundle=off config.

Fixes: #89720

Release note: None


89910: sql: `tree.ParseJSON` should not ignore trailing data r=miretskiy a=miretskiy

`tree.ParseJSON` uses `Decoder.More()` method to determine if the input contains trailing data.  The implementation made incorrect assumptions as to the reason why `Decoder.More()` allows input to contain `]` or `}` characters, even when JSON object has been consumed.

This PR fixes and comments those faulty assumptions, and fixes multiple existing tests that seemed to rely on those faulty assumptions.

In particular, prior to this PR, the following input would be allowed (i.e. extra end of object character `}`):
```
[email protected]:26257/movr> select '{"longKey1":"longValue1"}}'::jsonb;
            jsonb
------------------------------
  {"longKey1": "longValue1"}
(1 row)
```

But so would the following be allowed:
```
[email protected]:26257/movr> select '{"longKey1":"longValue1"}} should this data be ignored?'::jsonb;
            jsonb
------------------------------
  {"longKey1": "longValue1"}
(1 row)
```

So, the  issue is: if above conversion was executed to insert JSONB into the database, we would silently
truncate (corrupt) JSON input, and instead of returning an error, we would return success.

This behavior is wrong, and this PR fixes it so that an error is returned:
```
select '{"longKey1":"longValue1"}} should this data be ignored?'::jsonb;
ERROR: could not parse JSON: trailing characters after JSON document
SQLSTATE: 22P02
```

Release note (bug fix): Do not silently truncate trailing characters when attempting to convert corrupt JSON string input into JSONb.
Release note (backward-incompatible change): This change may be backward incompatible to the applications
that previously might have been able to insert corrupt JSON data, but now will receive an error.

Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Yevgeniy Miretskiy <[email protected]>
nicktrav added a commit to nicktrav/cockroach that referenced this issue Oct 14, 2022
The existing `enc_util` package contains a tool that could be used to
dump the files in an encryption registry. This command has been broken
since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of
files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files
contained in the registry of an encrypted store. This is useful for
debugging which store / data key was used to encrypt each file,
replacing the equivalent functionality in `enc_util`.

Touches: cockroachdb#89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an
operator to list the files present in the Encryption-At-Rest file
registry.
craig bot pushed a commit that referenced this issue Oct 14, 2022
87938: bazci: move the logic of posting github issues to bazci r=srosenberg a=healthy-pod

This code change moves the logic of filtering tests JSON output files and posting github issues to bazci.

Release note: None
Epic CRDB-15060

89257: upgrades: remove unused struct field r=stevendanna a=stevendanna

Epic: None

Release note: None

89873: cliccl: add `encryption-registry-list` command r=jbowenns a=nicktrav

The existing `enc_util` package contains a tool that could be used to dump the files in an encryption registry. This command has been broken since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files contained in the registry of an encrypted store. This is useful for debugging which store / data key was used to encrypt each file, replacing the equivalent functionality in `enc_util`.

Touches: #89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an operator to list the files present in the Encryption-At-Rest file registry.

89988: rangedesciter: carve out library for range desc iteration r=irfansharif a=irfansharif

Informs #87503; pure code-movement. Going to use it in future commits as part of multi-tenant replication reports (#89987) where we'll need to iterate over the set of range descriptors.

Release note: None

Co-authored-by: healthy-pod <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
@nicktrav
Copy link
Collaborator Author

Going to close this out as we now have the decryption and listing sub-commands.

kvoli pushed a commit to kvoli/cockroach that referenced this issue Oct 17, 2022
The existing `enc_util` package contains a tool that could be used to
dump the files in an encryption registry. This command has been broken
since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of
files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files
contained in the registry of an encrypted store. This is useful for
debugging which store / data key was used to encrypt each file,
replacing the equivalent functionality in `enc_util`.

Touches: cockroachdb#89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an
operator to list the files present in the Encryption-At-Rest file
registry.
nicktrav added a commit to nicktrav/cockroach that referenced this issue May 30, 2023
During storage-level L2 investigations, files from problematic stores
are often requested (e.g. the MANIFEST file(s), SSTables, etc.). In
cases where the store is using encryption-at-rest, the debug artifacts
are useless unless they have been decrypted.

Add a new debug command that can be used to decrypt a file in-situ,
given the encryption spec for the store, and a path to an encrypted file
in the store. For example:

```bash
$ cockroach debug encryption-decrypt \
  --enterprise-encryption=$encryption_spec \
  /path/to/store \
  /path/to/encrypted/file \
  /path/to/decrypted/output/file
```

Touches cockroachdb#89095.

Release note (ops change): A new debug tool was added to allow for
decrypting files in a store using encryption-at-rest. This tool is
intended for use while debugging, or for providing debug artifacts to
Cockroach Labs to aid with support investigations. It is intended to be
run "in-situ" (i.e. on site), as it prevents having to move sensitive
key material.
nicktrav added a commit to nicktrav/cockroach that referenced this issue May 30, 2023
The existing `enc_util` package contains a tool that could be used to
dump the files in an encryption registry. This command has been broken
since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of
files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files
contained in the registry of an encrypted store. This is useful for
debugging which store / data key was used to encrypt each file,
replacing the equivalent functionality in `enc_util`.

Touches: cockroachdb#89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an
operator to list the files present in the Encryption-At-Rest file
registry.
nicktrav added a commit to nicktrav/pebble that referenced this issue May 31, 2023
Currently, the tools write directly to `os.{Stdout,Stderr}`, which
complicates testing in the case where the command is run from same
process running the tests (i.e. from a `testing.T` func).

Adapt the tools to make use of the return values from
`(*cobra.Command).{OutOrStdout,OutOrStderr}`. In testing scenarios, the
tools can use `SetOut` and `SetErr` to pass in a writer that can
intercept anything written to stdout / stderr. In productions scenarios,
the tools will emit to the appropriate channel.

Touches cockroachdb/cockroach#89095.
nicktrav added a commit to cockroachdb/pebble that referenced this issue May 31, 2023
Currently, the tools write directly to `os.{Stdout,Stderr}`, which
complicates testing in the case where the command is run from same
process running the tests (i.e. from a `testing.T` func).

Adapt the tools to make use of the return values from
`(*cobra.Command).{OutOrStdout,OutOrStderr}`. In testing scenarios, the
tools can use `SetOut` and `SetErr` to pass in a writer that can
intercept anything written to stdout / stderr. In productions scenarios,
the tools will emit to the appropriate channel.

Touches cockroachdb/cockroach#89095.
nicktrav added a commit to nicktrav/cockroach that referenced this issue May 31, 2023
During storage-level L2 investigations, files from problematic stores
are often requested (e.g. the MANIFEST file(s), SSTables, etc.). In
cases where the store is using encryption-at-rest, the debug artifacts
are useless unless they have been decrypted.

Add a new debug command that can be used to decrypt a file in-situ,
given the encryption spec for the store, and a path to an encrypted file
in the store. For example:

```bash
$ cockroach debug encryption-decrypt \
  --enterprise-encryption=$encryption_spec \
  /path/to/store \
  /path/to/encrypted/file \
  /path/to/decrypted/output/file
```

Touches cockroachdb#89095.

Release note (ops change): A new debug tool was added to allow for
decrypting files in a store using encryption-at-rest. This tool is
intended for use while debugging, or for providing debug artifacts to
Cockroach Labs to aid with support investigations. It is intended to be
run "in-situ" (i.e. on site), as it prevents having to move sensitive
key material.
nicktrav added a commit to nicktrav/cockroach that referenced this issue May 31, 2023
The existing `enc_util` package contains a tool that could be used to
dump the files in an encryption registry. This command has been broken
since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of
files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files
contained in the registry of an encrypted store. This is useful for
debugging which store / data key was used to encrypt each file,
replacing the equivalent functionality in `enc_util`.

Touches: cockroachdb#89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an
operator to list the files present in the Encryption-At-Rest file
registry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-storage Storage Team
Projects
None yet
Development

No branches or pull requests

1 participant