Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Kudelski Audit Tasks #2461

Open
1 of 5 tasks
trevor-crypto opened this issue Jan 19, 2021 · 0 comments
Open
1 of 5 tasks

Kudelski Audit Tasks #2461

trevor-crypto opened this issue Jan 19, 2021 · 0 comments

Comments

@trevor-crypto
Copy link
Contributor

trevor-crypto commented Jan 19, 2021

Draft of findings for Crypto.com
Signatory, Remote Attestation, and KMS

3.1 No nonce implemented

Finding ID: KS-CRYPTOCOM-F-08
Severity: [Low]
Status: [Open]
Description
The nonce field in the attestation evidence data structure is set to none upon initialization and then never updated before it is sent to the IAS server.
Proof of Issue
Filename: thaler/chain-tx-enclave-next/ra-common/src/sp/attestation_evidence.rs
Beginning Line Number: 17
Severity and Impact Summary
According to Intel, this field can be used to ensure that an attestation verification report cannot be used in a replay attack. Leaving this optional value unused increases vulnerability to replay attacks.
Recommendation
Implement nonce for attestation evidence or document why this is not needed.
References
https://api.trustedservices.intel.com/documents/sgx-attestation-api-spec.pdf (section 4.1)

3.2 Private key exposed as public in struct

Finding ID: KS-CRYPTOCOM-F-09
Severity: [Low]
Status: [Open]
Description
Fields for private keys are exposed as public in the definition of a struct, letting users of the module access it freely. If the interface allows a user to directly access sensitive data, it may allow unsafe usage of the key when using the module.
Because the field is never used inside the module, it is assumed this functionality is not needed.
Proof of Issue
Filename: thaler/chain-tx-enclave-next/ra-enclave/src/certificate.rs
Beginning Line Number: 12
Severity and Impact Summary
A user of the module may expose a private key as it is presented as exposed piece of data.
Recommendation
Mark the field as private (non-public) and implement the signing logic on to the data type. This hides any usage and handling of the private key and every user of the module does not need to do it themselves.

3.3 Tests missing

Finding ID: KS-CRYPTOCOM-F-10
Severity: [Low]
Status: [Open]
Description
The module ra-enclave does not include a test suite.
Proof of Issue
No tests in the specified module.
Filename: thaler/chain-tx-enclave-next/ra-enclave/src/*.rs
Beginning Line Number: -
Severity and Impact Summary
Without a test suite it is difficult to reason about the correctness of the implementation. Additionally, any changes to the code could introduce errors which may remain unnoticed until an actual error occurs.
Recommendation
Create a test suite which includes test cases for correct data in a favorable environment as well as invalid data in an unfavorable environment to verify the continuous validity of the module.

3.4 Confusing usage of optional type

Finding ID: KS-CRYPTOCOM-F-11
Severity: [Informational]
Status: [Open]
Description
In the definition of EnclaveInfo, the field mr_enclave has an optional type, in which there are separate and specific meanings to Some and None respectively. Giving meaningful purpose to a None-value may be confusing for maintainers and reviewers.
Giving explicit meaning to a None-value often requires a lot of explanations whenever it is used.
Proof of Issue
Filename: thaler/chain-tx-enclave-next/ra-client/src/config.rs
Example: thaler/chain-tx-enclave-next/ra-client/src/verifier.rs
Beginning Line Number: 66, 263-286
Severity and Impact Summary
Confusing for users of the module and maintainers.
Recommendation
Make use of an Enum type to directly describe each scenario of its value.

3.5 No validation on certificate

Finding ID: KS-CRYPTOCOM-F-12
Severity: [Low]
Status: [Open]
Description
The implementation of the trait ServerCertVerifier for the type EnclaveCertVerifier does not follow the trait documentation. The documentation clearly states that the given certifications should be verified against the given root certificate. The implementation does not perform this verification thus the presented certificates are blindly trusted.
Proof of Issue
Filename: thaler/chain-tx-enclave-next/ra-client/src/verifier.rs
Beginning Line Number: 336
Severity and Impact Summary
Without verification, the presented certifications are blindly trusted and a fraudulent certification will be accepted.
Recommendation
Perform verifications according to the trait’s documentation. If it is not actually necessary, the reason needs to be explained in comments and documentation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant