WIP - Transactional storage #292

cipherboy · 2024-04-13T18:54:14Z

This adds support for interactive transactions across the backend, physical, and plugin storage levels. This will ultimately allow for greater assurance in secrets management operations built on top of OpenBao.

See the RFC in #296 for more detailed information about the design of this change.

TODO:

When a non-empty path is given that ends in a slash and after=., the resulting seek prefix from joining the path and the period has the slash trimmed off. This results in no list results, because this seek prefix lacks the trailing slash and is thus not prefixed by the expected prefix. E.g., for foo/ and after=., we end up with seekPrefix=foo, which doesn't start with our prefix (foo/). Correctly handle this edge case. This was caught by randomized testing across multiple storage backends. Signed-off-by: Alexander Scheel <[email protected]>

This removes the legacy one-shot transaction interface and its various implementers, including in the File, InMem, and Raft backends. In the former's case (file), this might not have truly had the strict consistency guarantees one would expect from a transaction implementation. Signed-off-by: Alexander Scheel <[email protected]>

command/commands.go

physical/raft/raft.go

command/server.go

helper/testhelpers/teststorage/teststorage.go

cipherboy · 2024-04-15T17:47:42Z

physical/raft/fsm.go

+ verifyReadOp
+ verifyListOp
+ beginTxOp
+ commitTxOp


beginTxOp / commitTxOp are mostly transaction sentinels for the time being. However, when implementing the single hash method (number 3), we can send the hash type in beginTxOp and the final hash in commitTxOp. We'll need verifyReadOp because while we can re-order any un-modified reads to occur at the end (sending them along in the commitTxOp if we wanted fewer operations in the log), we can't avoid sending reads before writes. We'll need verifyListOp, because its results can be impacted by writes and we won't necessarily know to compact (or not compact) them, so we'd prefer to keep them in execution order.

physical/raft/fsm.go

sdk/physical/transactions.go

physical/raft/transaction.go

sdk/helper/keysutil/encrypted_key_storage.go

sdk/logical/storage_view.go

cipherboy · 2024-04-15T19:15:01Z

vault/barrier.go

 // SecurityBarrier must provide the encryption APIs
 BarrierEncryptor
 }

-// BarrierStorage is the storage only interface required for a Barrier.
-type BarrierStorage interface {


BarrierStorage was unused and conformed to logical.Storage.

vault/barrier_aes_gcm.go

cipherboy · 2024-04-15T19:17:37Z

vault/sealunwrapper.go

-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
-package vault


This removal could be a separate PR as well.

Signed-off-by: Alexander Scheel <[email protected]>

This converts StorageView to an interface, aligning with the Storage interface. The only consumer, as far as I can tell, is the BarrierView, which was easy enough to convert to an interface user, and will itself need to be made transactional in a future commit. Signed-off-by: Alexander Scheel <[email protected]>

This converts BarrierView to an interface, updating definitions everywhere to use it as such. This lets us replace it with two different structs (barrierView and transacitonalBarrierView). Signed-off-by: Alexander Scheel <[email protected]>

This extends SecurityBarrier into a TransactionalSecurityBarrier variant, in which access to the underlying storage (but not the barrier itself!) can be modified in a transaction-aware way. Signed-off-by: Alexander Scheel <[email protected]>

Signed-off-by: Alexander Scheel <[email protected]>

SealUnwrapper exists to err about storage which was at one point in time seal wrapped. This only exists on Vault Enterprise, and would require a migration from Vault Enterprise -> OpenBao to trigger. As we likely won't support this (as we have no way of unsealing the underlying storage as we don't have the seal wrap subsystem in OSS), and thus would require it to have been completely unseal wrapped manually prior to migration, we can safely remove this and assume that other failures would instead catch this unlikely event. This is also one less layer to add transactional storage to. Signed-off-by: Alexander Scheel <[email protected]>

Signed-off-by: Alexander Scheel <[email protected]>

This adds a cross-storage backend testing interface, suitable for testing all physical backends at various levels of indirection (from direct access to caches to error interposers). This will eventually include testing logical.Storage interfaces as well, to ensure various combinations of layers work together nicely (physical + barrier + views) and match interface expectations. Signed-off-by: Alexander Scheel <[email protected]>

cipherboy force-pushed the transactional-storage branch from 36b23ba to c949738 Compare April 13, 2024 19:00

cipherboy marked this pull request as draft April 13, 2024 19:16

cipherboy added 2 commits April 14, 2024 16:30

cipherboy force-pushed the transactional-storage branch from c949738 to 27cda85 Compare April 15, 2024 12:13

cipherboy mentioned this pull request Apr 15, 2024

RFC - Transactional Storage for Plugins & Core #296

Open