fix(dtls): replace panicking unwrap/expect in crypto hot paths

[nightness]

Summary

• crypto/mod.rs generate_self_signed_with_alg: replaces 3x .unwrap() with ? -- function already returns Result<Self>
• config.rs ConfigBuilder::build(): replaces .build().unwrap() on the rustls WebPkiServerVerifier builder with .map_err(|e| Error::Other(...))? -- function already returns Result<HandshakeConfig>
• config.rs HandshakeConfig::default(): replaced panicking WebPkiServerVerifier::build().expect(...) with a PlaceholderServerCertVerifier; ConfigBuilder::build() now sets all fields explicitly instead of relying on ..Default::default()
• flight/flight5.rs Flight5::generate(): extracts the certificate ref with ok_or_else() returning a proper Fatal/InternalError DTLS alert instead of panicking with .unwrap()
• crypto/mod.rs CryptoPrivateKey: fields are now pub(crate) (with public accessors kind() / serialized_der()) to enforce the invariant that kind and serialized_der are always consistent, making Clone's .expect() sound
• rtc/peer_connection/certificate: refactored to use CryptoPrivateKey::from_key_pair() instead of constructing the struct directly with raw fields

Why This Matters

Panics in DTLS crypto paths terminate the peer connection (and potentially the process) instead of propagating a negotiation error. The flight5 unwrap in particular can fire during an active handshake if the local certificate is unexpectedly absent.

Test Plan

• cargo check passes (all crates)
• cargo clippy passes
• cargo fmt --check passes
• cargo test -p rtc-dtls passes (49/49)

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 37.50000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.15%. Comparing base (9feb4a3) to head (c8122f9).

Files with missing lines	Patch %	Lines
rtc-dtls/src/flight/flight5.rs	23.07%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
- Coverage   71.17%   71.15%   -0.02%     
==========================================
  Files         442      442              
  Lines       67330    67338       +8     
==========================================
- Hits        47922    47917       -5     
- Misses      19408    19421      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR reduces panic risk in DTLS crypto/handshake code paths by replacing several unwrap()/expect() calls with proper error propagation and DTLS alert reporting.

Changes:

• Propagates rcgen failures in Certificate::generate_self_signed_with_alg instead of panicking.
• Makes ConfigBuilder::build() handle WebPkiServerVerifier::build() errors instead of unwrapping.
• Avoids panicking on missing local certificate during Flight5 certificate-verify generation, returning a fatal DTLS alert + error instead.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
rtc-dtls/src/flight/flight5.rs	Replaces a certificate unwrap() with a Result-based error/alert return.
rtc-dtls/src/crypto/mod.rs	Replaces rcgen unwrap()s with ?, and improves diagnosability of CryptoPrivateKey::clone panics.
rtc-dtls/src/config.rs	Replaces verifier builder unwrap() with error mapping in ConfigBuilder::build(), and adds an expect() in Default.

Comments suppressed due to low confidence (1)

rtc-dtls/src/config.rs:354

• HandshakeConfig { ..Default::default() } will eagerly evaluate HandshakeConfig::default() even though several fields are overridden. Since Default::default() currently constructs a WebPkiServerVerifier with .expect(...), this build() can still panic here (and also duplicates verifier construction work). Consider avoiding struct update syntax and instead initialize all fields explicitly (or add a fallible constructor) so ConfigBuilder::build() doesn’t depend on a potentially-panicking Default implementation.

        Ok(HandshakeConfig {
            local_psk_callback: self.psk.take(),
            local_psk_identity_hint: self.psk_identity_hint.take(),
            local_cipher_suites,
            local_signature_schemes,
            extended_master_secret: self.extended_master_secret,
            local_srtp_protection_profiles: self.srtp_protection_profiles,
            server_name,
            client_auth: self.client_auth,
            local_certificates: self.certificates,
            insecure_skip_verify: self.insecure_skip_verify,
            insecure_verification: self.insecure_verification,
            verify_peer_certificate: self.verify_peer_certificate.take(),
            roots_cas: self.roots_cas,
            server_cert_verifier: rustls::client::WebPkiServerVerifier::builder(Arc::new(
                gen_self_signed_root_cert(),
            ))
            .build()
            .map_err(|e| Error::Other(e.to_string()))?,
            client_cert_verifier: None,
            retransmit_interval,
            initial_epoch: 0,
            maximum_transmission_unit,
            replay_protection_window,
            ..Default::default()
        })

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The // Safety: rationale for the .expect(...)s in CryptoPrivateKey::clone assumes serialized_der is always produced by from_key_pair, but CryptoPrivateKey’s fields are pub, so callers can construct inconsistent kind/serialized_der pairs that will panic on clone. Either enforce the invariant (e.g., make fields private and provide constructors/accessors), or adjust the comment/message to clearly state this is an internal invariant and that panicking indicates misuse/bug.

[rainliu]

please fix fmt warning before merge.

[rainliu]

fix fmt warning

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

HandshakeConfig::default() now installs a PlaceholderServerCertVerifier that always errors. Since HandshakeConfig is pub (public API), this makes HandshakeConfig::default() effectively unusable and changes behavior from the previous real WebPkiServerVerifier default. Consider keeping a real verifier in Default and handling build failures without panicking (e.g., build().unwrap_or_else(|e| ...) / logging + fallback), or make HandshakeConfig construction go through ConfigBuilder only.

[Copilot]

This change introduces a new failure path when certificate is None (returning a fatal InternalError alert). Consider adding a unit/integration test that exercises Flight5 generation with no local certificate to ensure the alert and error are propagated as intended and to prevent regressions back to panics.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

certificate.as_ref().ok_or_else(...) is effectively unreachable here: this block is guarded by state.remote_requested_certificate && !cfg.local_certificates.is_empty(), and certificate is constructed as Some(..) precisely when cfg.local_certificates is non-empty. As a result, the new alert/error path can never trigger, and the extra Result plumbing adds complexity without changing behavior. Consider rewriting this section to avoid the redundant Option handling (e.g., pattern-match certificate once and only enter the CertificateVerify path when it is Some).

[nightness]

Rebased onto upstream/master so this PR contains only its own changes. Previous branch structure caused merge conflicts when PRs were merged in sequence. Each PR is now independently mergeable.