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

Added E2E tests using the SQLiteKIM #529

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,19 @@ jobs:
# 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 cryptoauthlib --no-stress-test

sqlite-kim:
name: SQLiteKIM E2E tests on all providers
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
# Use the following step when updating the `parsec-service-test-all` image
# - name: Build the container
# 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
# 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

# Disabled due to the issue discussed in https://github.com/parallaxsecond/parsec/issues/514
# fuzz-test-checker:
# name: Check that the fuzz testing framework is still working
Expand Down
31 changes: 25 additions & 6 deletions ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ where PROVIDER_NAME can be one of:
- cryptoauthlib
- all
- coverage
- sqlite-kim
"
}

Expand Down Expand Up @@ -117,18 +118,20 @@ while [ "$#" -gt 0 ]; do
--no-stress-test )
NO_STRESS_TEST="True"
;;
mbed-crypto | pkcs11 | tpm | trusted-service | cryptoauthlib | all | cargo-check)
mbed-crypto | pkcs11 | tpm | trusted-service | cryptoauthlib | all | cargo-check | sqlite-kim)
if [ -n "$PROVIDER_NAME" ]; then
error_msg "Only one provider name must be given"
fi
PROVIDER_NAME=$1

# If running anything but cargo-check, copy config
if [ "$PROVIDER_NAME" != "cargo-check" ]; then
if [ "$PROVIDER_NAME" != "cargo-check" ] && [ "$PROVIDER_NAME" != "sqlite-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
fi

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

trap cleanup EXIT

if [ "$PROVIDER_NAME" = "tpm" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ]; then
if [ "$PROVIDER_NAME" = "tpm" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; 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 @@ -165,7 +168,7 @@ if [ "$PROVIDER_NAME" = "tpm" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_
tpm2_startup -T mssim
fi

if [ "$PROVIDER_NAME" = "pkcs11" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ]; then
if [ "$PROVIDER_NAME" = "pkcs11" ] || [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "coverage" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; 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 @@ -220,7 +223,7 @@ if [ "$PROVIDER_NAME" = "coverage" ]; then
exit 0
fi

if [ "$PROVIDER_NAME" = "all" ]; then
if [ "$PROVIDER_NAME" = "all" ] || [ "$PROVIDER_NAME" = "sqlite-kim" ]; then
# Start SPIRE server and agent
pushd /tmp/spire-0.11.1
./bin/spire-server run -config conf/server/server.conf &
Expand All @@ -238,6 +241,22 @@ if [ "$PROVIDER_NAME" = "all" ]; 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
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering, if we are going to promote the SQL KIM as the default KIM from now on, if we should change all config files to use the SQL KIM and add that if case for the legacy KIM.

Also, we might need stability/persistence tests for it: that old mappings stored with that KIM can still be read in current PRs. Check the "per provider key mappings tests" here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was also wondering whether we should make it the default. The reason it is currently only the subset of tests (all_providers::normal) is that quite a few of the E2E tests currently rely on the fact that the OnDiskKIM is able to store keys with the same name in different providers. Tests that rely on this logic obviously fail. Quite a few of the E2E tests would therefore need refactoring to account for this, hence why I went with this approach for now 😅 better to have some E2E tests that none.

As for stability tests I already have an issue tracking this here that hopefully somebody will pick up, or me when I'm back.

Copy link
Member

Choose a reason for hiding this comment

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

Oh nice that you already thought about stability in an issue 👍
Understand about the refactoring needed for the tests. Could you please open an extra-issue about "Making the SQL KIM the default" to track what needs to be done for it?
I am fine to merge this PR then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do


echo "Shutdown Parsec"
stop_service

exit 0
fi

echo "Build test"

if [ "$PROVIDER_NAME" = "cargo-check" ]; then
Expand Down
53 changes: 53 additions & 0 deletions e2e_tests/provider_cfg/all/sqlite-kim-all-providers.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
[core_settings]
# The CI already timestamps the logs
log_timestamp = false
log_error_details = true

# The container runs the Parsec service as root, so make sure we disable root
# checks.
allow_root = true

[listener]
listener_type = "DomainSocket"
timeout = 200 # in milliseconds
socket_path = "/tmp/parsec.sock"

[authenticator]
auth_type = "Direct"
admins = [ { name = "list_clients test" }, { name = "1000" }, { name = "client1" }, { name = "spiffe://example.org/parsec-client-1" } ]
#workload_endpoint="unix:///tmp/agent.sock"

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

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

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

[[provider]]
provider_type = "Pkcs11"
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 = "sqlite-manager"
device_type = "always-success"
iface_type = "test-interface"
# wake_delay = 1500
# rx_retries = 20
# # i2c parameters for i2c-pseudo proxy
# slave_address = 0xc0
# bus = 1
# baud = 400000
2 changes: 1 addition & 1 deletion e2e_tests/tests/all_providers/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ fn list_and_delete_clients() {

client.set_default_auth(Some(all_providers_user.clone()));
client
.generate_rsa_sign_key("all-providers-user-key".to_string())
.generate_rsa_sign_key(format!("{}-all-providers-user-key", provider.id))
.unwrap();

client.set_default_auth(Some(format!("user_{}", provider.id)));
Expand Down