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

Generate DC test vectors and add other signature schemes #35

Merged
merged 8 commits into from
Feb 22, 2021

Conversation

claucece
Copy link
Collaborator

@claucece claucece commented Feb 10, 2021

Open for comments ;)

cc./ @cjpatton @chris-wood @xvzcf

@xvzcf
Copy link
Owner

xvzcf commented Feb 10, 2021

To me, it seems like -make-dcvectors and -make-dc are the same thing. Also, instead of a -dc-algo argument, we could have an -alg argument that is passed to make-dc, make-root, etc. I say alg and not sig-alg because we might potentially wrap post-quantum KEMs in DCs.

Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

To me, it seems like -make-dcvectors and -make-dc are the same thing. Also, instead of a -dc-algo argument, we could have an -alg argument that is passed to make-dc, make-root, etc. I say alg and not sig-alg because we might potentially wrap post-quantum KEMs in DCs.

I agree .... the new option is much more general. Though I would just stick with "-alg" or "-dc-algo" and not differentiate between KEM/signatures at this stage.

@claucece
Copy link
Collaborator Author

To me, it seems like -make-dcvectors and -make-dc are the same thing. Also, instead of a -dc-algo argument, we could have an -alg argument that is passed to make-dc, make-root, etc. I say alg and not sig-alg because we might potentially wrap post-quantum KEMs in DCs.

I like that of the -alg argument. Around the -make-dc one: I wanted to separate between the vectors and the dc used to run the test. But it could also be that the all of them are generated and the test just chooses one.

@xvzcf
Copy link
Owner

xvzcf commented Feb 11, 2021

I like that of the -alg argument. Around the -make-dc one: I wanted to separate between the vectors and the dc used to run the test. But it could also be that the all of them are generated and the test just chooses one.

Yeah, I was thinking the latter. For each testcase, the CI runner will dynamically generate the artifacts, run a client-server interaction and then clean up by deleting the artifacts. If (for example) signature algorithms for the root, intermediate cert, and DC are chosen randomly for each testcase run, this might get us decent coverage of the many possible combinations without too much additional complexity.

@claucece
Copy link
Collaborator Author

@xvzcf @cjpatton check how it is working now. It is not so simple to choose randomly a cert, as it has to be a valid cert for the dc that are generated upon them. For the moment, it is randomly choosing a DC.

@claucece
Copy link
Collaborator Author

This is done now. I added some todos regarding:

  • Generating the randomness for the algorithms to be chosen should be done by golang itself as discussed with @xvzcf
  • Generating certificates with all other tls 1.3 supported algorithms need to be supported.

I can send that as part of other PRs.

Let me know what you think @xvzcf

)

type ECDSASigner struct {
// Signer represents an structure holding the signing information
type Signer struct {
Copy link
Owner

Choose a reason for hiding this comment

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

This should be fine for now, but I'm guessing once we handle RSA we should probably have a separate struct for that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could also be that we have a bool for each case: isRSA, is ed25519 and curve param for eddsa.

Copy link
Owner

@xvzcf xvzcf left a comment

Choose a reason for hiding this comment

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

As discussed, the TODOs can be addressed once I break out the common code into an internal library.

@xvzcf xvzcf merged commit 632b830 into xvzcf:main Feb 22, 2021
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.

3 participants