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

Refactor Storage Layer #106

Merged
merged 3 commits into from
Oct 29, 2024
Merged

Refactor Storage Layer #106

merged 3 commits into from
Oct 29, 2024

Conversation

simo5
Copy link
Member

@simo5 simo5 commented Oct 23, 2024

This splits out the storage spefcific knowledge from the token to the actual storage backend.

The storage backend is restructured so that multiple storage types can use a common information organizational schema for non standardized objects like user pins and token info and non standardized authenticationand encryption layers for the actual data.

This is in preparation for adding code to read databases written by other software tokens that follow different storage philosophies.

While woring on this refactoring a few minor issues with how pin/login were handled was found and have been fixed.

Additionally the code now uses a common Key encryption Key that both the Ueer and SO Pins can unlock allowing recovery of the database via SO pin if the user pin is lost. This means the SO pin needs to be set to a non-default value on initialization to maintain the confidentaility of the token.
It can be set to a random value if recovery is never desired.

@simo5 simo5 requested a review from Jakuje October 23, 2024 21:21
@simo5
Copy link
Member Author

simo5 commented Oct 23, 2024

Note, this commit also drops the special use code available only in tests that was disabling encryption as it turns out that now that we do not decrypt every object on search but only on explicit fetch the test are not that slow anymore.

Or rather the slow tests are gated behind the "slow" feature, so it is not a big deal if thy are really slow.

@simo5
Copy link
Member Author

simo5 commented Oct 23, 2024

Let me know if you think this is too slow:

$ cargo test --features slow
[..]
test result: ok. 176 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 258.00s

I think it is fine given:

$ cargo test
[..]
test result: ok. 176 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 5.19s

@simo5 simo5 force-pushed the storage_refactor branch 2 times, most recently from 13daba9 to 5a45744 Compare October 23, 2024 22:00
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

mostly nits on the first read through

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/storage/format.rs Outdated Show resolved Hide resolved
@simo5 simo5 force-pushed the storage_refactor branch 2 times, most recently from 2762dd4 to 6d7c29c Compare October 28, 2024 15:40
simo5 added 2 commits October 28, 2024 15:56
This splits out the storage spefcific knowledge from the token
to the actual storage backend.

The storage backend is restructured so that multiple storage types can
use a common information organizational schema for non standardized
objects like user pins and token info and non standardized
authenticationand encryption layers for the actual data.

This is in preparation for adding code to read databases written by
other software tokens that follow different storage philosophies.

While woring on this refactoring a few minor issues with how pin/login
were handled was found and have been fixed.

Additionally the code now uses a common Key encryption Key that both the
Ueer and SO Pins can unlock allowing recovery of the database via SO
pin if the user pin is lost. This means the SO pin needs to be set to a
non-default value on initialization to maintain the confidentaility of
the token.
It can be set to a random value if recovery is never desired.

Signed-off-by: Simo Sorce <simo@redhat.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
src/storage/json.rs Show resolved Hide resolved
src/config.rs Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
Using 0 as the default slot number was masking cases when the
configuration may not be correctly imported.

By using an "invalid" value by default we ensure that a slot number has
been properly assigned, and better check that there are no duplicate
(same backing database) tokens loaded in different slots.

Signed-off-by: Simo Sorce <simo@redhat.com>
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

lgtm

@simo5 simo5 merged commit 4721238 into latchset:main Oct 29, 2024
5 checks passed
@simo5
Copy link
Member Author

simo5 commented Oct 29, 2024

Thank you!

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.

2 participants