engineccl: Support JWKS for encryption-at-rest keys and hook up v2

[bdarnell]

The JWKS format for key files is now supported in addition to the legacy format (which was just raw key bytes). Keys in JWKS format can request the faster V2 encryption implementation, supporting graceful rotation into the new implementation.

Fixes #119767

[cockroach-teamcity]

This change is 

[RaduBerinde]

Reviewed 3 of 5 files at r1, 4 of 4 files at r3, 7 of 7 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @bdarnell and @jbowens)

---

-- commits line 22 at r3:
[nit] left over

---

pkg/ccl/cliccl/gen.go line 60 at r4 (raw file):

		case 256:
			et = enginepbccl.EncryptionType_AES_256_CTR_V2
		}

default: return error?

---

pkg/ccl/storageccl/engineccl/pebble_key_manager.go line 183 at r4 (raw file):

			key.Info.EncryptionType = enginepbccl.EncryptionType_AES256_CTR
		default:
			return nil, errors.Wrapf(jwkErr, "could not parse store key. Key length %d is not valid for old-style key", keyLength)

[nit] If there is a bug when generating a JWK key (or it becomes corrupted for whatever reason), we will get this error. It would be nice to also show the jwkErr in the message.

---

pkg/ccl/storageccl/engineccl/enginepbccl/key_registry.go line 11 at r4 (raw file):

package enginepbccl

import fmt "fmt"

[nit] the fmt is unnecessary

---

pkg/ccl/storageccl/engineccl/enginepbccl/key_registry.go line 17 at r4 (raw file):

	switch e {
	case EncryptionType_AES128_CTR:
		return "cockroach-aes-128-ctr-v1", nil

[nit] it's unfortunate (error prone) that we have multiple instances of these string constants. Consider defining them only once in a [...]string{EncryptionType_AES128_CTR: "cockroach-aes-128-ctr-v1", ..} or adding a roundtripping test.

We could also make use of e.String() which is already defined (e.g. return "COCKROACH_" + e.String()).

[bdarnell]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @jbowens and @RaduBerinde)

---

-- commits line 22 at r3:

Previously, RaduBerinde wrote…

[nit] left over

Fixed

---

pkg/ccl/cliccl/gen.go line 60 at r4 (raw file):

Previously, RaduBerinde wrote…

default: return error?

Sure. It's redundant since we also checked at the top of the function, but it doesn't hurt.

---

pkg/ccl/storageccl/engineccl/pebble_key_manager.go line 183 at r4 (raw file):

Previously, RaduBerinde wrote…

[nit] If there is a bug when generating a JWK key (or it becomes corrupted for whatever reason), we will get this error. It would be nice to also show the jwkErr in the message.

jwkErr is there as the wrapped error, which I think is the right way to model this in our errors library (I also considered CombineErrors with both jwkErr and a new fmt.Errorf for the old-style key case).

With a small tweak here's what it looks like:

Received unexpected error:
                                could not parse store key. Key length 17 is not valid for old-style key. Parse error for new-style key: failed to unmarshal JWK set: error reading token: EOF

---

pkg/ccl/storageccl/engineccl/enginepbccl/key_registry.go line 11 at r4 (raw file):

Previously, RaduBerinde wrote…

[nit] the fmt is unnecessary

Weird, I wonder why goimports did that.

---

pkg/ccl/storageccl/engineccl/enginepbccl/key_registry.go line 17 at r4 (raw file):

Previously, RaduBerinde wrote…

[nit] it's unfortunate (error prone) that we have multiple instances of these string constants. Consider defining them only once in a [...]string{EncryptionType_AES128_CTR: "cockroach-aes-128-ctr-v1", ..} or adding a roundtripping test.

We could also make use of e.String() which is already defined (e.g. return "COCKROACH_" + e.String()).

Meh, I feel better about our ability to manually keep two short lists in sync than to write something clever to avoid the duplication.

[RaduBerinde]

Reviewed 4 of 4 files at r6, 6 of 7 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jbowens)

---

pkg/ccl/storageccl/engineccl/pebble_key_manager.go line 183 at r4 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

jwkErr is there as the wrapped error, which I think is the right way to model this in our errors library (I also considered CombineErrors with both jwkErr and a new fmt.Errorf for the old-style key case).

With a small tweak here's what it looks like:

Received unexpected error:
                                could not parse store key. Key length 17 is not valid for old-style key. Parse error for new-style key: failed to unmarshal JWK set: error reading token: EOF

Looks great!

[bdarnell]

bors r=RaduBerinde

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)