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

Add an OpenSSL harness #13

Merged
merged 22 commits into from
Jul 5, 2023
Merged

Add an OpenSSL harness #13

merged 22 commits into from
Jul 5, 2023

Conversation

woodruffw
Copy link
Collaborator

@woodruffw woodruffw commented Jul 4, 2023

Still a WIP. This just gives us another thing to sanity-check against.

Closes #14.

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw self-assigned this Jul 4, 2023
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Just a convenient helper for debugging.

Signed-off-by: William Woodruff <william@trailofbits.com>
This testcase doesn't rely on an intermediate CA being a leaf,
so don't use one as such.

Signed-off-by: William Woodruff <william@trailofbits.com>
These didn't need to be full module paths.

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Should be None, not an empty list.

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw marked this pull request as ready for review July 5, 2023 15:44
@woodruffw woodruffw changed the title [WIP] Add an OpenSSL harness Add an OpenSSL harness Jul 5, 2023
@woodruffw woodruffw requested a review from tnytown July 5, 2023 15:44
@woodruffw
Copy link
Collaborator Author

This is a chunky PR, but most of it is two single-header libraries that can be ignored for review purposes.

Key changes:

  1. We now have another test harness, this one for OpenSSL.
  2. We now have JSON schema support for testcase "result" models.
  3. The OpenSSL harness emits testcase results in the format specified by (2), to a ./results.json file.

I'm thinking that, once this is merged, we should probably update the crypto/x509 harness to behave similarly. That way, we can begin comparing the two in a more structured manner (as well as putting them into CI in a more structured manner than just uncoordinated running). Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NB: Got lazy; this assumes macOS with Homebrew at the moment.

@alex
Copy link
Collaborator

alex commented Jul 5, 2023

When you say a schema for results, do you mean expected results (i.e., "This testcase should validate, this one should fail") or do you mean for the results of running a test?

@woodruffw
Copy link
Collaborator Author

When you say a schema for results, do you mean expected results (i.e., "This testcase should validate, this one should fail") or do you mean for the results of running a test?

The latter -- the testcase schema contains the expected results (via expected_result), and the "results" schema contains the "actual" results (via actual_result). Terrible naming, sorry.

@alex
Copy link
Collaborator

alex commented Jul 5, 2023 via email

@woodruffw
Copy link
Collaborator Author

Why do we want/need that? Seems like how you consume results will be very project specific (e.g., for pyca/cryptography it'll just be part of our pytest suite)

I don't think it'll useful for pyca/cryptography's suite, but my thinking was that it'd be useful for quickly diffing/visualizing the expected behavior here (i.e. as we're getting stood up, it'll be nice to have a structured way to compare how Go and OpenSSL behave differently).

In other words I think end consumers probably won't need the result models; they're mostly just useful for our own CI/diagnosing as we build out testcases here.

@woodruffw
Copy link
Collaborator Author

Relaying from chat: @alex brought up a good point that the result models probably won't be externally useful, so I'm going to exclude them from the generated limbo-schema.json.

These are only useful within our own repo.

Signed-off-by: William Woodruff <william@trailofbits.com>
@tnytown
Copy link
Contributor

tnytown commented Jul 5, 2023

the result models probably won't be externally useful, so I'm going to exclude them from the generated limbo-schema.json.

One thing to note is that the Go implementation parses the test cases with structs automatically generated from JSON schema. We can use an unstructured map[string]interface{} for results or write some structs manually, but I'd prefer some kind of synchronized JSON structure.

Dumb question: other than the test harnesses, where is the Limbo schema going to be used?

@woodruffw
Copy link
Collaborator Author

We can use an unstructured map[string]interface{} for results or write some structs manually, but I'd prefer some kind of synchronized JSON structure.

Let's write them manually -- we don't expect this result schema to change significantly and, if it does, changes to it should be considered part of our internal use.

Dumb question: other than the test harnesses, where is the Limbo schema going to be used?

Not a dumb question! The ultimate idea here is to make both limbo.json and limbo-schema.json reusable within third-party libraries/X.509 validation implementations. In other words: our own harnesses won't be the source of ground truth; each upstream should be consuming the testcases and integrating them into their own testsuites.

That's the medium-term plan with pyca/cryptography as well.

Copy link
Contributor

@tnytown tnytown left a comment

Choose a reason for hiding this comment

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

Not very familiar with the OpenSSL APIs. Looks good overall, build nits notwithstanding.

harness/openssl/Makefile Outdated Show resolved Hide resolved
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Collaborator Author

@tnytown JFYI, I've written an informal description of each harness's expected behavior here: 55b3f5c

@woodruffw woodruffw merged commit 996f42c into main Jul 5, 2023
3 of 4 checks passed
@woodruffw woodruffw deleted the ww/harness-openssl branch July 5, 2023 16:58
carl-wallace added a commit to baloo/x509-limbo that referenced this pull request May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define a testcase result model/schema
3 participants