From 2b67da142556c45ae7cc8516a4be7e80cb0d6565 Mon Sep 17 00:00:00 2001 From: evdokimovs Date: Tue, 15 Oct 2024 22:23:04 -0300 Subject: [PATCH 1/5] chore: add `secrecy` crate --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 604778643..88ff182f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ derive_more = { version = "1.0.0-beta.6", features = ["debug", "display", "error futures = "0.3.30" log = "0.4" rand = "0.8" +secrecy = "0.10" stun_codec = "0.3.5" tokio = { version = "1.32", default-features = false, features = ["io-util", "macros", "net", "rt-multi-thread", "time"] } tokio-util = { version = "0.7.11", features = ["codec"] } From 9263659ef9dc961eeb2199de39928fcd0274d145 Mon Sep 17 00:00:00 2001 From: evdokimovs Date: Tue, 15 Oct 2024 22:23:50 -0300 Subject: [PATCH 2/5] refactor: use `secrecy::SecretString` for ICE password --- src/lib.rs | 5 +++-- src/server/request.rs | 47 +++++++++++++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 35a18391c..2688dfd1b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -159,6 +159,7 @@ pub mod transport; use std::{net::SocketAddr, sync::Arc}; use derive_more::{Display, Error as StdError, From}; +use secrecy::SecretString; #[cfg(test)] pub(crate) use self::allocation::Allocation; @@ -190,7 +191,7 @@ pub trait AuthHandler { username: &str, realm: &str, src_addr: SocketAddr, - ) -> Result, Error>; + ) -> Result; } impl AuthHandler for Arc { @@ -199,7 +200,7 @@ impl AuthHandler for Arc { username: &str, realm: &str, src_addr: SocketAddr, - ) -> Result, Error> { + ) -> Result { (**self).auth_handle(username, realm, src_addr) } } diff --git a/src/server/request.rs b/src/server/request.rs index 0411d7f7c..6dbdae86f 100644 --- a/src/server/request.rs +++ b/src/server/request.rs @@ -9,6 +9,7 @@ use std::{ }; use rand::{distributions::Alphanumeric, random, Rng}; +use secrecy::{ExposeSecret, SecretString}; use stun_codec::{ rfc5389::{ errors::{BadRequest, StaleNonce, Unauthorized, UnknownAttribute}, @@ -205,7 +206,7 @@ async fn handle_allocate_request( five_tuple: FiveTuple, uname: Username, realm: Realm, - pass: Box, + pass: SecretString, ) -> Result<(), Error> { // 1. The server MUST require that the request be authenticated. This // authentication MUST be done using the long-term credential @@ -413,7 +414,10 @@ async fn handle_allocate_request( msg.add_attribute(XorMappedAddress::new(five_tuple.src_addr)); let integrity = MessageIntegrity::new_long_term_credential( - &msg, &uname, &realm, &pass, + &msg, + &uname, + &realm, + pass.expose_secret(), ) .map_err(|e| Error::Encode(*e.kind()))?; msg.add_attribute(integrity); @@ -436,7 +440,7 @@ async fn authenticate_request( nonces: &mut HashMap, five_tuple: FiveTuple, realm: &str, -) -> Result)>, Error> { +) -> Result, Error> { let Some(integrity) = msg.get_attribute::() else { respond_with_nonce( msg, @@ -504,9 +508,11 @@ async fn authenticate_request( return Err(Error::NoSuchUser); }; - if let Err(err) = - integrity.check_long_term_credential(uname_attr, realm_attr, &pass) - { + if let Err(err) = integrity.check_long_term_credential( + uname_attr, + realm_attr, + pass.expose_secret(), + ) { respond_with_err(msg, err, conn, five_tuple.src_addr).await?; Err(Error::IntegrityMismatch) @@ -554,7 +560,7 @@ async fn handle_refresh_request( five_tuple: FiveTuple, uname: Username, realm: Realm, - pass: Box, + pass: SecretString, ) -> Result<(), Error> { log::trace!("Received `RefreshRequest` from {}", five_tuple.src_addr); @@ -600,9 +606,13 @@ async fn handle_refresh_request( Lifetime::new(lifetime_duration) .map_err(|e| Error::Encode(*e.kind()))?, ); - let integrity = - MessageIntegrity::new_long_term_credential(&msg, &uname, &realm, &pass) - .map_err(|e| Error::Encode(*e.kind()))?; + let integrity = MessageIntegrity::new_long_term_credential( + &msg, + &uname, + &realm, + pass.expose_secret(), + ) + .map_err(|e| Error::Encode(*e.kind()))?; msg.add_attribute(integrity); send_to(msg, conn, five_tuple.src_addr).await @@ -622,7 +632,7 @@ async fn handle_create_permission_request( five_tuple: FiveTuple, uname: Username, realm: Realm, - pass: Box, + pass: SecretString, ) -> Result<(), Error> { log::trace!("Received `CreatePermission` from {}", five_tuple.src_addr); @@ -672,7 +682,10 @@ async fn handle_create_permission_request( let mut msg = Message::new(resp_class, CREATE_PERMISSION, msg.transaction_id()); let integrity = MessageIntegrity::new_long_term_credential( - &msg, &uname, &realm, &pass, + &msg, + &uname, + &realm, + pass.expose_secret(), ) .map_err(|e| Error::Encode(*e.kind()))?; msg.add_attribute(integrity); @@ -732,7 +745,7 @@ async fn handle_channel_bind_request( channel_bind_lifetime: Duration, uname: Username, realm: Realm, - pass: Box, + pass: SecretString, ) -> Result<(), Error> { if let Some(alloc) = allocs.get_alloc(&five_tuple) { let Some(ch_num) = @@ -788,7 +801,10 @@ async fn handle_channel_bind_request( ); let integrity = MessageIntegrity::new_long_term_credential( - &msg, &uname, &realm, &pass, + &msg, + &uname, + &realm, + pass.expose_secret(), ) .map_err(|e| Error::Encode(*e.kind()))?; msg.add_attribute(integrity); @@ -963,6 +979,7 @@ mod handle_spec { }; use rand::random; + use secrecy::SecretString; use stun_codec::{ rfc5766::methods::REFRESH, Message, MessageClass, TransactionId, }; @@ -991,7 +1008,7 @@ mod handle_spec { _username: &str, _realm: &str, _src_addr: SocketAddr, - ) -> Result, Error> { + ) -> Result { Ok(STATIC_KEY.to_owned().into()) } } From df13f4e8815cb03c0c561fd7e567bc565a2ee800 Mon Sep 17 00:00:00 2001 From: evdokimovs Date: Wed, 16 Oct 2024 10:24:47 -0300 Subject: [PATCH 3/5] chore: update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0d446f44..30a5e7130 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,11 @@ All user visible changes to this project will be documented in this file. This p ### BC Breaks - Bumped up [MSRV] to 1.81 because for `#[expect]` attribute usage. ([todo]) +- Changed return type of `AuthHandler::auth_handle` to [`SecretString`]. ([#3]) +[`SecretString`]: https://docs.rs/secrecy/0.10.3/secrecy/type.SecretString.html [todo]: /../../commit/todo +[#3]: /../../pull/3 From bb91a865b5323e3fa61b885ce1bd5644fb128b2c Mon Sep 17 00:00:00 2001 From: evdokimovs Date: Wed, 16 Oct 2024 10:28:34 -0300 Subject: [PATCH 4/5] chore: add commit ref to CHANGELOG --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a5e7130..1371f0a0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,11 +13,11 @@ All user visible changes to this project will be documented in this file. This p ### BC Breaks -- Bumped up [MSRV] to 1.81 because for `#[expect]` attribute usage. ([todo]) +- Bumped up [MSRV] to 1.81 because for `#[expect]` attribute usage. ([b0a1dfb6]) - Changed return type of `AuthHandler::auth_handle` to [`SecretString`]. ([#3]) [`SecretString`]: https://docs.rs/secrecy/0.10.3/secrecy/type.SecretString.html -[todo]: /../../commit/todo +[b0a1dfb6]: /../../commit/b0a1dfb696b044d08fa720f2d3e52ed65a12e521 [#3]: /../../pull/3 From 6b028afd620abf4a76a210307b4fa8ca923e48e6 Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 16 Oct 2024 22:48:28 +0300 Subject: [PATCH 5/5] Corrections --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1371f0a0e..071e248fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,11 @@ All user visible changes to this project will be documented in this file. This p ### BC Breaks - Bumped up [MSRV] to 1.81 because for `#[expect]` attribute usage. ([b0a1dfb6]) -- Changed return type of `AuthHandler::auth_handle` to [`SecretString`]. ([#3]) +- Changed return type of `AuthHandler::auth_handle()` to [`secrecy::SecretString`]. ([#3]) -[`SecretString`]: https://docs.rs/secrecy/0.10.3/secrecy/type.SecretString.html -[b0a1dfb6]: /../../commit/b0a1dfb696b044d08fa720f2d3e52ed65a12e521 +[`secrecy::SecretString`]: https://docs.rs/secrecy/0.10.3/secrecy/type.SecretString.html [#3]: /../../pull/3 +[b0a1dfb6]: /../../commit/b0a1dfb696b044d08fa720f2d3e52ed65a12e521