Stability: Persistent state (key mappings)

[hug-dev]

Definition: Old key mappings should still be read correctly by future stable Parsec versions.
Enforcement: The way the KIM persistently stores the mappings need to be backward-compatible in regards with reading/writing.

Enforcement Check

Mappings between KeyTriple and KeyInfo are persistently stored somewhere. New stable Parsec versions should successfully load old mappings and store current ones (of the same KeyInfoManager).

For the OnDiskKeyInfoManager:

1. base64 encoding of application names are used as directory names
2. provider ID are used as directory names
3. base64 encoding of key names are used as file names
4. key ID type is serialised/deserialised by Serde
5. the Attributes field directly come from the psa-crypto crate, (de)serialised by Serde

Stability:

1. The application names should remain representable as UTF-8 strings. ListClients and DeleteClient are based on that. Stable.
2. Might become a problem if provider ID switch from static numbers (1: MbedCrypto, 2: PKCS 11, 3: TPM, etc) to dynamically allocated IDs (allowing external providers to be dynamically loaded, without prior Parsec knowledge). Providers UUID could be a stable provider reference (but could the same provider be loaded twice? For example two PKCS 11 providers pointing at different PKCS 11 libraries).
3. Stable as key names are UTF-8 strings.
4. If the key ID type is not stable accross Parsec versions, then it won't be possible to load old mappings.
5. If the Attributes structure is modified, then deserialisiation will fail for old mappings. The protobuf version could be used for more stability.

4 and 5. If Serde modifies the way it (de)serialise data, then old mappings won't be read as well.

#271: if multiple authenticators are to be supported simultaneously, that will also probably impact the mappings, specially the OnDiskKeyInfoManager. The same application name should yield different keys under different authenticators.

Testing

Different version of the service should successfully load old mappings and store new ones.

From @ionut-arm in #386:

My initial idea was that we could create another Docker image where, apart from having all the dependencies installed, we also create a number of keys for a number of clients. We then replace the service binary and check that everything still works.

[hug-dev]

About current data stability

The key mappings (and in the future maybe other kind of data) is a database containing information about a specific key:

1. its application name
2. its provider
3. its name
4. its associated data (key ID for example)
5. its attributes

In the near future, we might want to add "6. its authenticator" to the list to handle #271. "6" would be a stable auth_type field value. More information might also be added.

"1" and "3" are stable UTF-8 strings. "2" might need to switch to UUID for stability and if dynamic provider ID are introduced (see #404).
"4" is just a vector of bytes so it stable on the API. The KeyInfoManagerClient, which is meant to be used by providers, uses bincode to (De)serialize the associated data (key ID). bincode is stable and suitable for storage. It will be the responsability of provider to not make breaking change in their serialised associatied data. It is tricky when the data directly come from an external crate (for example TpmsContext for the TPM provider). Testing can prevent breaking changes.
"5" could benefit from the protobuf stability of the version stored in the serialised Protobuf one. That way we know that a newer Parsec version will always be able to read old keys as per our contract stability requirements.

Changing the store medium

To better scale, stabilise and make Parsec more robust we might want to move from our custom file and directory based model (the On-Disk Key Info Manager) to a system based on a pre-existing database software product.
I had a look at different relation-model databases (SQLite, MySQL, PostgreSQL) and key-value store systems (etcd, tikv, redis, etc). I think that the Parsec need of a data store is pretty much simple and straightforward:

• it does not need to be distributed (one service will only access its own mappings)
• it can stay local (the mappings can be expected to live in some Parsec installation folder)
• it is small (Parsec is not expected to store that much keys on a single system)
• it is simple (not many different kind of information per key is stored now).

Finally, performance is important but not critical as the time it takes to execute CRUD operations on the store might be negligible with the time needed to execute some crypto operations on real-hardware.

For those reasons, using SQLite through rusqlite might be a very good option. All the database can be contained in one small file (which path can easily be configured through similar mechanisms than what exists today), it is simple and easy to use through the well-known crate. It seems to be the good candidate for the mappings, and also the other kind of data we might want to store in the future.

A table with the following columns can be created:

• authenticator (integer, primary key)
• application name (text, primary key)
• UUID (text, primary key)
• key name (text, primary key)
• key id (blob)
• key attributes (blob)

This would mean that we would create a new key-info manager and leave that as an option in the configuration file.

Testing

For testing, the service should try on the CI to load a reasonable amount of different old mappings.
Also try to store and load current version mappings.

[hug-dev]

During the community meeting, it was decided that a new KIM is not strictly needed for a baseline stability. This requirement is mostly about ensuring that current users of the KIM will still be able to use it in the future. For that we should have tests where we try to successfully read old mappings.

[hug-dev]

Currently our only tests around this are the persistence tests:

• check that a key can be read after a service reload (but using the same Parsec version to write/read)
• check that manually crafted key mappings can be read and are deleted as they don't map with any key (that is however currently only tested for pkcs11 and mbed Crypto)

What I propose (and that will cover this comment) is:

• using a specific old Parsec version, 0.7.0 for example, create a lot of keys in the Dockerfile and put the mappings somewhere (/tmp/persistence/mappings). Include also mappings that don't map to an actual key (to test if deleted later).
• upload the Dockerfile
• remove the persistence tests (include the logic in the tests above) and create key mappings tests where:
  • the service is reloaded with /tmp/persistence/mappings mappings path
  • the tests check that the key can be correctly loaded, used and that their policies are still valid
  • the tests also check that the appropriate mappings are deleted
• do this for all providers

[hug-dev]

I will create a new issue "Create a SQLite Key Info Manager" from this one when closed.

[hug-dev]

I tried to see if Serde could cope with unbreaking changes in the Attributes structure. The answer is no, sadly 😭. See the following example:

use serde::{Serialize, Deserialize};

#[derive(Serialize, Deserialize, Debug)]
struct StructToto {
    toto1: u32,
    toto2: u32,
    // Was commented out to generate struct_encoded
    toto3: u32,
}

#[derive(Serialize, Deserialize, Debug)]
enum EnumToto {
    Toto1,
    Toto2,
    // Was commented out to generate enum_encoded
    Toto3,
}

fn main() {
    /*
    let enum_toto = EnumToto::Toto2;
    let struct_toto = StructToto {
        toto1: 23,
        toto2: 78,
    };

    let struct_encoded: Vec<u8> = bincode::serialize(&struct_toto).unwrap();
    let enum_encoded: Vec<u8> = bincode::serialize(&enum_toto).unwrap();

    println!("let struct_encoded = {:?};", struct_encoded);
    println!("let enum_encoded = {:?};", enum_encoded);
    */

    let struct_encoded = [23, 0, 0, 0, 78, 0, 0, 0];
    let enum_encoded = [1, 0, 0, 0];

    let enum_decoded: EnumToto = bincode::deserialize(&enum_encoded[..]).unwrap();
    println!("let enum_decoded = {:?};", enum_decoded);
    // prints:
    // let enum_decoded = Toto2;

    let struct_decoded: StructToto = bincode::deserialize(&struct_encoded[..]).unwrap();
    // panics:
    // thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Io(Kind(UnexpectedEof))', src/main.rs:42:80
    println!("let struct_decoded = {:?};", struct_decoded);

}

Adding new enum variants is OK but new fields makes the code panic. That would mean:

• non-breaking changes should be limited to adding new enum variants
• same KIM could be used but with protobuf types instead (but still breaking changes)