From e85c320e7d0685aed3bcbf1f8ed5363a64cf0883 Mon Sep 17 00:00:00 2001
From: Benjamin Saunders <ben.e.saunders@gmail.com>
Date: Wed, 23 Jan 2019 01:17:22 -0800
Subject: [PATCH] Improve configuration error checking

---
 quinn-proto/src/endpoint.rs | 51 ++++++++++++++++++++++++++-----------
 quinn-proto/src/lib.rs      |  2 +-
 quinn-proto/src/varint.rs   |  3 +++
 quinn/src/builders.rs       | 12 ++++-----
 4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs
index 662636757..9b73f60ce 100644
--- a/quinn-proto/src/endpoint.rs
+++ b/quinn-proto/src/endpoint.rs
@@ -25,7 +25,7 @@ use crate::packet::{ConnectionId, EcnCodepoint, Header, Packet, PacketDecodeErro
 use crate::stream::{ReadError, WriteError};
 use crate::transport_parameters::TransportParameters;
 use crate::{
-    Directionality, Side, StreamId, Transmit, TransportError, MAX_CID_SIZE, MIN_CID_SIZE,
+    varint, Directionality, Side, StreamId, Transmit, TransportError, MAX_CID_SIZE, MIN_CID_SIZE,
     MIN_INITIAL_SIZE, RESET_TOKEN_SIZE, VERSION,
 };
 
@@ -59,13 +59,10 @@ impl Endpoint {
         log: Logger,
         config: Config,
         server_config: Option<ServerConfig>,
-    ) -> Result<Self, EndpointError> {
+    ) -> Result<Self, ConfigError> {
+        config.validate()?;
         let rng = OsRng::new().unwrap();
         let config = Arc::new(config);
-        assert!(
-            (config.local_cid_len == 0 || config.local_cid_len >= MIN_CID_SIZE)
-                && config.local_cid_len <= MAX_CID_SIZE
-        );
         Ok(Self {
             log,
             rng,
@@ -914,6 +911,31 @@ impl Default for Config {
     }
 }
 
+impl Config {
+    fn validate(&self) -> Result<(), ConfigError> {
+        if (self.local_cid_len != 0 && self.local_cid_len < MIN_CID_SIZE)
+            || self.local_cid_len > MAX_CID_SIZE
+        {
+            return Err(ConfigError::IllegalValue(
+                "local_cid_len must be 0 or in [4, 18]",
+            ));
+        }
+        if let Some((name, _)) = [
+            ("stream_window_bidi", self.stream_window_bidi),
+            ("stream_window_uni", self.stream_window_uni),
+            ("receive_window", self.receive_window),
+            ("stream_receive_window", self.stream_receive_window),
+            ("idle_timeout", self.idle_timeout),
+        ]
+        .iter()
+        .find(|&&(_, x)| x > varint::MAX_VALUE)
+        {
+            return Err(ConfigError::VarIntBounds(name));
+        }
+        Ok(())
+    }
+}
+
 /// Parameters governing incoming connections.
 pub struct ServerConfig {
     /// TLS configuration used for incoming connections.
@@ -958,15 +980,14 @@ impl Default for ServerConfig {
 
 /// Errors in the configuration of an endpoint
 #[derive(Debug, Error)]
-pub enum EndpointError {
-    #[error(display = "failed to configure TLS: {}", _0)]
-    Tls(crypto::TLSError),
-}
-
-impl From<crypto::TLSError> for EndpointError {
-    fn from(x: crypto::TLSError) -> Self {
-        EndpointError::Tls(x)
-    }
+pub enum ConfigError {
+    /// The supplied configuration contained an invalid value
+    #[error(display = "illegal configuration value: {}", _0)]
+    IllegalValue(&'static str),
+    /// A configuration field that will be encoded as a variable-length integer exceeds the 0..2^62
+    /// range
+    #[error(display = "{} must be at most 2^62-1", _0)]
+    VarIntBounds(&'static str),
 }
 
 /// Events of interest to the application
diff --git a/quinn-proto/src/lib.rs b/quinn-proto/src/lib.rs
index 60a191ff8..41431ea70 100644
--- a/quinn-proto/src/lib.rs
+++ b/quinn-proto/src/lib.rs
@@ -34,7 +34,7 @@ pub use crate::frame::{ApplicationClose, ConnectionClose};
 
 mod endpoint;
 pub use crate::endpoint::{
-    Config, ConnectionHandle, Endpoint, EndpointError, Event, ServerConfig, Timer,
+    Config, ConfigError, ConnectionHandle, Endpoint, Event, ServerConfig, Timer,
 };
 
 mod packet;
diff --git a/quinn-proto/src/varint.rs b/quinn-proto/src/varint.rs
index 21924c96f..5b00d3470 100644
--- a/quinn-proto/src/varint.rs
+++ b/quinn-proto/src/varint.rs
@@ -2,6 +2,9 @@ use byteorder::{BigEndian, ByteOrder};
 use bytes::{Buf, BufMut};
 use err_derive::Error;
 
+/// The largest value representable as a varint
+pub const MAX_VALUE: u64 = (1 << 62) - 1;
+
 pub fn size(x: u64) -> Option<usize> {
     if x < 2u64.pow(6) {
         Some(1)
diff --git a/quinn/src/builders.rs b/quinn/src/builders.rs
index d9272fc57..6449bab24 100644
--- a/quinn/src/builders.rs
+++ b/quinn/src/builders.rs
@@ -133,14 +133,14 @@ pub enum EndpointError {
     /// Errors relating to web PKI infrastructure
     #[error(display = "webpki failed: {:?}", _0)]
     WebPki(webpki::Error),
+    /// An error in the Quinn transport configuration
+    #[error(display = "configuration error: {:?}", _0)]
+    Config(quinn::ConfigError),
 }
 
-impl From<quinn::EndpointError> for EndpointError {
-    fn from(x: quinn::EndpointError) -> Self {
-        use crate::quinn::EndpointError::*;
-        match x {
-            Tls(x) => EndpointError::Tls(x),
-        }
+impl From<quinn::ConfigError> for EndpointError {
+    fn from(x: quinn::ConfigError) -> Self {
+        EndpointError::Config(x)
     }
 }