Merge pull request #335 from NordSecurity/msz/FILE-562-downgrade-attack

Remove protocols V1, V2, V4 and V5

changelog.md

@@ -1,3 +1,11 @@
+### UNRELEASED
+### **Fresh air**
+---
+* Remove support for deprecated, unsecure protocols V1, V2, V4 and V5
+
+---
+<br>
+
 ### 7.0.0
 ### **The United States of FFI***
 ---

drop-transfer/src/check.rs

@@ -78,7 +78,7 @@ async fn ask_server(state: &State, xfer: &IncomingTransfer, logger: &Logger) ->
 
     let client = hyper::Client::builder().build::<_, hyper::Body>(connector);
 
-    let versions_to_try = [protocol::Version::V6, protocol::Version::V5];
+    let versions_to_try = [protocol::Version::V6];
 
     for version in versions_to_try {
         match make_request(
@@ -131,22 +131,13 @@ async fn make_request(
 
     debug!(logger, "Making HTTP request: {url}");
 
-    let mut req = hyper::Request::get(url.clone());
+    let req = hyper::Request::get(url.clone());
 
-    use protocol::Version as Ver;
-    let server_auth_scheme = match version {
-        Ver::V1 | Ver::V2 | Ver::V4 | Ver::V5 => None,
-        _ => {
-            let nonce = drop_auth::Nonce::generate_as_client();
-
-            let (key, value) = auth::create_www_authentication_header(&nonce);
-            req = req.header(key, value);
-
-            Some(nonce)
-        }
-    };
+    let nonce = drop_auth::Nonce::generate_as_client();
+    let (key, value) = auth::create_www_authentication_header(&nonce);
 
     let req = req
+        .header(key, value)
         .body(hyper::Body::empty())
         .expect("Creating request should not fail");
 
@@ -156,12 +147,9 @@ async fn make_request(
         .context("Failed to perform HTTP request")?;
 
     let authorize = || {
-        if let Some(nonce) = &server_auth_scheme {
-            // Validate the server response
-            auth.authorize_server(&response, ip, nonce)
-                .context("Failed to authorize server. Closing connection")?;
-        }
-        anyhow::Ok(())
+        // Validate the server response
+        auth.authorize_server(&response, ip, &nonce)
+            .context("Failed to authorize server. Closing connection")
     };
 
     match response.status() {

drop-transfer/src/protocol/mod.rs

@@ -1,33 +1,22 @@
-pub mod v1;
-pub mod v2 {
-    pub use super::v1::*;
-}
-pub mod v4;
 pub mod v6;
 
 #[derive(Copy, Clone, strum::Display, strum::EnumString)]
 pub enum Version {
-    #[strum(serialize = "v1")]
-    V1,
-    #[strum(serialize = "v2")]
-    V2,
+    // Versions V1 and V2 were yanked because these lacked the client
+    // authentication, which is a security flaw.
+
     // There is no V3 for historical reasons. We yanked Version 3, because it was released with a
     // security flaw. It should never be added back.
-    #[strum(serialize = "v4")]
-    V4,
-    #[strum(serialize = "v5")]
-    V5,
+
+    // Verions V4 and V5 are removed because these did not support server side
+    // authentication. Yanked on the security grounds.
     #[strum(serialize = "v6")]
     V6,
 }
 
 impl From<Version> for i32 {
     fn from(version: Version) -> Self {
         match version {
-            Version::V1 => 1,
-            Version::V2 => 2,
-            Version::V4 => 4,
-            Version::V5 => 5,
             Version::V6 => 6,
         }
     }