Skip to content

[TLS 1.3] Client Authentication in the main Handshake#2957

Merged
reneme merged 6 commits intomasterfrom
tls13/client_auth
Oct 12, 2022
Merged

[TLS 1.3] Client Authentication in the main Handshake#2957
reneme merged 6 commits intomasterfrom
tls13/client_auth

Conversation

@hrantzsch
Copy link
Copy Markdown
Collaborator

@hrantzsch hrantzsch commented Apr 13, 2022 •
edited by reneme
Loading

Pull Request Dependencies

Before merging this, we should first review merge:

Description

This implements:

  • client authentication during the main handshake
  • coalescing of multiple handshake messages (which is required for the RFC 8448 client auth test) into a single (encrypted) record

Post-handshake authentication is left for future work. Rationale: A server may always ask for client authentication during the handshake. Post-handshake auth can be disabled by the client, by not negotiating the "post_handshake_auth" extension in the Client Hello.

TODO

  • We should explicitly support the "signature_algorithm_cert" extension
    The client can choose to not use this extension. Hence its support becomes crucial when implementing the server only.

@reneme reneme mentioned this pull request Apr 13, 2022
Merged
16 tasks
@reneme reneme force-pushed the tls13/client_auth branch from 20b48d3 to 9f25710 Compare April 17, 2022 11:03
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 19, 2022

This pull request introduces 1 alert when merging 348a5bb into 8976bf2 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@reneme reneme force-pushed the tls13/client_auth branch from 348a5bb to c22912f Compare April 19, 2022 11:49
@reneme reneme marked this pull request as ready for review April 19, 2022 12:01
@reneme reneme changed the title [TLS 1.3] Client Auth [TLS 1.3] Client Authentication in the main Handshake Apr 19, 2022
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 19, 2022

This pull request introduces 1 alert when merging c22912f into 8976bf2 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@reneme reneme force-pushed the dev/tls-13 branch 4 times, most recently from 00b35a6 to 2e01455 Compare April 27, 2022 11:21
@reneme reneme force-pushed the tls13/client_auth branch 2 times, most recently from f98334d to 3abfc13 Compare April 27, 2022 12:16
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 27, 2022

This pull request introduces 1 alert when merging 3abfc13 into 45b74cc - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@reneme reneme force-pushed the tls13/client_auth branch from 3abfc13 to 9549e06 Compare May 18, 2022 11:38
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 18, 2022

This pull request introduces 1 alert when merging 9549e06 into bae64de - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

reneme
reneme reviewed May 27, 2022
View reviewed changes
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 27, 2022

This pull request introduces 1 alert when merging 3ed31d4 into bae64de - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 27, 2022

This pull request introduces 1 alert when merging 86bacc1 into bae64de - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@reneme reneme force-pushed the tls13/client_auth branch from 86bacc1 to 08efbc2 Compare June 1, 2022 16:39
@reneme reneme force-pushed the tls13/client_auth branch from 9a564e5 to cf2928a Compare July 5, 2022 08:09
@reneme reneme changed the base branch from dev/tls-13 to master July 5, 2022 08:09
@reneme
Copy link
Copy Markdown
Collaborator

reneme commented Jul 5, 2022

Rebased and retargeted to master.

reneme and others added 3 commits July 6, 2022 14:32
@reneme
AggregatedMessages helper class to coalesce messages
dceb6ff 
Co-Authored-By: Hannes Rantzsch <hannes.rantzsch@nexenio.com>
@reneme
Rename Handshake_State_13::sent -> ::sending
9ed8346 
After handshake messages are created they are dropped into this method for
book keeping and compile-time message type checking. Afterwards messages are
further processed and eventually sent. Hence, the old method name was fairly
misleading.

Co-Authored-By: Hannes Rantzsch <hannes.rantzsch@nexenio.com>
@reneme
TLS 1.3 Client Authentication in main handshake
0da7f0c 
This allows for client authentication using certificates in the main handshake.
Post-handshake authentication is out of scope of this commit

Co-authored-by: René Meusel <rene.meusel@nexenio.com>
@reneme reneme force-pushed the tls13/client_auth branch from cf2928a to 8a1ad51 Compare July 6, 2022 12:32
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 6, 2022 •
edited
Loading

Codecov Report

Base: 92.57% // Head: 92.55% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (ffe1c30) compared to base (987c7af).
Patch coverage: 92.68% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2957      +/-   ##
==========================================
- Coverage   92.57%   92.55%   -0.02%     
==========================================
  Files         596      600       +4     
  Lines       69729    70073     +344     
  Branches     6613     6625      +12     
==========================================
+ Hits        64552    64858     +306     
- Misses       5144     5182      +38     
  Partials       33       33
Impacted Files Coverage Δ
src/lib/tls/tls_extensions.cpp 91.40% <75.00%> (-0.68%) ⬇️
src/lib/tls/tls13/msg_certificate_req_13.cpp 80.00% <80.00%> (ø)
src/tests/test_tls_rfc8448.cpp 91.26% <92.20%> (+0.97%) ⬆️
src/lib/tls/msg_cert_verify.cpp 94.25% <93.54%> (-1.21%) ⬇️
src/lib/tls/tls13/tls_client_impl_13.cpp 92.50% <97.56%> (+1.14%) ⬆️
src/bogo_shim/bogo_shim.cpp 88.85% <100.00%> (+0.03%) ⬆️
src/lib/tls/msg_cert_req.cpp 89.85% <100.00%> (ø)
src/lib/tls/tls12/tls_client_impl_12.cpp 91.54% <100.00%> (ø)
src/lib/tls/tls12/tls_handshake_state.cpp 86.13% <100.00%> (ø)
src/lib/tls/tls12/tls_server_impl_12.cpp 88.41% <100.00%> (ø)
... and 44 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@reneme reneme added this to the Botan 3.0.0 milestone Sep 20, 2022
lieser
lieser reviewed Oct 11, 2022
View reviewed changes
@reneme reneme merged commit 6b36f30 into master Oct 12, 2022
@randombit randombit deleted the tls13/client_auth branch December 1, 2022 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reneme reneme reneme left review comments

@lieser lieser lieser approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Botan 3.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@hrantzsch @reneme @codecov-commenter @lieser