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 different certificate signature algorithms to benchmarks #4080

Merged
merged 16 commits into from
Jul 25, 2023

Conversation

tinzh
Copy link
Contributor

@tinzh tinzh commented Jun 29, 2023

Description of changes:

Add RSA2048, RSA4096, and EC384 certificates to benchmarks.

Testing:

Add certificate signature algorithms to existing handshake test.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@tinzh tinzh requested review from maddeleine and jmayclin June 29, 2023 19:07
@tinzh tinzh marked this pull request as ready for review June 29, 2023 19:11
@tinzh tinzh requested a review from lrstewart July 12, 2023 19:09
@tinzh tinzh requested a review from maddeleine July 19, 2023 17:58
@tinzh tinzh requested a review from maddeleine July 19, 2023 18:49
@tinzh tinzh requested a review from jmayclin July 19, 2023 18:59
CACert => "ca-cert.pem",
};

let dir = match sig_type {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make more sense to just implement Display for signature type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Display would be slightly misleading, since when you print (for example) CACert you would probably expect something like "CACert" or "root certificate", not the filename associated with the variant, which would be "ca-cert.pem" in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, I was just thinking that CACert could display as ca-cert. Or more generally, the filename that corresponds to that particular enum should be information associated with the enum, rather than information associated with this helper function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense that PemType and SigType should own the information about themselves. Since the full path to a cert (ex. certs/ec384/server-key.pem) doesn't make sense to be owned by either one of them, I still want to keep the get_cert_path() function but just move the information about each enum to an impl for each enum (get_filename() for PemType and get_dir_name() for SigType).

@tinzh tinzh requested a review from jmayclin July 19, 2023 19:29
cert-gen ec 384
cert-gen rsa 2048
cert-gen rsa 3072
cert-gen rsa 4096

Copy link
Contributor

@maddeleine maddeleine Jul 21, 2023

Choose a reason for hiding this comment

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

It might be time to add a clean function in this script that deletes all the generated pems. I'm thinking something like:

./generate_certs.sh clean

Do you think that would be useful? Otherwise the PR lgtm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it wouldn't hurt, and the control flow is fairly easy to understand; just added it.

@tinzh tinzh requested a review from maddeleine July 21, 2023 21:39
@tinzh tinzh merged commit 6881358 into aws:main Jul 25, 2023
@tinzh tinzh deleted the cert-algs branch July 25, 2023 17:12
goatgoose added a commit to goatgoose/s2n-tls that referenced this pull request Jul 31, 2023
commit eafb8a2
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 18:05:50 2023 -0400

    shell -> bash

commit 12071b8
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 17:59:09 2023 -0400

    add ubuntu quickstart back to readme

commit 10bf557
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 17:52:06 2023 -0400

    fixes

commit 74adf8d
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 16:46:19 2023 -0400

    fixes

commit 0548d07
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 16:43:08 2023 -0400

    consolidate

commit cbe8f2d
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 14:55:30 2023 -0400

    remove old doc sections

commit f194321
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 12:25:28 2023 -0400

    more content

commit 882eb1d
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 09:08:24 2023 -0400

    fixes

commit ce37d0e
Author: Sam Clark <[email protected]>
Date:   Mon Jul 31 09:03:45 2023 -0400

    fixes

commit 011d15f
Author: Sam Clark <[email protected]>
Date:   Sat Jul 29 22:59:51 2023 -0400

    cmake consuming

commit 7feadc1
Author: Sam Clark <[email protected]>
Date:   Sat Jul 29 22:27:02 2023 -0400

    fixes

commit 2914950
Author: Sam Clark <[email protected]>
Date:   Sat Jul 29 21:34:24 2023 -0400

    traditional make

commit 02f9841
Author: Sam Clark <[email protected]>
Date:   Sat Jul 29 19:45:43 2023 -0400

    s2n-tls build section

commit 86c4983
Author: Sam Clark <[email protected]>
Date:   Sat Jul 29 11:56:32 2023 -0400

    Update build documentation

commit ea6d02a
Author: Sam Clark <[email protected]>
Date:   Fri Jul 28 16:49:21 2023 -0400

    bindings: release 0.0.35 (aws#4122)

commit 35d08ba
Author: Justin Zhang <[email protected]>
Date:   Fri Jul 28 12:31:21 2023 -0700

    refactor(bench): separate out client and server connections in benching harness (aws#4113)

    Enables more better control of connections for benching experiments

commit 65e74ca
Author: Lindsay Stewart <[email protected]>
Date:   Wed Jul 26 02:26:40 2023 -0700

    Print error for 32bit test (aws#4107)

commit b0b253e
Author: toidiu <[email protected]>
Date:   Wed Jul 26 00:30:44 2023 -0700

    ktls: set keys on socket and enable ktls (aws#4071)

commit 403d5e6
Author: Lindsay Stewart <[email protected]>
Date:   Tue Jul 25 16:03:09 2023 -0700

    Trying to use an invalid ticket should not mutate state (aws#4110)

commit bce2b1a
Author: James Mayclin <[email protected]>
Date:   Tue Jul 25 14:44:33 2023 -0700

    fix: get_session behavior for TLS 1.3 (aws#4104)

commit 6881358
Author: Justin Zhang <[email protected]>
Date:   Tue Jul 25 10:10:21 2023 -0700

    feat(bench): add different certificate signature algorithms to benchmarks (aws#4080)

commit aab13d5
Author: Justin Zhang <[email protected]>
Date:   Mon Jul 24 18:17:30 2023 -0700

    feat(bench): add memory bench with valgrind/massif (aws#4081)

commit 20b0174
Author: Justin Zhang <[email protected]>
Date:   Mon Jul 24 13:26:32 2023 -0700

    feat(bench): add historical performance benchmark (aws#4083)

commit 5cc827d
Author: Doug Chapman <[email protected]>
Date:   Thu Jul 20 11:50:50 2023 -0700

    nix: pin corretto version (aws#4103)
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