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

pkg/cliccl: add debug encryption-decrypt command #89668

Merged
merged 1 commit into from
Oct 12, 2022

Conversation

nicktrav
Copy link
Collaborator

@nicktrav nicktrav commented 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:

$ 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.

@nicktrav nicktrav requested review from jbowens and a team October 10, 2022 16:41
@nicktrav nicktrav requested review from a team as code owners October 10, 2022 16:41
@nicktrav nicktrav requested review from benbardin and removed request for a team October 10, 2022 16:41
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nicktrav nicktrav force-pushed the nickt/pebble-master-f122ff497182 branch 3 times, most recently from bdd18b7 to e8fc99e Compare October 10, 2022 21:48
Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r1, 1 of 6 files at r2, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @benbardin and @nicktrav)


-- commits line 23 at r2:
this example command is missing a decrypt, right? eg, cockroach debug encryption-at-rest decrypt ..


pkg/cli/BUILD.bazel line 109 at r2 (raw file):

        "//pkg/ccl/sqlproxyccl",
        "//pkg/ccl/sqlproxyccl/tenantdirsvr",
        "//pkg/ccl/storageccl/engineccl",

I think these new ccl imports are still prohibited, although I'm not sure why a linter didn't catch it or the sqlproxyccl package is permitted.

To avoid this, I think we can define this new command within pkg/ccl/cliccl/debug.go.


pkg/cli/ear.go line 42 at r2 (raw file):

	Use:   "decrypt <encryption-spec> <in-file> [out-file]",
	Short: "decrypt a file from an encrypted store",
	Long: `Decrypts an file from an encrypted store, and outputs it to the

nit: s/an file/a file/


pkg/cli/ear.go line 59 at r2 (raw file):

	}

	encSpec, err := baseccl.NewStoreEncryptionSpec(encSpecStr)

Once in the ccl/cliccl package, I think we can make use of the existing --enterprise-encryption cli flag

@nicktrav nicktrav force-pushed the nickt/pebble-master-f122ff497182 branch 3 times, most recently from c6d5ee6 to 23f28fa Compare October 12, 2022 16:41
Copy link
Collaborator Author

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be good for another look. Moved everything over to cclcli to invert the package dependencies.

There's a merge conflict due to the Pebble bump commit. I'll rip out that first commit once #89839 lands.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @benbardin and @jbowens)


-- commits line 23 at r2:

Previously, jbowens (Jackson Owens) wrote…

this example command is missing a decrypt, right? eg, cockroach debug encryption-at-rest decrypt ..

Fixed. Thanks!


pkg/cli/BUILD.bazel line 109 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

I think these new ccl imports are still prohibited, although I'm not sure why a linter didn't catch it or the sqlproxyccl package is permitted.

To avoid this, I think we can define this new command within pkg/ccl/cliccl/debug.go.

Thanks for the pointer. Done.


pkg/cli/ear.go line 42 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: s/an file/a file/

Done.


pkg/cli/ear.go line 59 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

Once in the ccl/cliccl package, I think we can make use of the existing --enterprise-encryption cli flag

Done.

@nicktrav nicktrav changed the title pkg/cli: add debug encryption-at-rest decrypt command pkg/cliccl: add debug encryption-decrypt command Oct 12, 2022
Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

:lgtm:

Reviewed 5 of 10 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @benbardin)

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 nicktrav force-pushed the nickt/pebble-master-f122ff497182 branch from 23f28fa to 6e47daa Compare October 12, 2022 19:40
@nicktrav
Copy link
Collaborator Author

TFTR!

bors r=jbowens

@craig
Copy link
Contributor

craig bot commented Oct 12, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants