search

webrtc-rs / dtls

A pure Rust implementation of DTLS
https://webrtc.rs
Apache License 2.0
42 stars 23 forks source link

Use the negotiated algorithm for key verification #16

Closed robashton closed 2 years ago

robashton commented 2 years ago

As intimated in https://github.com/webrtc-rs/webrtc/issues/184, we were using the signature algorithm on the supplied certificate to decision on key verification.

That is probably incorrect. In this pull request I

1) Bundle up the hash/signature algorithm where we use both of them 2) Supply this bundled hash/signature to the key verification function 3) Use this to determine what verification method we should be using

I have tested this locally with

1) Chrome egest 2) Chrome ingest 3) Firefox egest 4) Firefox ingest 5) Millicast egest

And the right decision is being made.

There is a chance that this also fixes #15, but this is only a hypothesis and isn't a guarantee.

robashton commented 2 years ago

I see I forgot to update the tests, I'll sort this out in due course (tomorrow morning, I'm done with work for the day!)

rainliu commented 2 years ago

Thank you for this PR, this will help improve the robustness of DTLS crate.

Wait for your updated tests.

robashton commented 2 years ago

(Forgot I was travelling today, I'll get this in tomorrow morning !)

robashton commented 2 years ago

As promised!

codecov[bot] commented 2 years ago

Codecov Report

Merging #16 (f52965e) into main (4049f16) will increase coverage by 0.06%. The diff coverage is 95.00%.

@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   62.37%   62.44%   +0.06%     
==========================================
  Files          75       75              
  Lines        5507     5517      +10     
  Branches     1045     1044       -1     
==========================================
+ Hits         3435     3445      +10     
  Misses       1268     1268              
  Partials      804      804
Impacted Files Coverage Δ
src/crypto/crypto_test.rs 68.18% <ø> (ø)
src/flight/flight4.rs 56.56% <85.71%> (+0.33%) :arrow_up:
src/conn/conn_test.rs 72.89% <100.00%> (+0.01%) :arrow_up:
src/flight/flight5.rs 65.12% <100.00%> (+0.16%) :arrow_up:
...erify/handshake_message_certificate_verify_test.rs 72.22% <100.00%> (+1.63%) :arrow_up:
...ange/handshake_message_server_key_exchange_test.rs 75.00% <100.00%> (-1.20%) :arrow_down:
src/state.rs 69.56% <0.00%> (+0.44%) :arrow_up:
src/handshaker.rs 40.22% <0.00%> (+0.67%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 611f052...f52965e. Read the comment docs.