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

tests: add tests for newly introduced primitives #46

Open
tshakalekholoane opened this issue Jun 28, 2023 · 2 comments
Open

tests: add tests for newly introduced primitives #46

tshakalekholoane opened this issue Jun 28, 2023 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@tshakalekholoane
Copy link
Collaborator

tshakalekholoane commented Jun 28, 2023

Is your feature request related to a problem? Please describe.

The ECRYPT-CSA report specified additional primitives (see commits below) that aren't mentioned in all the other standards and thus do not have corresponding tests in these modules.

Commits:

Describe the solution you'd like

Write tests for these.

Describe alternatives you've considered

Leave it as is. The return value is the same in all of them given they'll be assessed against an exception list that already specifies primitives that the standard explicitly mentions so the value add seems minimal. Nonetheless it might be a good thing to have.

@tshakalekholoane tshakalekholoane added enhancement New feature or request good first issue Good for newcomers labels Jun 28, 2023
@tshakalekholoane tshakalekholoane added this to the 0.1.0 milestone Jun 28, 2023
@tshakalekholoane tshakalekholoane moved this to 📋 Backlog in wardstone Jun 28, 2023
@tshakalekholoane tshakalekholoane removed this from the 0.1.0 milestone Jul 7, 2023
@ndcroos
Copy link

ndcroos commented Aug 31, 2023

Hi, I am interested in this, but I am not sure how tests for these should look like, or how to go about this task. Can you elaborate on that, or give an example?

@tshakalekholoane
Copy link
Collaborator Author

Hi. Thank you for your interest.

To give an overview, the core crate has primitive instances that are supposed to represent their real-world equivalents. So when one scans a TLS certificate or SSH key using the cmd/wardstone crate, it parses that key and creates these internal representations from that information.

These primitive representations are then what gets passed in individually to functions that assess their compliance. For example, something like the test id_ecdsa_256_wardstone.pub public key would be translated into the NIST P-256 elliptic curve key representation and that would then get passed into the validate_ecc function of any type that implements the Standard trait.

(The types that implement this trait in this crate are all in the crates/core/src/standard directory).

But now the issue is some of these standards were implemented before we had an "exhaustive" list of primitives. Therefore, some unit tests do not capture these newer primitives. For example, the is no unit test in the NIST standard that assesses the BLAKE3 hash function.

So in order to improve the test coverage, this would require going through all the standards test modules in the crate, finding primitive instances that do not have test cases, and then writing tests for those using macros that are aimed at simplifying this task just as I showed above.

At this point I'd expect the logic to be solid enough that anyone working on this issue wouldn't need to verify the result against the relevant publication. So it's largely just a matter of manually going through the test modules and writing tests for primitives that are not captured.

Does that help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants