ironcorelabs / ironoxide Goto Github PK

As a developer using ironoxide in my application
I want to be able to share an IronOxde struct between threads
so that I am not forced to to create an IronOxide for each thread

--

AC:

• - a test exists demonstrating sharing a single IronOxide between two or more threads
• - there are no mut function params in IronOxide's public API

--

Some experimental work has been done in this are in https://github.com/IronCoreLabs/ironoxide/tree/use-no-mut-recrypt

As of #27 resolve_keys_for_grants now exists and is shared between encrypt_document and edek_encrypt_document. It seems like this new function could also be used in document_grant_access as well.

It may make sense to combine this work with that of #18

Over the past several PRs we've been trying out a process where the change log entry for a PR (if any) is added to CHANGELOG.md file so it can be reviewed along with the code.

This process is opposed to requiring a change log entry as part of a PR description, or just using the PR name and description to create a change log entry at the time of the release.

Do we like this process? If so, we should update RELEASING.md to reflect the new process.

Open Questions

• should the version number be assigned in the changelog based on the content of the PR? Or is that done only at the time of release (with a "Unreleased" section at the top of the change log file)? It seems like the next released version should always a single bump in one of the version numbers.
• how will we keep track of released vs unreleased versions in the change log?
• when we tag a release, what should go in the commit message?

As an application developer
I want to be able to specify the admins for a group at group creation time
so that I don't have to make additional calls to set up the administrators

AC:

• the calling user should not be be forced to be an admin or the "owner" of the group
• the SDK should enforce that at least one admin is specified

Here is a proposed design for GroupCreateOpts

pub struct GroupCreateOpts {
    // ... (non-pertinent) fields ommitted
    // None (default) - the server will make the creating user the owner
    // Some(UserId) - the provided UserId must be in the admin list, then will be passed to the server as the group owner.
    owner: Option<UserId>,
    // true (default) - creating user will be added to the group's list of administrations
    // false - creating user will not be added as the group's admin
    //         Note that the creating user did generate the group's private key so if the creating user
    //         Is not being added as an admin, it is recommended that `needs_rotation` be set to true
    //         so that the group's private key is rotated.
    add_as_admin: bool,
    // List of additional users to add to the group's administrators.
    admins: Vec<GroupAdmin>,
}

An earlier design can be viewed in #40

As an application developer
I want to be able to see if my user's key needs to be rotated on user verify

AC

• UserVerifyResult contains needs_rotation flag

Unsure if responses from GET users (User list) should also return this flag.

As an ironoxide user
I want SDK functions to execute quickly without significant uncessary overhead

As part of some experimentation with flame I pulled the Tokio Runtime out of each public API. I don't really know if that had a noticeable effect on performance, but it certainly felt cleaner. I had previously suspected that creating/destroying a Runtime might have significant overhead.

We have decided not to leave flame annotations in our normal code due to limitations with concurrent operations. The tokio runtime work should be pulled out of that branch and PR'd separately.

AC:

• All DeviceContext authenticated SDK functions use a shared Runtime struct
• cached Runtime allows for true multithreaded behavior

Non AC:

• JWT auth'd SDK functions are using a shared Runtime. We may choose to try to address this at some point, but might want to use a more sophisticated pattern (like what reqwest does).

If DocumentOps::document_encrypt is called with a PolicyGrant defined, but no policy has been set up on the server, document_encrypt will fail with IronOxideErr::RequestErr containing the message

Request failed with HTTP status code 'Some(404)' message 'Requested resource was not found.' and code 'PolicyGet'

It would be nice if the error gave a clearer indication of what the caller could do to resolve this issue. Perhaps something like:

No policy is defined. Please visit https://admin.ironcorelabs.com/policy to set up a policy.

Note that this error message is not applicable to all policy errors (like a policy being present, but not matching the PolicyGrant being used)

This api should be document_edek_encrypt(data: &[u8], encrypt_opts:EncryptOpts): (DocumentEncryptResult, Edeks) (Not sure about tuple vs newtype).

