diff --git a/CHANGELOG.md b/CHANGELOG.md index 767ac3eb..ab36587c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- \[lib\]\[deprecated\] `magic_wormhole::transfer::send_*` and `request_file` methods to take an `OfferSend` and `OfferReceive` instead of using separate methods for files and folders. Use `transfer::send()` and `transfer::receive()` for the new methods. -- \[lib\]\[breaking\] struct `transfer::ReceiveRequest` became an enum to prepare for transfer v2 +- \[all\]\[\breaking\] Code words with a secret password section shorter than 4 bytes are no longer accepted. This only breaks completely invalid uses of the code. +- \[all\] Code words with a weak password section or a non-integer nameplate will throw an error in the long. This error can be upgraded to a hard error by enabling the "entropy" feature. This feature will become the default in the next major release. ## [0.7.1] - 2024-07-25 diff --git a/cli/src/main.rs b/cli/src/main.rs index e31b09b8..dc9a40aa 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -18,7 +18,7 @@ use std::{io::Write, path::PathBuf}; use magic_wormhole::{ forwarding, transfer, transit::{self, TransitInfo}, - MailboxConnection, Wormhole, + MailboxConnection, ParseCodeError, ParsePasswordError, Wormhole, }; fn install_ctrlc_handler( @@ -608,22 +608,27 @@ async fn parse_and_connect( // Split the nameplate parsing from the code parsing to ensure we allow non-integer nameplates // until the next breaking release let res: Option> = code.as_ref().map(|c| c.parse()); - let code: Option = { - match res { - Some(Ok(code)) => Some(code), - // Check if an interactive terminal is connected - Some(Err(err)) if std::io::stdin().is_terminal() => { - // Only fail for the case where the password is < 4 characters. - // Anything else will just print an error for now. - return Err(err.into()); - }, - Some(Err(_)) => { - // The library crate already emits an error log for this. - code.map(|c| c.into()) - }, - None => None, - } - }; + let code: Option = + { + match res { + Some(Ok(code)) => Some(code), + // Check if an interactive terminal is connected + Some(Err(err @ ParseCodeError::Password(ParsePasswordError::TooShort { .. }))) => { + // Only fail for the case where the password is < 4 characters. + // Anything else will just print an error for now. + return Err(err.into()); + }, + // If we have an interactive terminal connected, also fail for low entropy + Some(Err( + err @ ParseCodeError::Password(ParsePasswordError::LittleEntropy { .. }), + )) if std::io::stdin().is_terminal() => return Err(err.into()), + Some(Err(_)) => { + // The library crate already emits an error log for this. + code.map(|c| c.into()) + }, + None => None, + } + }; /* We need to track that information for when we generate a QR code */ let mut uri_rendezvous = None; diff --git a/src/core.rs b/src/core.rs index 525a8d16..2914867f 100644 --- a/src/core.rs +++ b/src/core.rs @@ -55,7 +55,7 @@ pub enum WormholeError { UnclaimedNameplate(Nameplate), /// The provided code is invalid #[error("The provided code is invalid: {_0}")] - CodeInvalid(#[from] CodeFromStringError), + CodeInvalid(#[from] ParseCodeError), } impl WormholeError { @@ -166,7 +166,7 @@ impl MailboxConnection { let (mut server, welcome) = RendezvousServer::connect(&config.id, &config.rendezvous_url).await?; let (nameplate, mailbox) = server.allocate_claim_open().await?; - let password = password.parse().map_err(CodeFromStringError::from)?; + let password = password.parse().map_err(ParseCodeError::from)?; let code = Code::from_components(nameplate, password); Ok(MailboxConnection { @@ -751,8 +751,9 @@ impl AsRef for Phase { )] pub struct Mailbox(pub String); +/// An error occurred when parsing a nameplate: Nameplate is not a number. #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Copy, derive_more::Display, Error)] -#[display("Nameplate is not a number. Nameplates must be a number.")] +#[display("Nameplate is not a number. Nameplates must be a number >= 1.")] #[non_exhaustive] pub struct ParseNameplateError {} @@ -778,9 +779,14 @@ impl Nameplate { note = "Nameplates will be required to be numbers soon. Use the [std::str::FromStr] implementation" )] pub fn new(n: impl Into) -> Self { + let nameplate = n.into(); + if let Err(err) = Nameplate::from_str(&nameplate) { + log::error!("{err}"); + } + #[allow(unsafe_code)] unsafe { - Self::new_unchecked(n) + Self::new_unchecked(nameplate) } } @@ -794,8 +800,11 @@ impl FromStr for Nameplate { type Err = ParseNameplateError; fn from_str(s: &str) -> Result { - let _num: usize = s.parse().map_err(|_| ParseNameplateError {})?; - Ok(Self(s.to_string())) + if !s.chars().all(|c| c.is_ascii_digit()) || u128::from_str(s) == Ok(0) { + Err(ParseNameplateError {}) + } else { + Ok(Self(s.to_string())) + } } } @@ -813,7 +822,13 @@ impl From for String { #[allow(deprecated)] impl From for Nameplate { fn from(value: String) -> Self { - log::error!("Nameplate created without checking whether it is a number. This will be a compile error in the future. Use Nameplate::FromStr."); + log::debug!( + "Implementation of From for Nameplate is deprecated. Use the FromStr implementation instead" + ); + + if let Err(err) = Nameplate::from_str(&value) { + log::error!("{err} This will be a hard error in the future."); + } Self(value) } @@ -847,12 +862,24 @@ impl AsRef for Nameplate { #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Copy, derive_more::Display, Error)] #[non_exhaustive] pub enum ParsePasswordError { + /// Password too short #[display("Password too short. It is only {value} bytes, but must be at least {required}")] - TooShort { value: usize, required: usize }, + TooShort { + /// The calculated value + value: usize, + /// The value that is required + required: usize, + }, + /// Password does not have enough entropy #[display( "Password too weak. It can be guessed with an average of {value} tries, but must be at least {required}" )] - LittleEntropy { value: u64, required: u64 }, + LittleEntropy { + /// The calculated value + value: u64, + /// The value that is required + required: u64, + }, } /// Wormhole codes look like 4-purple-sausages, consisting of a number followed by some random words. @@ -949,13 +976,17 @@ impl FromStr for Password { } } +/// An error occurred parsing the string as a valid wormhole mailbox code #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Copy, derive_more::Display, Error)] #[non_exhaustive] -pub enum CodeFromStringError { +pub enum ParseCodeError { + /// A code must contain at least one '-' to separate nameplate from password #[display("A code must contain at least one '-' to separate nameplate from password")] SeparatorMissing, + /// An error occurred when parsing the nameplate #[display("{_0}")] Nameplate(#[from] ParseNameplateError), + /// An error occurred when parsing the code #[display("{_0}")] Password(#[from] ParsePasswordError), } @@ -982,7 +1013,9 @@ impl Code { note = "Use [`from_components`] or the [std::str::FromStr] implementation" )] pub fn new(nameplate: &Nameplate, password: &str) -> Self { - log::error!("Code created without checking the entropy of the password. This will be a compile error in the future. Use Code::FromStr."); + if let Err(err) = Password::from_str(password) { + log::error!("{err}"); + } #[allow(unsafe_code)] unsafe { @@ -1036,14 +1069,20 @@ impl From for String { /// Safety: Does not check the entropy of the password, or if one exists at all. This can be a security risk. impl From for Code { fn from(value: String) -> Self { - log::error!("Code created without checking the entropy of the password. This will be a compile error in the future. Use Code::FromStr."); + log::debug!( + "Implementation of From for Code is deprecated. Use the FromStr implementation instead" + ); + + if let Err(err) = Code::from_str(&value) { + log::error!("{err} This will be a hard error in the future."); + } Self(value) } } impl FromStr for Code { - type Err = CodeFromStringError; + type Err = ParseCodeError; fn from_str(s: &str) -> Result { match s.split_once('-') { @@ -1053,7 +1092,7 @@ impl FromStr for Code { Ok(Self(format!("{}-{}", nameplate, password))) }, - None => Err(CodeFromStringError::SeparatorMissing), + None => Err(ParseCodeError::SeparatorMissing), } } } diff --git a/src/lib.rs b/src/lib.rs index e2b4845e..b8e35bfb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,6 +40,6 @@ pub mod uri; #[allow(deprecated)] pub use crate::core::{ key::{GenericKey, Key, KeyPurpose, WormholeKey}, - rendezvous, AppConfig, AppID, Code, MailboxConnection, Mood, Nameplate, Wormhole, - WormholeError, WormholeWelcome, + rendezvous, AppConfig, AppID, Code, MailboxConnection, Mood, Nameplate, ParseCodeError, + ParseNameplateError, ParsePasswordError, Wormhole, WormholeError, WormholeWelcome, };