More comments on the limitations of tor-rtcompat's TLS API

Also, more comments on why these limitations are safe within the
context of Tor, but you wouldn't want to use them elsewhere.

crates/tor-rtcompat/src/impls/rustls.rs

@@ -233,6 +233,9 @@ fn convert_scheme(
     use rustls::internal::msgs::enums::SignatureScheme as R;
     use x509_signature::SignatureScheme as X;
 
+    // Yes, we do allow PKCS1 here.  That's fine in practice when PKCS1 is only
+    // used (as in TLS 1.2) for signatures; the attacks against correctly
+    // implemented PKCS1 make sense only when it's used for encryption.
     Ok(match scheme {
         R::RSA_PKCS1_SHA256 => X::RSA_PKCS1_SHA256,
         R::ECDSA_NISTP256_SHA256 => X::ECDSA_NISTP256_SHA256,

crates/tor-rtcompat/src/traits.rs

@@ -172,10 +172,29 @@ pub trait CertifiedConn {
 /// An object that knows how to wrap a TCP connection (where the type of said TCP
 /// connection is `S`) with TLS.
 ///
-/// (Note that because of Tor's peculiarities, this is not a
+/// # Usage notes
+///
+/// Note that because of Tor's peculiarities, this is not a
 /// general-purpose TLS type.  Unlike typical users, Tor does not want
-/// its TLS library to check whether the certificates are signed
-/// within the web PKI hierarchy, or what their hostnames are.
+/// its TLS library to check whether the certificates used in TLS are signed
+/// within the web PKI hierarchy, or what their hostnames are, or even whether
+/// they are valid.  It *does*, however, check that the subject public key in the
+/// certificate is indeed correctly used to authenticate the TLS handshake.
+///
+/// If you are implementing something other than Tor, this is **not** the
+/// functionality you want.
+///
+/// How can this behavior be remotely safe, even in Tor?  It only works for Tor
+/// because the certificate that a Tor relay uses in TLS is not actually being
+/// used to certify that relay's public key.  Instead, the certificate only used
+/// as a container for the relay's public key.  The real certification happens
+/// later, inside the TLS session, when the relay presents a CERTS cell.
+///
+/// Such sneakiness was especially necessary before TLS 1.3, which encrypts more
+/// of the handshake, and before pluggable transports, which make
+/// "innocuous-looking TLS handshakes" less important than they once were.  Once
+/// TLS 1.3 is completely ubiquitous, we might be able to specify a simpler link
+/// handshake than Tor uses now.
 #[async_trait]
 pub trait TlsConnector<S> {
     /// The type of connection returned by this connector
@@ -197,6 +216,10 @@ pub trait TlsConnector<S> {
 /// This is separate from [`TlsConnector`] because eventually we may
 /// eventually want to support multiple `TlsConnector` implementations
 /// that use a single [`Runtime`].
+///
+/// See the [`TlsConnector`] documentation for a discussion of the Tor-specific
+/// limitations of this trait: If you are implementing something other than Tor,
+/// this is **not** the functionality you want.
 pub trait TlsProvider<S> {
     /// The Connector object that this provider can return.
     type Connector: TlsConnector<S, Conn = Self::TlsStream> + Send + Sync + Unpin;