This api should generate the dek and encrypt using the normal workflow. Instead of sending that as a document create to the api, it should instead take all that data and encode it using the protobuf schema that we've defined. Return that directly to the user.

This is the first api where we'll be working with PB as well as declaring a new trait and refactoring the current encrypt code.

As a user
I want to be able to rotate my private key (without changing my public key)
so that I can be sure that anyone who had access to my private key previously, can't decrypt data

AC:

• new init function exists of the form pub fn initialize_check_rotation(device_context: &DeviceContext) -> Result<InitAndRotationCheck>
• new function in the user interface fn rotate_curr_users_priv_key(&self, password: &str) -> Result<UserPrivateKeyRotationResult>
• calling fn rotate_curr_users_priv_key should result in the user's private key being changed, and old version of the private key should be useless. Call the PUT users/{userId}/keys/{userKeyId} in the ironcore service.
• after the private key is rotated, existing device transform keys should continue to work
• after private key rotation, the needsRotation flag should be unset on subsequent initialilze calls (and on other calls that can get needsRotation

related to IronCoreLabs/ironweb#38

As an ironoxide consumer
I want the SDK to support reading and writing EDOC v3 format

AC

• ability to read/write v2 of encrypted data is maintained. Tests should ensure this remains true.
• EDOC pre-header should be implemented as descibed below
• EDOC header content should be defined in proto (optional -- could be delayed)

EDOC v3

Header

The encrypted doc will have a header similar in form to our current encrypted document headers written by the IronCore SDKs. This constitutes version 3 of our encrypted document header.

[header_version : IRON : header_size : header] : encrypted_doc

• header_version - one byte to represent the header version. starting at 3, we have two prior versions
• IRON - four bytes of ASCII magic to more well define our binary than a small number
• header_size - two bytes to represent the size of the header bytes to read after this, most significant byte first
• header - protobuf binary of our metadata

This should interop with our current header design. Version is still first and for version 3+ IRON will follow.

The structure of the protobuf EDOC header is yet undefined.

The Schnorr signature should be done using the group's private key over the group's provided id. This needs to be sent as signature in the request to add admins to a group.

As part of #27 an EncryptedDek type was created. This is a better packaging of the tuple (WithKey<UserOrGroup>, EncryptedValue) There are few usages and most/many of them could be replaced with EncryptedDek
`

As an application developer
I want to be able to create a cryptographic group before the admins and members have taken ownership of their keys
so groups can be created independently of the admins and members logging in and initializing the SDK

AC:

• GroupCreateOpts should contain needs_rotation, add_as_admin, members, and admins
• SDK should validate that at least one admin is has been specified (webservice should also be checking this)
• the first admin sent in the REST request will be considered to have special status (owner? -- if we keep this concept)
• result coming back from group_create should contain all the added fields. This probably means it should be (or be similar to) GroupGetResult

Prototype of what GroupCreateOpts might looks like. I've also included a concept for Admin/SuperAdmin that might augment or replace our current concept of Owner.

pub struct GroupCreateOpts {
    // unique id of a group within a segment. If none, the server will assign an id.
    id: Option<GroupId>,
    // human readable name of the group. Does not need to be unique.
    name: Option<GroupName>,
    // true (default) - creating user will be added to the group's membership (in addition to being the group's admin);
    // false - creating user will not be added to the group's membership
    add_as_member: bool,
    // true (default) - creating user will be added to the group's list of administrations
    // false - creating user will not be added as the group's admin
    //         Note that the creating user did generate the group's private key so if the creating user
    //         Is not being added as an admin, it is recommended that `needs_rotation` be set to true
    //         so that the group's private key is rotated.
    add_as_admin: bool,
    // List of additional users to add to the group's membership. Use `add_as_member` to add the current user
    members: Vec<UserId>,
    // List of additional users to add to the group's administrators.
    admins: Vec<GroupAdmin>,
    // true  - group is marked for private key rotation. The next time a group admin initializes
    //         the SDK, they will have the opportunity to perform the key rotation.
    // false (default) - group is not marked for private key rotation.
    needs_rotation: bool,
}

/// Permission levels of group administrator.
enum GroupAdmin {
    // can't be removed by Admins, but can be by SuperAdmins. Last SuperAdmin can't be removed.
    SuperAdmin(UserId),
    Admin(UserId),
}

As a ironoxide developer
I want to use Rust's upcoming async/await syntax for async code

AC:

• Determine the list of easily separable steps for upgrading from where we are to async/await
• Is futures 0.3 the right thing to be using?
• Are our dependencies all compatible with futures 0.3/async-await?

There is inconsistency among the public Result types as to if they provide Debug, Clone, and PartialEq. Unless there's a good reason, all of these types should provide instances these traits.

IronCore auth v2 API auth gives stronger guarantees about tamper evidence of recorded audit information as well as eliminates the possibility of replay-style attacks.

We will add a new header X-IronCore-User-Context which has the following fields comma separated. Timestamp, SegmentId, ProvidedUserId, SigningKey (as base64).
For example 1569357958000,100,foo_the_id_for_user,onYf/14UssVOqERTpAsgQPb4bxyfyHqWAW4pvFijjb0=

The first signature will be over the value of the X-IronCore-User-Context header.
let sig1 = ed25519_sign(utf8_encode(header_value(X-IronCore-User-Context)))

The authorization header will then be authorization IronCore 2.<base64(sig1)>.

The second signature will be over the following:
let sig2 = ed25519_sign(utf8_encode(header_value(X-IronCore-User-Context)) ++ utf8_encode(Method) ++ utf8_encode(path_url ++ query_params)) ++ body_bytes)

X-IronCore-Request-Sig = base64(sig2)

Note a few things about the values being fed in:

• we're using the literal string from the X-IronCore-User-Context. This means more bytes fed into the signature algorithm, but speeds up verification.
• method is case sensitive, but all methods are capitalized today (We'll use it "as is" when it comes in)
• path_url + query_params should be the entire url (starting with / after the hostname) and should include the ? and query string params. This value should signed over in its percent encoded form if the values in the path segments need to be url encoded.
  • path segments and query strings should be percent encoded using a very specific encode set as there seems to be differences between implementations. A-Z a-z 0-9 - _ . ! ~ * ' ( ) are the only ASCII characters we don't want to encode.
• If any of the values are missing an empty byte array should be used.
• Note that none of the keys for headers are included. This bypasses the fact that header keys are technically not case sensitive, but some libs treat them as case sensitive.
• provided ids cannot contain , so no valid providedId would ever cause the parsing of the X-IronCore-User-Context to fail.
• This should allow the server to check the signature without validating the type of the body (JSON vs protobuf vs raw bytes).

If either of the signatures fail validation the request will be rejected.

The service now supports the ability to use policy to determine who to encrypt to. In order to support this in IronOxide, we should add a function document_encrypt_via_policy which takes all the same options as document_encrypt, but instead of grants it takes 4 new options.

classification: Option<String>, sensitivity: Option<String>, data_subject: Option<String> substitute_id: Option<String> each of these do have limited character sets so if we want we could make newtypes that valid by construction.

The first 3 must follow the following regex: [A-Za-z0-9_-]{1,100}. The last one must be a valid user id in the IronCore service so taking User there instead of String would be preferable.

The api that should be called is GET /api/1/policys?classification=<value>&sensitivity=<value2>&dataSubject=<value3>&substitutionId=<id>. All values are optional and should not be included if they were None.

The return structure will be the following:

struct PolicyResult{
  users: List<IdAndKey>,
  groups: List<IdAndKey>,
  invalidUsers: List<ProvidedId>,
  invalidGroups: List<ProvidedId>
}

struct IdAndKey{
  id: ProvidedId,
  key: PublicKey
}

The invalidUsers and invalidGroups will be filled out if the policy resulted an ids that could not be found in the ironcore system, if they are empty all groups and users were successfully looked up.

As an IronCore developer
I want to be able to run the integration tests against any environment, including local
so that I'm not limited to testing against stage only.

AC
TODO

This issue should result in a write up of the viable PBKDF2 implementations we could use in ironoxide.

Our primary issue with the current (ring) implementation is that we suspect the performance could be better.

Doesn't have to be pure Rust because IronOxide doesn't need WASM support.

As an SDK method needing the current key id
I want to be able to get the current key id for the current user or for any group the current user is an admin of
so that I can use that key id as part of subsequent requests I'm going to make

AC:

• user's currentKeyId added to UserVerifyResponse, UserCreateResponse
• group's currentKeyid added to GroupBasicApiResponse, GroupApiResponse
• currentKeyIds should not be part of the public API

As an IronCore audit log user
I want to know what SDK and version make a request to the IronCore service

• so that SDKs with specific behaviors/defects can be recognized and isolated
• so that I can tell if there are out of date clients making requests for encrypted data

AC:

• Every REST request to the IronCore service should include a header X-IronCore-Sdk-Version. Values of the header should be of the form ironoxide <MAJOR>.<MINOR>.<PATCH> and should match the current version of the SDK sending the request.
• This header should be visible at the ironcore service and should be logged to the requests audit trail

This looks like a promising path: https://doc.rust-lang.org/cargo/reference/environment-variables.html

New boolean option, (grant_to_author default true) to DocumentEncryptOps that allows callers to not encrypt to the user performing the operation, but only to those users/groups in the provided grant list. Validation must then be performed to verify that at least one user/group is being encrypted to, that is, this flag cannot be turned off if the list of grants is empty.

As a ironoxide user
I want examples of how to use ironoxide to do basic operations

We need to decide which examples we want to provide. These should be annotated as they will form the front page of the ironoxide docs.rs site.

As a group admin
I want to know if the group I'm associated with needs to be rotated

AC:

• GroupMetaResult, GroupGetResult, GroupListResult contain "needsRotation" iff the caller is an admin of the group being queried

As an ironoxide user
I want SDK functions to execute quickly without significant unnecessary overhead

Currently we make a new Client for each request in rest.rs. According to the docs we should be reusing the client to take advantage of connection pooling. Hopefully this will make our requests much faster.

AC:

• A fixed number of Client instances are maintained inside of rest.rs

As an ironoxide consumer
I want to understand the threading behavior of of ironoxide
so that I can tune the performance (number of threads) in my application.

tokio is used, but a new runtime is created for each API call and there's nothing being done to constrain or define the threading behavior. The goal here is to understand what is being done and know what steps are necessary to make it better.

Changing it to a different approach would be a different issue(s).

document_decrypt_unmanaged(encrypted_data: &[u8], encrypted_deks: &[u8]): DocumentDecryptUnmanagedResult

This should take the edeks and send them to the edeks/transform endpoint. Assuming it comes back with a transformed EDEK, decryption should be the same as after the document GET call in the normal decrypt function.

See discussion at IronCoreLabs/ironoxide-swig-bindings#37 (comment)

This would apply to both DocumentEncryptResult and DocumentEncryptUnmanagedResult as well as (possibly) some internal data structures.

If this is done, tickets in ironoxide-java/scala should be created as well.

Add two new public methods to the User operations:

• device_sign_data
• device_verify_data

device_sign_data takes a borrowed reference to an array of bytes to sign. It should append a byte string containing the current device public signing key (base64 encoded), the current date-time (RFC3339 format), and the provided user ID, then use the current device's private signing key to generate the ed25519 signature (as a base64 encoded string) and return the byte string with the signing key date-time, provided user id, and signature.

device_verify_data takes borrowed reference to an array of bytes to verify and another borrowed reference to a string containing the public signing key, date-time, and provided user id, followed by the signature. It should extract the public key, pull off the signature, and validate the signature using the public key. Return boolean

The file internal/document_api/mod.rs has become very large and hard to navigate. We have a couple more functions that are planned, so it will only get worse.

Some ideas:

• perhaps using sub-modules that split functionality along functional boundaries (similar to what we do in the document_api/requests would be nice? Separate files (per module) could also be used.
• perhaps public result types should be collected in a module?
• reorganizing of the file might also help. All structs at the top; private functions at the bottom, etc
• make sure all naming is consistent (encrypt_document vs document_get_metadata)

Should not require the device ID to initialize the SDK. Remove it from the deviceContext structure. Requires a breaking change to DeviceContext to remove device ID.

In our AES encrypt method in crypto/aes.rs, we're cloning the provided plaintext vector so we can resize it to add 16 bytes to it to support adding on the GCM auth tag. As people could be encrypting large large documents with the SDK, doing a clone is bad. We need to figure out a way to avoid it.

It would also be nice to see if we can have the various AES methods take slices instead of vecs as some inputs to encrypt have to be copied to get them into vectors. See this code review comment

As an application developer
I want to be able to create a cryptographic group before the admins and members have taken ownership of their keys
so groups can be created independently of the admins and members logging in and initializing the SDK

AC:

• GroupCreateOpts should contain a needs_rotation option
• Result coming back from group create should contain the needs_rotation flag

The Schnorr signature should be done using the group's private key over the group's provided id. This needs to be sent as signature in the request to add members to a group.

The current DocumentOps::document_encrypt allows the caller to specify user/groups by ID. The service then looks up the public keys for the users/groups for encryption.

It would be nice if we provided an interface that also allowed encryption directly to public keys (if the caller already has them). This will save 2 calls to the service.

An internal API already exists with (almost) this functionality. See internal::document_api::recrypt_document

There are other sets of steps which produce this same result, but this is the simplest IMO.

1. Initialize IronOxide.
2. Call document_encrypt with DocumentEncryptOpts which only has ExplicitGrants{grant_to_author:false, grants: [UserOrGroup::User("does_not_exist")}

Result: The function will error with grants' failed validation with the error 'Access must be granted to document DocumentId("XXXXXXXXXXXXXXXX") by explicit grant or via a policy'

Expected result: The function should error telling you which people it tried to share with and why that didn't work. It's right to error all the way out, but since I did send in a valid ExplicitGrant I shouldn't get this error. As the caller I'm confused because I did send an ExplicitGrant, it's just a grant that didn't result in valid users.

I think we should add an error that catches the case and tells them what users or groups we tried to share with and why it couldn't be successful.

As a group admin
I want to be able to rotate my private key (without changing my public key)
so that I can be sure that anyone who had access to the old private key can't administer the group after rotation.

AC:

• if the user elects to check for group private key rotation on initialization, groups that need rotation will be returned to them
• new function exists in the group interface fn rotate_group_priv_key(id: GroupId) -> Result<GroupPrivateKeyRotationResult>
• calls the PUT groups/{groupId}/keys/{groupKeyId} in the ironcore service
• after the group's private key is rotated prior group private keys should not be able to be used for group administration operations
• group's currentKeyid added to GroupBasicApiResponse, GroupApiResponse (currentKeyId should not be part of the public API)

currently blocked by IronCoreLabs/recrypt-rs#83

Check other dependencies as well.

We are going to standardize the DeviceContext/DeviceKeys JSON format across all SDKs. The proposed format (in Typescript) is:

{
    deviceId: number;
    accountId: string;
    segmentId: number;
    devicePrivateKey: Base64String;
    // “expanded private key” (both pub/priv)
    signingPrivateKey: Base64String; 
}

see: IronCoreLabs/ironnode#27

As a ironoxide user
I want to understand the performance characteristics of unmanaged encryption

This ticket is sort of a test-balloon to see if using criterion for benchmarking ironoxide public APIs is feasible.

When cargo package is run. the transform.rs file is flagged because it's outside the OUT_DIR.

See https://docs.rs/crate/ironoxide/0.12.0/builds/191826

When running IRONCORE_ENV=http://127.0.0.1:9090/api/1/ cargo test, there is about a 50% chance that exactly 2 tests will fail. It is not the same 2 every time, but it is always exactly 2.
Error:
---- group_add_admin_on_create stdout ---- thread 'group_add_admin_on_create' panicked at 'called Result::unwrap() on an Err value: RequestServerErrors { errors: [ServerError { message: "Validating user signature failed with \'Signature does not match the new device signed with user\'s key.\'", code: 4 }], code: UserDeviceAdd, http_status: Some(403) }', src/libcore/result.rs:1187:5

Edit: I no longer think it's always 2, though it does seem to happen in pairs most often.

We have some identical or nearly identical types in the request layer that could be cleaned up and/or unified.

Note that

Examples:

In group_api/requests

#[derive(Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
pub struct SuccessRes {
    pub(crate) user_id: UserId,
}

#[derive(Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
pub struct FailRes {
    pub(crate) user_id: UserId,
    pub(crate) error_message: String,
}

#[derive(Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
pub struct GroupUserEditResponse {
    pub(crate) succeeded_ids: Vec<SuccessRes>,
    pub(crate) failed_ids: Vec<FailRes>,
}

In document_api/requests

        #[derive(Deserialize, Debug)]
        #[serde(rename_all = "camelCase")]
        struct SuccessRes {
            pub(crate) user_or_group: UserOrGroupRes,
        }

        #[derive(Deserialize, Debug)]
        #[serde(rename_all = "camelCase")]
        struct FailRes {
            pub(crate) user_or_group: UserOrGroupRes,
            pub(crate) error_message: String,
        }

        #[derive(Deserialize, Debug)]
        #[serde(tag = "type", rename_all = "camelCase")]
        enum UserOrGroupRes {
            User { id: String },
            Group { id: String },
        }

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[serde(rename_all = "camelCase")]
pub struct AccessGrant {
    pub(crate) user_or_group: UserOrGroupWithKey,
    #[serde(flatten)]
    pub(crate) encrypted_value: EncryptedOnceValue,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[serde(rename_all = "camelCase", tag = "type")]
pub enum UserOrGroupWithKey {
    #[serde(rename_all = "camelCase")]
    User {
        id: String,
        // optional because the resp on document create does not return a public key
        master_public_key: Option<PublicKey>,
    },
    #[serde(rename_all = "camelCase")]
    Group {
        id: String,
        master_public_key: Option<PublicKey>,
    },
}

Notice that UserOrGroupWithKey's master_public_key is optional to support the response coming back from Document create. Document access revoke and other group apis may also be using similar types.

As a ironoxide developer,
I want the internal APIs to use async fn instead of Future (0.1)
so that the code will be easier to maintain and integrate with other async Rust libraries

AC:

• User API
• user/requests
• Group API
• group/requests
• Document API
• document/requests
• rest.rs

Not included in this issue:

• creating a public async API
• trying to cache the tokio runtime
• trying to cache reqwest clients

The IronCore web-service defines segment_id to be an i32 so declaring it as a usize is misleading.

As a group admin
I want to be able to transfer ownership of a group to another group admin
so that I can be removed from the admin list, even if I created the group.

AC:

• new method in GroupOps. Suggested sig:

fn group_transfer_ownership(group_id: &GroupId, new_owner: &UserId) -> Result<GroupTransferOwnershipResult>

• only the current owner of the group should be able to transfer ownership of a group
• the new owner must already be a group admin of the group
• after transferring the (now) non owner should still be an admin of the group
• the previous owner should be able to remove themselves as a group admin. This could be added as a flag on group_transfer_ownership if we think it's common enough.

As as application developer
I want to be able to specify the initial group members on create
so that I don't have to make additional calls to set up the membership

AC:

• GroupCreateOpts should contain both add_as_member to mark the calling user as a member and members: Vec<UserId> to allow other members to be added
• return structure for group create should reflect the membership.

I'm uncertain if the web service supports partial failure for adding members on create. If it does, the users that failed to be added to the membership on create should also be included in the result.

As an SDK user
I want to be able to set timeout on SDK calls
so that I can configure the maximum time I'm willing to wait and handle a timeout in a reasonable manner.

--

Likely, we would make this a global timeout for all operations.

As an application developer
I want to be able to create a user's cryptographic identity ahead of them logging in
so that I can add them to groups and/or share data with them.

AC:

• - user_create takes an options object that allows a user to created with a flag set marking them as needing rotation
• - needsRotation is returned as part of the value returned to the caller

Create a new type for the data returned by createDevice, which will include more fields (name, created, updated).

This type will be a superset of the fields in the device context - write an into method that will create a device context from the device type.