Skip to content

Make SQLite KIM default #570

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 14, 2022
Merged

Make SQLite KIM default #570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6df89a0
Make SQLite KIM default
ionut-arm Jan 6, 2022
2e37306
Make `name` look like an option in config
ionut-arm Jan 10, 2022
06c81f8
Merge branch 'main' of github.com:parallaxsecond/parsec into sqlite-k…
ionut-arm Jan 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ jobs:
# When running the container built on the CI
# run: CONTAINER_TAG=parsec-service-test-all ./fuzz.sh test

sqlite-kim:
name: SQLiteKIM E2E tests on all providers
on-disk-kim:
name: OnDiskKIM E2E tests on all providers
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand All @@ -128,9 +128,9 @@ jobs:
# run: pushd e2e_tests/docker_image && docker build -t parsec-service-test-all -f parsec-service-test-all.Dockerfile . && popd
- name: Run the container to execute the test script
run:
docker run -v $(pwd):/tmp/parsec -w /tmp/parsec ghcr.io/parallaxsecond/parsec-service-test-all /tmp/parsec/ci.sh sqlite-kim
docker run -v $(pwd):/tmp/parsec -w /tmp/parsec ghcr.io/parallaxsecond/parsec-service-test-all /tmp/parsec/ci.sh on-disk-kim
# When running the container built on the CI
# run: docker run -v $(pwd):/tmp/parsec -w /tmp/parsec -t parsec-service-test-all /tmp/parsec/ci.sh sqlite-kim
# run: docker run -v $(pwd):/tmp/parsec -w /tmp/parsec -t parsec-service-test-all /tmp/parsec/ci.sh on-disk-kim

cross-compilation:
# Currently only the Mbed Crypto, PKCS 11, and TPM providers are tested as the other ones need to cross-compile other libraries.
Expand Down
77 changes: 36 additions & 41 deletions ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ where PROVIDER_NAME can be one of:
- cryptoauthlib
- all
- coverage
- sqlite-kim
- on-disk-kim
"
}

Expand Down Expand Up @@ -106,6 +106,25 @@ run_key_mappings_tests() {
RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml key_mappings
}

