[peer_connection] RTCCertificate: rework from_pem and serialize_pem (#333)

melekes · web-flow · commit c0579c375151 · 2022-11-04T17:36:07.000+04:00
which makes it possible to serialise and persist a certificate to disk. Both methods require `pem` feature to be enabled (I think having a feature is good since I don't expect many people to use persistent certificate). Previous implementation of `from_pem` was incorrect. It used a given certificate as a CA (hence the 2nd argument) instead of just using the certificate directly (see the related TODO in tests where `cert1 == cert3` should be matching). API reflects one in Pion - https://github.com/pion/webrtc/blob/master/certificate.go#L179 (see `CertificateFromPEM` and `PEM` functions). Also - modify `RTCCertificate::get_fingerprints` to return `Vec<Fingerprint>`. No need for `Result`. - refactor `dtls::crypto::Certificate` - fixes TODO where two certificates should be equal when `from_pem` is being used.
diff --git a/dtls/CHANGELOG.md b/dtls/CHANGELOG.md
@@ -3,6 +3,9 @@
 ## Unreleased
 
 * Increased minimum support rust version to `1.60.0`.
+* Add `RTCCertificate::from_pem` and `RTCCertificate::serialize_pem` (only work with `pem` feature enabled) [#333]
+
+[#333]: https://github.com/webrtc-rs/webrtc/pull/333
 
 ## v0.6.0
 
diff --git a/dtls/Cargo.toml b/dtls/Cargo.toml
@@ -49,6 +49,7 @@ serde = { version = "1.0.110", features = ["derive"] }
 subtle = "2.4"
 log = "0.4.16"
 thiserror = "1.0"
+pem = { version = "1", optional = true }
 
 [dev-dependencies]
 tokio-test = "0.4.0" # must match the min version of the `tokio` crate above
@@ -57,6 +58,9 @@ chrono = "0.4.19"
 clap = "3.2.6"
 hub = {path = "examples/hub"}
 
+[features]
+pem = ["dep:pem"]
+
 [[example]]
 name = "dial_psk"
 path = "examples/dial/psk/dial_psk.rs"
diff --git a/dtls/src/crypto/mod.rs b/dtls/src/crypto/mod.rs
@@ -15,103 +15,102 @@ use der_parser::{oid, oid::Oid};
 use rcgen::KeyPair;
 use ring::rand::SystemRandom;
 use ring::signature::{EcdsaKeyPair, Ed25519KeyPair, RsaKeyPair};
+use std::convert::TryFrom;
 use std::sync::Arc;
 
-#[derive(Clone, PartialEq)]
+/// A X.509 certificate(s) used to authenticate a DTLS connection.
+#[derive(Clone, PartialEq, Debug)]
 pub struct Certificate {
+    /// DER-encoded certificates.
     pub certificate: Vec<rustls::Certificate>,
+    /// Private key.
     pub private_key: CryptoPrivateKey,
 }
 
 impl Certificate {
+    /// Generate a self-signed certificate.
+    ///
+    /// See [`rcgen::generate_simple_self_signed`].
     pub fn generate_self_signed(subject_alt_names: impl Into<Vec<String>>) -> Result<Self> {
         let cert = rcgen::generate_simple_self_signed(subject_alt_names)?;
-        let certificate = cert.serialize_der()?;
         let key_pair = cert.get_key_pair();
-        let serialized_der = key_pair.serialize_der();
-        let private_key = if key_pair.is_compatible(&rcgen::PKCS_ED25519) {
-            CryptoPrivateKey {
-                kind: CryptoPrivateKeyKind::Ed25519(
-                    Ed25519KeyPair::from_pkcs8(&serialized_der)
-                        .map_err(|e| Error::Other(e.to_string()))?,
-                ),
-                serialized_der,
-            }
-        } else if key_pair.is_compatible(&rcgen::PKCS_ECDSA_P256_SHA256) {
-            CryptoPrivateKey {
-                kind: CryptoPrivateKeyKind::Ecdsa256(
-                    EcdsaKeyPair::from_pkcs8(
-                        &ring::signature::ECDSA_P256_SHA256_ASN1_SIGNING,
-                        &serialized_der,
-                    )
-                    .map_err(|e| Error::Other(e.to_string()))?,
-                ),
-                serialized_der,
-            }
-        } else if key_pair.is_compatible(&rcgen::PKCS_RSA_SHA256) {
-            CryptoPrivateKey {
-                kind: CryptoPrivateKeyKind::Rsa256(
-                    RsaKeyPair::from_pkcs8(&serialized_der)
-                        .map_err(|e| Error::Other(e.to_string()))?,
-                ),
-                serialized_der,
-            }
-        } else {
-            return Err(Error::Other("Unsupported key_pair".to_owned()));
-        };
 
         Ok(Certificate {
-            certificate: vec![rustls::Certificate(certificate)],
-            private_key,
+            certificate: vec![rustls::Certificate(cert.serialize_der()?)],
+            private_key: CryptoPrivateKey::try_from(key_pair)?,
         })
     }
 
+    /// Generate a self-signed certificate with the given algorithm.
+    ///
+    /// See [`rcgen::Certificate::from_params`].
     pub fn generate_self_signed_with_alg(
         subject_alt_names: impl Into<Vec<String>>,
         alg: &'static rcgen::SignatureAlgorithm,
     ) -> Result<Self> {
         let mut params = rcgen::CertificateParams::new(subject_alt_names);
         params.alg = alg;
         let cert = rcgen::Certificate::from_params(params)?;
-        let certificate = cert.serialize_der()?;
         let key_pair = cert.get_key_pair();
-        let serialized_der = key_pair.serialize_der();
-        let private_key = if key_pair.is_compatible(&rcgen::PKCS_ED25519) {
-            CryptoPrivateKey {
-                kind: CryptoPrivateKeyKind::Ed25519(
-                    Ed25519KeyPair::from_pkcs8(&serialized_der)
-                        .map_err(|e| Error::Other(e.to_string()))?,
-                ),
-                serialized_der,
-            }
-        } else if key_pair.is_compatible(&rcgen::PKCS_ECDSA_P256_SHA256) {
-            CryptoPrivateKey {
-                kind: CryptoPrivateKeyKind::Ecdsa256(
-                    EcdsaKeyPair::from_pkcs8(
-                        &ring::signature::ECDSA_P256_SHA256_ASN1_SIGNING,
-                        &serialized_der,
-                    )
-                    .map_err(|e| Error::Other(e.to_string()))?,
-                ),
-                serialized_der,
-            }
-        } else if key_pair.is_compatible(&rcgen::PKCS_RSA_SHA256) {
-            CryptoPrivateKey {
-                kind: CryptoPrivateKeyKind::Rsa256(
-                    RsaKeyPair::from_pkcs8(&serialized_der)
-                        .map_err(|e| Error::Other(e.to_string()))?,
-                ),
-                serialized_der,
+
+        Ok(Certificate {
+            certificate: vec![rustls::Certificate(cert.serialize_der()?)],
+            private_key: CryptoPrivateKey::try_from(key_pair)?,
+        })
+    }
+
+    /// Parses a certificate from the ASCII PEM format.
+    #[cfg(feature = "pem")]
+    pub fn from_pem(pem_str: &str) -> Result<Self> {
+        let mut pems = pem::parse_many(pem_str).map_err(|e| Error::InvalidPEM(e.to_string()))?;
+        if pems.len() < 2 {
+            return Err(Error::InvalidPEM(format!(
+                "expected at least two PEM blocks, got {}",
+                pems.len()
+            )));
+        }
+        if pems[0].tag != "PRIVATE_KEY" {
+            return Err(Error::InvalidPEM(format!(
+                "invalid tag (expected: 'PRIVATE_KEY', got: '{}')",
+                pems[0].tag
+            )));
+        }
+
+        let keypair = rcgen::KeyPair::from_der(&pems[0].contents)
+            .map_err(|e| Error::InvalidPEM(format!("can't decode keypair: {}", e)))?;
+
+        let mut rustls_certs = Vec::new();
+        for p in pems.drain(1..) {
+            if p.tag != "CERTIFICATE" {
+                return Err(Error::InvalidPEM(format!(
+                    "invalid tag (expected: 'CERTIFICATE', got: '{}')",
+                    p.tag
+                )));
             }
-        } else {
-            return Err(Error::Other("Unsupported key_pair".to_owned()));
-        };
+            rustls_certs.push(rustls::Certificate(p.contents));
+        }
 
         Ok(Certificate {
-            certificate: vec![rustls::Certificate(certificate)],
-            private_key,
+            certificate: rustls_certs,
+            private_key: CryptoPrivateKey::try_from(&keypair)?,
         })
     }
+
+    /// Serializes the certificate (including the private key) in PKCS#8 format in PEM.
+    #[cfg(feature = "pem")]
+    pub fn serialize_pem(&self) -> String {
+        let mut data = vec![pem::Pem {
+            tag: "PRIVATE_KEY".to_string(),
+            contents: self.private_key.serialized_der.clone(),
+        }];
+        for rustls_cert in &self.certificate {
+            data.push(pem::Pem {
+                tag: "CERTIFICATE".to_string(),
+                contents: rustls_cert.0.clone(),
+            });
+        }
+        pem::encode_many(&data)
+    }
 }
 
 pub(crate) fn value_key_message(
@@ -134,14 +133,20 @@ pub(crate) fn value_key_message(
     plaintext
 }
 
+/// Either ED25519, ECDSA or RSA keypair.
+#[derive(Debug)]
 pub enum CryptoPrivateKeyKind {
     Ed25519(Ed25519KeyPair),
     Ecdsa256(EcdsaKeyPair),
     Rsa256(RsaKeyPair),
 }
 
+/// Private key.
+#[derive(Debug)]
 pub struct CryptoPrivateKey {
+    /// Keypair.
     pub kind: CryptoPrivateKeyKind,
+    /// DER-encoded keypair.
     pub serialized_der: Vec<u8>,
 }
 
@@ -196,6 +201,44 @@ impl Clone for CryptoPrivateKey {
     }
 }
 
+impl TryFrom<&KeyPair> for CryptoPrivateKey {
+    type Error = Error;
+
+    fn try_from(key_pair: &KeyPair) -> Result<Self> {
+        let serialized_der = key_pair.serialize_der();
+        if key_pair.is_compatible(&rcgen::PKCS_ED25519) {
+            Ok(CryptoPrivateKey {
+                kind: CryptoPrivateKeyKind::Ed25519(
+                    Ed25519KeyPair::from_pkcs8(&serialized_der)
+                        .map_err(|e| Error::Other(e.to_string()))?,
+                ),
+                serialized_der,
+            })
+        } else if key_pair.is_compatible(&rcgen::PKCS_ECDSA_P256_SHA256) {
+            Ok(CryptoPrivateKey {
+                kind: CryptoPrivateKeyKind::Ecdsa256(
+                    EcdsaKeyPair::from_pkcs8(
+                        &ring::signature::ECDSA_P256_SHA256_ASN1_SIGNING,
+                        &serialized_der,
+                    )
+                    .map_err(|e| Error::Other(e.to_string()))?,
+                ),
+                serialized_der,
+            })
+        } else if key_pair.is_compatible(&rcgen::PKCS_RSA_SHA256) {
+            Ok(CryptoPrivateKey {
+                kind: CryptoPrivateKeyKind::Rsa256(
+                    RsaKeyPair::from_pkcs8(&serialized_der)
+                        .map_err(|e| Error::Other(e.to_string()))?,
+                ),
+                serialized_der,
+            })
+        } else {
+            Err(Error::Other("Unsupported key_pair".to_owned()))
+        }
+    }
+}
+
 impl CryptoPrivateKey {
     pub fn from_key_pair(key_pair: &KeyPair) -> Result<Self> {
         let serialized_der = key_pair.serialize_der();
@@ -458,3 +501,21 @@ pub(crate) fn generate_aead_additional_data(h: &RecordLayerHeader, payload_len:
 
     additional_data
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[cfg(feature = "pem")]
+    #[test]
+    fn test_certificate_serialize_pem_and_from_pem() -> Result<()> {
+        let cert = Certificate::generate_self_signed(vec!["webrtc.rs".to_owned()])?;
+
+        let pem = cert.serialize_pem();
+        let loaded_cert = Certificate::from_pem(&pem)?;
+
+        assert_eq!(loaded_cert, cert);
+
+        Ok(())
+    }
+}
diff --git a/dtls/src/error.rs b/dtls/src/error.rs
@@ -166,6 +166,10 @@ pub enum Error {
     #[error("keying material: {0}")]
     KeyingMaterial(#[from] KeyingMaterialExporterError),
 
+    /// Error parsing a given PEM string.
+    #[error("invalid PEM: {0}")]
+    InvalidPEM(String),
+
     #[allow(non_camel_case_types)]
     #[error("{0}")]
     Other(String),
diff --git a/webrtc/CHANGELOG.md b/webrtc/CHANGELOG.md
@@ -20,6 +20,14 @@ directions that should not send. [#316](https://github.com/webrtc-rs/webrtc/pull
 change for MediaEngine::register_header_extension
 * Removes support for Plan-B. All major implementations of WebRTC now support unified and continuing support for plan-b is an undue maintenance burden when unified can be used. See [“Unified Plan” Transition Guide (JavaScript)](https://docs.google.com/document/d/1-ZfikoUtoJa9k-GZG1daN0BU3IjIanQ_JSscHxQesvU/) for an overview of the changes required to migrate. [#320](https://github.com/webrtc-rs/webrtc/pull/320) by [@algesten](https://github.com/algesten).
 
+#### Breaking changes
+
+* Remove 2nd argument from `RTCCertificate::from_pem` and guard it with `pem` feature [#333]
+* Rename `RTCCertificate::pem` to `serialize_pem`  and guard it with `pem` feature [#333]
+* Remove `RTCCertificate::expires` [#333]
+* `RTCCertificate::get_fingerprints` no longer returns `Result` [#333]
+
+[#333]: https://github.com/webrtc-rs/webrtc/pull/333
 
 ## 0.5.1
 
@@ -33,12 +41,12 @@ change for MediaEngine::register_header_extension
 
 * The serialized format for `RTCIceCandidateInit` has changed to match what the specification i.e. keys are camelCase. [#153 Make RTCIceCandidateInit conform to WebRTC spec](https://github.com/webrtc-rs/webrtc/pull/153) contributed by [jmatss](https://github.com/jmatss).
 * Improved robustness when proposing RTP extension IDs and handling of collisions in these. This change is only breaking if you have assumed anything about the nature of these extension IDs. [#154 Fix RTP extension id collision](https://github.com/webrtc-rs/webrtc/pull/154) contributed by [k0nserv](https://github.com/k0nserv)
-* Transceivers will now not stop when either or both directions are disabled. That is, applying and SDP with `a=inactive` will not stop the transceiver, instead attached senders and receivers will pause. A transceiver can be resurrected by setting direction back to e.g. `a=sendrecv`. The desired direction can be controlled with the newly introduced public method `set_direction` on `RTCRtpTransceiver`.  
+* Transceivers will now not stop when either or both directions are disabled. That is, applying and SDP with `a=inactive` will not stop the transceiver, instead attached senders and receivers will pause. A transceiver can be resurrected by setting direction back to e.g. `a=sendrecv`. The desired direction can be controlled with the newly introduced public method `set_direction` on `RTCRtpTransceiver`.
   * [#201 Handle inactive transceivers more correctly](https://github.com/webrtc-rs/webrtc/pull/201) contributed by [k0nserv](https://github.com/k0nserv)
-  * [#210 Rework transceiver direction support further](https://github.com/webrtc-rs/webrtc/pull/210) contributed by [k0nserv](https://github.com/k0nserv) 
+  * [#210 Rework transceiver direction support further](https://github.com/webrtc-rs/webrtc/pull/210) contributed by [k0nserv](https://github.com/k0nserv)
   * [#214 set_direction add missing Send + Sync bound](https://github.com/webrtc-rs/webrtc/pull/214) contributed by [algesten](https://github.com/algesten)
   * [#213 set_direction add missing Sync bound](https://github.com/webrtc-rs/webrtc/pull/213) contributed by [algesten](https://github.com/algesten)
-  * [#212 Public RTCRtpTransceiver::set_direction](https://github.com/webrtc-rs/webrtc/pull/212) contributed by [algesten](https://github.com/algesten) 
+  * [#212 Public RTCRtpTransceiver::set_direction](https://github.com/webrtc-rs/webrtc/pull/212) contributed by [algesten](https://github.com/algesten)
   * [#268 Fix current direction update when applying answer](https://github.com/webrtc-rs/webrtc/pull/268) contributed by [k0nserv](https://github.com/k0nserv)
   * [#236 Pause RTP writing if direction indicates it](https://github.com/webrtc-rs/webrtc/pull/236) contributed by [algesten](https://github.com/algesten)
 * Generated the `a=msid` line for `m=` line sections according to the specification. This might be break remote peers that relied on the previous, incorrect, behaviour. This also fixes a bug where an endless negotiation loop could happen. [#217 Correct msid handling for RtpSender](https://github.com/webrtc-rs/webrtc/pull/217) contributed by [k0nserv](https://github.com/k0nserv)
@@ -108,7 +116,7 @@ The various sub-crates have been updated as follows:
 
 Their respective change logs are found in the old, now archived, repositories and within their respective `CHANGELOG.md` files in the monorepo.
 
-### Contributors 
+### Contributors
 
 A big thanks to all the contributors that have made this release happen:
 
diff --git a/webrtc/Cargo.toml b/webrtc/Cargo.toml
@@ -46,6 +46,7 @@ ring = "0.16.20"
 sha2 = "0.10.2"
 lazy_static = "1.4"
 hex = "0.4.3"
+pem = { version = "1", optional = true }
 
 # [minimal-versions]
 # fixes "the trait bound `time::Month: From<u8>` is not satisfied"
@@ -56,3 +57,6 @@ time = "~0.3.1"
 [dev-dependencies]
 tokio-test = "0.4.0" # must match the min version of the `tokio` crate above
 env_logger = "0.9.0"
+
+[features]
+pem = ["dep:pem", "dtls/pem"]
diff --git a/webrtc/src/api/mod.rs b/webrtc/src/api/mod.rs
@@ -86,7 +86,7 @@ impl API {
         if !certificates.is_empty() {
             let now = SystemTime::now();
             for cert in &certificates {
-                cert.expires()
+                cert.expires
                     .duration_since(now)
                     .map_err(|_| Error::ErrCertificateExpired)?;
             }
diff --git a/webrtc/src/dtls_transport/mod.rs b/webrtc/src/dtls_transport/mod.rs
@@ -156,7 +156,7 @@ impl RTCDtlsTransport {
         let mut fingerprints = vec![];
 
         for c in &self.certificates {
-            fingerprints.extend(c.get_fingerprints()?);
+            fingerprints.extend(c.get_fingerprints());
         }
 
         Ok(DTLSParameters {
@@ -336,7 +336,7 @@ impl RTCDtlsTransport {
         }
 
         let certificate = if let Some(cert) = self.certificates.first() {
-            cert.certificate.clone()
+            cert.dtls_certificate.clone()
         } else {
             return Err(Error::ErrNonCertificate);
         };
diff --git a/webrtc/src/error.rs b/webrtc/src/error.rs
@@ -408,6 +408,10 @@ pub enum Error {
     #[error("parse url: {0}")]
     ParseUrl(#[from] url::ParseError),
 
+    /// Error parsing a given PEM string.
+    #[error("invalid PEM: {0}")]
+    InvalidPEM(String),
+
     #[allow(non_camel_case_types)]
     #[error("{0}")]
     new(String),
diff --git a/webrtc/src/peer_connection/certificate.rs b/webrtc/src/peer_connection/certificate.rs
diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs
diff --git a/webrtc/src/peer_connection/peer_connection_internal.rs b/webrtc/src/peer_connection/peer_connection_internal.rs
diff --git a/webrtc/src/peer_connection/sdp/sdp_test.rs b/webrtc/src/peer_connection/sdp/sdp_test.rs

Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ impl API {`
`86`	`86`	`if !certificates.is_empty() {`
`87`	`87`	`let now = SystemTime::now();`
`88`	`88`	`for cert in &certificates {`
`89`		`- cert.expires()`
	`89`	`+ cert.expires`
`90`	`90`	`.duration_since(now)`
`91`	`91`	`.map_err(\|_\| Error::ErrCertificateExpired)?;`
`92`	`92`	`}`
Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ impl RTCDtlsTransport {`
`156`	`156`	`let mut fingerprints = vec![];`
`157`	`157`
`158`	`158`	`for c in &self.certificates {`
`159`		`- fingerprints.extend(c.get_fingerprints()?);`
	`159`	`+ fingerprints.extend(c.get_fingerprints());`
`160`	`160`	`}`
`161`	`161`
`162`	`162`	`Ok(DTLSParameters {`
`@@ -336,7 +336,7 @@ impl RTCDtlsTransport {`
`336`	`336`	`}`
`337`	`337`
`338`	`338`	`let certificate = if let Some(cert) = self.certificates.first() {`
`339`		`- cert.certificate.clone()`
	`339`	`+ cert.dtls_certificate.clone()`
`340`	`340`	`} else {`
`341`	`341`	`return Err(Error::ErrNonCertificate);`
`342`	`342`	`};`