Stateless DTLS cookie verify

[nametoolong]

#2318 I've ported the approach in our production code back into Botan. It works fine but is a bit hacky.

Use static method TLS::Server::pre_verify_cookie to verify the client hello. Pass the resultant DTLS_Prestate into the newly-added TLS::Server constructor. Call TLS::Server::received_data again with the verified client hello.

As far as I have tested, no old code is broken. Interoperability is untested.

Possible concerns:

• Can we avoid passing the client hello twice? This might require more architectural change.
• Several methods in Datagram_Handshake_IO are made static. Should it be done this way?
• We could refactor TLS::write_record and remove unconnected_write_record from tls_server.cpp.
• Some shared code can be reused between pre_verify_cookie and other places in TLS::Channel.

[codecov-io]

Codecov Report

Merging #2320 into master will increase coverage by 0.02%. The diff coverage is 87.86%.

@@            Coverage Diff             @@
##           master    #2320      +/-   ##
==========================================
+ Coverage   92.59%   92.61%   +0.02%     
==========================================
  Files         557      557              
  Lines       60736    61135     +399     
  Branches     6420     6450      +30     
==========================================
+ Hits        56239    56621     +382     
- Misses       4497     4514      +17     

Impacted Files	Coverage Δ
src/lib/tls/tls_channel.cpp	88.34% <72.72%> (-1.69%)	:arrow_down:
src/lib/tls/tls_server.cpp	88.53% <86.74%> (-0.42%)	:arrow_down:
src/tests/unit_tls.cpp	94.41% <89.90%> (-0.89%)	:arrow_down:
src/lib/tls/msg_client_hello.cpp	91.75% <90.90%> (-0.43%)	:arrow_down:
src/lib/tls/tls_record.cpp	97.59% <94.44%> (-0.42%)	:arrow_down:
src/lib/tls/tls_handshake_io.cpp	92.67% <100.00%> (+10.13%)	:arrow_up:
...c/lib/x509/certstor_flatfile/certstor_flatfile.cpp	83.33% <100.00%> (ø)
src/lib/x509/x509path.cpp	77.69% <100.00%> (ø)
src/tests/test_certstor.cpp	82.88% <100.00%> (-0.10%)	:arrow_down:
src/tests/test_ocsp.cpp	97.42% <100.00%> (+0.02%)	:arrow_up:
... and 7 more

---

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 538f161...15aa0f8. Read the comment docs.

[randombit]

Nice thank you. I will review this next week.

[nametoolong]

Thanks for reviewing and I am sorry to issue such an hackish PR (though it has been in production for a while :( ). I will try to fix those issues.

[nametoolong]

@randombit What's the idiom in Botan to restrict a DTLS_Prestate to be used exactly once, like returning an unique_ptr<DTLS_Prestate>? If we store the client hello in prestate I think it makes sense to prevent each prestate from being used more than once.

[randombit]

Can you rebase against master to pick up #2338 I think the new write_unecrypted_record fn replaces the code you have in tls_server.cpp for same purpose.

[nametoolong]

Rebased (and kind of messed up commit timestamps). Cleaned up tls_server.cpp.

[nametoolong]

But parsing it is only required to get at the version, type, and cookie (which are relatively easy to find) plus to compute the hello verify MAC input which currently relies on reserializing the cookie.

This is no longer the case with DTLS 1.3 draft (which puts the cookie in an extension). I am not sure whether the parsing should be avoided or not. Maybe we need a way to construct a half-parsed Client_Hello instead of computing MAC input separately.

[codecov-commenter]

Codecov Report

Merging #2320 (15aa0f8) into master (538f161) will increase coverage by 0.02%. The diff coverage is 87.86%.

:exclamation: Current head 15aa0f8 differs from pull request most recent head cbf9736. Consider uploading reports for the commit cbf9736 to get more accurate results

@@            Coverage Diff             @@
##           master    #2320      +/-   ##
==========================================
+ Coverage   92.59%   92.61%   +0.02%     
==========================================
  Files         557      557              
  Lines       60736    61135     +399     
  Branches     6420     6450      +30     
==========================================
+ Hits        56239    56621     +382     
- Misses       4497     4514      +17     

Impacted Files	Coverage Δ
src/lib/tls/tls_channel.cpp	88.34% <72.72%> (-1.69%)	:arrow_down:
src/lib/tls/tls_server.cpp	88.53% <86.74%> (-0.42%)	:arrow_down:
src/tests/unit_tls.cpp	94.41% <89.90%> (-0.89%)	:arrow_down:
src/lib/tls/msg_client_hello.cpp	91.75% <90.90%> (-0.43%)	:arrow_down:
src/lib/tls/tls_record.cpp	97.59% <94.44%> (-0.42%)	:arrow_down:
src/lib/tls/tls_handshake_io.cpp	92.67% <100.00%> (+10.13%)	:arrow_up:
...c/lib/x509/certstor_flatfile/certstor_flatfile.cpp	83.33% <100.00%> (ø)
src/lib/x509/x509path.cpp	77.69% <100.00%> (ø)
src/tests/test_certstor.cpp	82.88% <100.00%> (-0.10%)	:arrow_down:
src/tests/test_ocsp.cpp	97.42% <100.00%> (+0.02%)	:arrow_up:
... and 7 more

---

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 538f161...cbf9736. Read the comment docs.