setup_mappings() {
# Add the Docker image's mappings in this Parsec service for the key mappings
# test.
# The key mappings test in e2e_tests/tests/per_provider/key_mappings.rs will try
# to use the key generated via the generate-keys.sh script in the test image.
cp -r /tmp/mappings/ .
# Add the fake mappings for the key mappings test as well. The test will check that
# those keys have successfully been deleted.
# TODO: add fake mappings for the Trusted Service and CryptoAuthLib providers.
cp -r $(pwd)/e2e_tests/fake_mappings/* mappings
# As Mbed Crypto saves its keys on the current directory we need to move them
# as well.
if [ "$PROVIDER_NAME" = "mbed-crypto" ]; then
cp /tmp/*.psa_its .
fi

reload_service
}

# Parse arguments
NO_CARGO_CLEAN=
NO_STRESS_TEST=
Expand All @@ -119,20 +138,21 @@ while [ "$#" -gt 0 ]; do
--no-stress-test )
NO_STRESS_TEST="True"
;;
mbed-crypto | pkcs11 | tpm | trusted-service | cryptoauthlib | all | cargo-check | sqlite-kim)
mbed-crypto | pkcs11 | tpm | trusted-service | cryptoauthlib | all | cargo-check | on-disk-kim)
if [ -n "$PROVIDER_NAME" ]; then
error_msg "Only one provider name must be given"
fi
PROVIDER_NAME=$1

# Copy provider specific config, unless CI is running `cargo-check` or `sqlite-kim` CI
if [ "$PROVIDER_NAME" != "cargo-check" ] && [ "$PROVIDER_NAME" != "sqlite-kim" ]; then
# Copy provider specific config, unless CI is running `cargo-check` or `on-disk-kim` CI
if [ "$PROVIDER_NAME" != "cargo-check" ] && [ "$PROVIDER_NAME" != "on-disk-kim" ]; then
cp $(pwd)/e2e_tests/provider_cfg/$1/config.toml $CONFIG_PATH
elif [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
cp $(pwd)/e2e_tests/provider_cfg/all/sqlite-kim-all-providers.toml $CONFIG_PATH
elif [ "$PROVIDER_NAME" = "on-disk-kim" ]; then
PROVIDER_NAME=all
cp $(pwd)/e2e_tests/provider_cfg/all/on-disk-kim-all-providers.toml $CONFIG_PATH
fi

if [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "cargo-check" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
if [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "cargo-check" ]; then
FEATURES="--features=all-providers,all-authenticators"
TEST_FEATURES="--features=all-providers"
else
Expand All @@ -157,7 +177,7 @@ fi

trap cleanup EXIT

if [ "$PROVIDER_NAME" = "tpm" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
if [ "$PROVIDER_NAME" = "tpm" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ]; then
# Copy the NVChip for previously stored state. This is needed for the key mappings test.
cp /tmp/NVChip .
# Start and configure TPM server
Expand All @@ -179,7 +199,7 @@ if [ "$PROVIDER_NAME" = "tpm" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_
popd
fi

if [ "$PROVIDER_NAME" = "pkcs11" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
if [ "$PROVIDER_NAME" = "pkcs11" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ]; then
pushd e2e_tests
# This command suppose that the slot created by the container will be the first one that appears
# when printing all the available slots.
Expand Down Expand Up @@ -237,7 +257,7 @@ if [ "$PROVIDER_NAME" = "coverage" ]; then
exit 0
fi

if [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
if [ "$PROVIDER_NAME" = "all" ]; then
# Start SPIRE server and agent
pushd /tmp/spire-0.11.1
./bin/spire-server run -config conf/server/server.conf &
Expand All @@ -255,22 +275,6 @@ if [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
popd
fi

# Test the SQLite KIM
if [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
echo "Start Parsec for end-to-end tests with sqlite-kim"
RUST_LOG=info RUST_BACKTRACE=1 cargo run --release $FEATURES -- --config $CONFIG_PATH &
# Sleep time needed to make sure Parsec is ready before launching the tests.
wait_for_service

echo "Execute all-providers sqlite-kim normal tests"
RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml all_providers::normal

echo "Shutdown Parsec"
stop_service

exit 0
fi

echo "Build test"

if [ "$PROVIDER_NAME" = "cargo-check" ]; then
Expand Down Expand Up @@ -329,21 +333,6 @@ RUST_BACKTRACE=1 cargo test $FEATURES
# Removing any mappings left over from integration tests
rm -rf mappings/

# Add the Docker image's mappings in this Parsec service for the key mappings
# test.
# The key mappings test in e2e_tests/tests/per_provider/key_mappings.rs will try
# to use the key generated via the generate-keys.sh script in the test image.
cp -r /tmp/mappings/ .
# Add the fake mappings for the key mappings test as well. The test will check that
# those keys have successfully been deleted.
# TODO: add fake mappings for the Trusted Service and CryptoAuthLib providers.
cp -r $(pwd)/e2e_tests/fake_mappings/* mappings
# As Mbed Crypto saves its keys on the current directory we need to move them
# as well.
if [ "$PROVIDER_NAME" = "mbed-crypto" ]; then
cp /tmp/*.psa_its .
fi

echo "Start Parsec for end-to-end tests"
RUST_LOG=info RUST_BACKTRACE=1 cargo run --release $FEATURES -- --config $CONFIG_PATH &
# Sleep time needed to make sure Parsec is ready before launching the tests.
Expand All @@ -353,6 +342,9 @@ if [ "$PROVIDER_NAME" = "all" ]; then
echo "Execute all-providers normal tests"
RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml all_providers::normal

echo "Execute all-providers cross tests"
RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml all_providers::cross

echo "Execute all-providers multi-tenancy tests"
# Needed because parsec-client-1 and 2 write to those locations owned by root
chmod 777 /tmp/parsec/e2e_tests
Expand All @@ -363,6 +355,7 @@ if [ "$PROVIDER_NAME" = "all" ]; then
su -c "PATH=\"/home/parsec-client-1/.cargo/bin:${PATH}\";RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml --target-dir /home/parsec-client-1 all_providers::multitenancy::client1_before" parsec-client-1
su -c "PATH=\"/home/parsec-client-2/.cargo/bin:${PATH}\";RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml --target-dir /home/parsec-client-2 all_providers::multitenancy::client2" parsec-client-2
su -c "PATH=\"/home/parsec-client-1/.cargo/bin:${PATH}\";RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml --target-dir /home/parsec-client-1 all_providers::multitenancy::client1_after" parsec-client-1

# Change the authentication method
sed -i 's/^\(auth_type\s*=\s*\).*$/\1\"UnixPeerCredentials\"/' $CONFIG_PATH
reload_service
Expand All @@ -383,6 +376,8 @@ if [ "$PROVIDER_NAME" = "all" ]; then
echo "Execute all-providers config tests"
RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml all_providers::config -- --test-threads=1
else
setup_mappings

# Per provider tests
run_normal_tests
run_old_e2e_tests
Expand Down
36 changes: 24 additions & 12 deletions config.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,24 @@ auth_type = "UnixPeerCredentials"
# Defined as an array of tables: https://github.com/toml-lang/toml#user-content-array-of-tables
[[key_manager]]
# (Required) Name of the key info manager. Used to tie providers to the manager supporting them.
name = "on-disk-manager"
name = "sqlite-manager"

# (Required) Type of key info manager to be used.
manager_type = "OnDisk"
# Possible values: "SQLite", "OnDisk"
# NOTE: The SQLite KIM is now the recommended type, with the OnDisk KIM to be deprecated at some
# point in the future.
manager_type = "SQLite"

# Path to the location where the mapping will be persisted (in this case, the filesystem path)
# Path to the location where the database will be persisted
#store_path = "/var/lib/parsec/kim-mappings/sqlite/sqlite-key-info-manager.sqlite3"

# Example of OnDisk Key Info Manager configuration
#[[key_manager]]
# (Required) Name of the key info manager.
#name = "on-disk-manager"
# (Required) Type of key info manager to be used.
#manager_type = "OnDisk"
# Path to the location where the mappings will be persisted (in this case, the filesystem path)
#store_path = "/var/lib/parsec/mappings"

# (Required) Provider configurations.
Expand Down Expand Up @@ -120,7 +132,7 @@ provider_type = "MbedCrypto"
# Crypto library by default within the working directory of the service, NOT in the same location
# as the mappings mentioned previously. If you want the keys to be persisted across reboots, ensure
# that the working directory is not temporary.
key_info_manager = "on-disk-manager"
key_info_manager = "sqlite-manager"

# Example of a PKCS 11 provider configuration
#[[provider]]
Expand All @@ -132,9 +144,9 @@ key_info_manager = "on-disk-manager"
# ⚠ WARNING: Changing provider name after use will lead to loss of existing keys.
# ⚠
# (Optional) The name of the provider
# name = "pkcs11-provider"
#name = "pkcs11-provider"
#provider_type = "Pkcs11"
#key_info_manager = "on-disk-manager"
#key_info_manager = "sqlite-manager"
# (Required for this provider) Path to the location of the dynamic library loaded by this provider.
# For the PKCS 11 provider, this library implements the PKCS 11 API on the target platform.
#library_path = "/usr/local/lib/softhsm/libsofthsm2.so"
Expand Down Expand Up @@ -162,9 +174,9 @@ key_info_manager = "on-disk-manager"
# ⚠ WARNING: Changing provider name after use will lead to loss of existing keys.
# ⚠
# (Optional) The name of the provider
# name = "tpm-provider"
#name = "tpm-provider"
#provider_type = "Tpm"
#key_info_manager = "on-disk-manager"
#key_info_manager = "sqlite-manager"
# (Required) TPM TCTI device to use with this provider. The string can include configuration values - if no
# configuration value is given, the defaults are used. Options are:
# - "device": uses a TPM device available as a file node; path can be given as a configuration string,
Expand Down Expand Up @@ -205,9 +217,9 @@ key_info_manager = "on-disk-manager"
# ⚠ WARNING: Changing provider name after use will lead to loss of existing keys.
# ⚠
# (Optional) The name of the provider
# name = "cryptoauthlib-provider"
#name = "cryptoauthlib-provider"
#provider_type = "CryptoAuthLib"
#key_info_manager = "on-disk-manager"
#key_info_manager = "sqlite-manager"
##########
# (Required) Interface for ATCA device
# Supported values: "i2c", "test-interface"
Expand Down Expand Up @@ -266,9 +278,9 @@ key_info_manager = "on-disk-manager"
# ⚠ WARNING: Changing provider name after use will lead to loss of existing keys.
# ⚠
# (Optional) The name of the provider
# name = "trusted-service-provider"
#name = "trusted-service-provider"
# (Required) Type of provider.
#provider_type = "TrustedService"

# (Required) Name of key info manager that will support this provider.
#key_info_manager = "on-disk-manager"
#key_info_manager = "sqlite-manager"
14 changes: 7 additions & 7 deletions e2e_tests/provider_cfg/all/config.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,31 @@ admins = [ { name = "list_clients test" }, { name = "1000" }, { name = "client1"
#workload_endpoint="unix:///tmp/agent.sock"

[[key_manager]]
name = "on-disk-manager"
manager_type = "OnDisk"
store_path = "./mappings"
name = "sqlite-manager"
manager_type = "SQLite"
database_path = "./kim-mappings/sqlite/sqlite-key-info-manager.sqlite3"

[[provider]]
provider_type = "MbedCrypto"
key_info_manager = "on-disk-manager"
key_info_manager = "sqlite-manager"
Copy link
Member

@MattDavis00 MattDavis00 Jan 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Not related to any of your changes specifically but food for thought around SQLite config files.

[[provider]]
name="mbed-crypto-provider"

I know we originally made the provider name globally optional so that existing configs would not fail with new versions of Parsec. Would it be better for it to be optional for OnDisk KIM implementations (to ensure config stability), and required for SQLite KIM configs? This way it forces the user to edit the config and set the provider names with their "preferred" naming scheme; preventing the (probably common) case of a user changing from the OnDisk to SQLite KIM without reading any of the provider naming warnings (quoted below).

In my mind, by going from OnDisk -> SQLite the user is "opting-in" to a new feature-set and its requirements. The only concern from the stability requirements set out is New options should be optional. I would argue that, transitively, provider name in this case is optional as the manager_type="OnDisk" would have to be mutated to manager_type="SQLite" in order for provider name to become a required field.

Parsec Config Stability:

Old configuration files should still work with future stable Parsec versions, with the same default for optional options.

Configuration options should not disappear in future stable Parsec versions. Configuration defaults should remain the same. New options should be optional.

Current provider name warnings:

⚠ WARNING: Provider name cannot change.
⚠ WARNING: Choose a suitable naming scheme for your providers now.
⚠ WARNING: Provider name defaults to "mbed-crypto-provider" if not provided, you will not be able to change the provider's name from this if you decide to use the default.
⚠ WARNING: Changing provider name after use will lead to loss of existing keys.

On the note of these warnings. WARNING: Provider name cannot change. would probably read nicer as WARNING: Provider name should not change once set.. And maybe loss of existing keys to LOSS OF EXISTING KEYS to really drive the point home and draw the attention of skim readers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Matt's point here makes me think that maybe our rules around config file stability need some refinement, because I would agree that there are cases where explicitly opting in to a new feature (and hence editing the config file anyway) might prompt the need for other required changes. The current rule of "new options should be optional" is perhaps rather too vague. The real intention here is simply to ensure that existing config files always continue working, but that's not the same as saying that it should be possible to edit one thing without changing anything else.

But then there is the question about what should be done in this specific case - should a switch to the SQL lite provider prompt the addition of names for existing providers? I'm in two minds about this. It might be enough simply to ensure that our out-of-box example config includes provider names. At the moment, a working deployment cannot be "switched" from on-disk to sql anyway, because there is no migration facility being offered as yet. If and when we decide to offer a migration path, we could possibly make it a requirement of that process to inject provider names into the config.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have moved this conversation over to #571 !


[[provider]]
provider_type = "Tpm"
key_info_manager = "on-disk-manager"
key_info_manager = "sqlite-manager"
tcti = "mssim"
owner_hierarchy_auth = "tpm_pass"

[[provider]]
provider_type = "Pkcs11"
key_info_manager = "on-disk-manager"
key_info_manager = "sqlite-manager"
library_path = "/usr/local/lib/softhsm/libsofthsm2.so"
user_pin = "123456"
# The slot_number mandatory field is going to replace the following line with a valid number
# slot_number

[[provider]]
provider_type = "CryptoAuthLib"
key_info_manager = "on-disk-manager"
key_info_manager = "sqlite-manager"
device_type = "always-success"
iface_type = "test-interface"
# wake_delay = 1500
Expand Down
14 changes: 7 additions & 7 deletions ...der_cfg/all/sqlite-kim-all-providers.toml → ...er_cfg/all/on-disk-kim-all-providers.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,31 @@ admins = [ { name = "list_clients test" }, { name = "1000" }, { name = "client1"
#workload_endpoint="unix:///tmp/agent.sock"

[[key_manager]]
name = "sqlite-manager"
manager_type = "SQLite"
database_path = "./kim-mappings/sqlite/sqlite-key-info-manager.sqlite3"
name = "on-disk-manager"
manager_type = "OnDisk"
store_path = "./mappings"

[[provider]]
provider_type = "MbedCrypto"
key_info_manager = "sqlite-manager"
key_info_manager = "on-disk-manager"

[[provider]]
provider_type = "Tpm"
key_info_manager = "sqlite-manager"
key_info_manager = "on-disk-manager"
tcti = "mssim"
owner_hierarchy_auth = "tpm_pass"

[[provider]]
provider_type = "Pkcs11"
key_info_manager = "sqlite-manager"
key_info_manager = "on-disk-manager"
library_path = "/usr/local/lib/softhsm/libsofthsm2.so"
user_pin = "123456"
# The slot_number mandatory field is going to replace the following line with a valid number
# slot_number

[[provider]]
provider_type = "CryptoAuthLib"
key_info_manager = "sqlite-manager"
key_info_manager = "on-disk-manager"
device_type = "always-success"
iface_type = "test-interface"
# wake_delay = 1500
Expand Down
8 changes: 4 additions & 4 deletions e2e_tests/tests/all_providers/config/tomls/allow_export.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ socket_path = "/tmp/parsec.sock"
auth_type = "Direct"

[[key_manager]]
name = "on-disk-manager"
manager_type = "OnDisk"
store_path = "./mappings"
name = "sqlite-manager"
manager_type = "SQLite"
database_path = "./kim-mappings/sqlite/sqlite-key-info-manager.sqlite3"

[[provider]]
provider_type = "Pkcs11"
key_info_manager = "on-disk-manager"
key_info_manager = "sqlite-manager"
library_path = "/usr/local/lib/softhsm/libsofthsm2.so"
user_pin = "123456"
# The slot_number mandatory field is going to replace the following line with a valid number
Expand Down
Loading