pion / dtls

DTLS 1.2 Server/Client implementation for Go
https://pion.ly/
MIT License
602 stars 160 forks source link

Only use algorithms in CertificateRequest #579

Closed mschexnaydre closed 1 year ago

mschexnaydre commented 1 year ago

For RFC compliance only algorithms in the certificate request shall be used.

Description We should only be using one of the signature algorithms specified in the CertificateRequest message when generating the CertificateVerify message. Prior to this fix SHA-256 was always being used.

This change stores the HASH algorithm from the CertificateRequest message in the State object so that we can reference these later when generating the CertificateVerify message.

Removed hard-coded usage of SHA-256 in generateCertificateVerify, now uses the Digest method of the passed in algorithm.

Reference issue Fixes https://github.com/pion/dtls/issues/418

mschexnaydre commented 1 year ago

New PR - still needs unit test

codecov[bot] commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Files Coverage Δ
crypto.go 60.00% <100.00%> (+0.88%) :arrow_up:
flight3handler.go 77.55% <100.00%> (+0.09%) :arrow_up:
flight5handler.go 76.25% <100.00%> (+0.07%) :arrow_up:
state.go 91.53% <ø> (ø)

... and 3 files with indirect coverage changes

:loudspeaker: Thoughts on this report? Let us know!.

mschexnaydre commented 1 year ago

Added test in conn_test that specifies SignatureScheme which fails with code prior to this fix.

--- FAIL: TestClientCertificate (0.22s) --- FAIL: TestClientCertificate/parallel (0.01s) --- FAIL: TestClientCertificate/parallel/RequestClientCert_cert_sigscheme (0.00s) conn_test.go:996: Server failed(handshake error: dtls fatal: connection can not be created, no SignatureScheme satisfy this Config) conn_test.go:1000: Client failed(handshake error: alert: Alert Fatal: InsufficientSecurity) panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference

mschexnaydre commented 1 year ago

Cleaning this up.