Commit 9c05b1dd327a
tarball5 files changed+108-7
@@ -213,6 +213,14 @@ engine stopping ends nothing, `Stopped` covers it
213213
with an encrypted store, refresh tokens and threads. the store is sqlite214214
natively and indexeddb in a browser. secret-bearing inputs don't derive `Clone`215215
216
+the homeserver a person typed is checked before the client sees it, and a bad217
+one is `AccountError::InvalidHomeserver`. a bare server name passes untouched,218
+since that's what sends the sdk to `.well-known`, and a host that "helpfully"219
+adds `https://` turns `example.org` into a homeserver url and skips discovery.220
+a url needs a host, no userinfo, and https. plain http needs221
+`allow_insecure_loopback` and a loopback host. `SoftLogout::password_login` sets222
+it, since its homeserver passed once already223
+216224
a `SessionStore` owns one login's record and its store. login saves the first217225
record, `restore` loads it, and the engine a `Session` builds saves every token218226
rotation after that, since a refresh token works once and a host that missed one@@ -33,7 +33,7 @@ pub async fn smoke(
3333
let sessions = Arc::new(EncryptedSessionStore::indexeddb(3434
StoreConfig::indexeddb(store_name.clone(), "macltok-web"),3535
));36
- let login = PasswordLogin::new(homeserver, username, password);36
+ let login = PasswordLogin::new(homeserver, username, password).allow_insecure_loopback(true);3737
let session = Session::login_password(login, sessions).await?;3838
let (engine, handle, events) = session.build(EngineConfig::default()).await?;3939
let (panicked, panics) = mpsc::unbounded();@@ -95,6 +95,7 @@ fn login() -> PasswordLogin {
9595
env!("MACLTOK_USERNAME"),9696
env!("MACLTOK_PASSWORD"),9797
)98
+ .allow_insecure_loopback(true)9899
}99100
100101
fn sessions(store: &str) -> Arc<dyn SessionStore> {@@ -55,6 +55,8 @@ pub struct PasswordLogin {
5555
pub device_display_name: Option<String>,5656
/// set after a soft logout so the store's keys still belong to us5757
pub device_id: Option<OwnedDeviceId>,58
+ /// let plain http through, for loopback hosts only59
+ pub allow_insecure_loopback: bool,5860
}5961
6062
impl PasswordLogin {@@ -70,6 +72,7 @@ impl PasswordLogin {
7072
password: password.into(),7173
device_display_name: None,7274
device_id: None,75
+ allow_insecure_loopback: false,7376
}7477
}7578
@@ -84,6 +87,12 @@ impl PasswordLogin {
8487
self.device_id = Some(device_id);8588
self8689
}90
+91
+ #[must_use]92
+ pub fn allow_insecure_loopback(mut self, allow: bool) -> Self {93
+ self.allow_insecure_loopback = allow;94
+ self95
+ }8796
}8897
8998
#[non_exhaustive]@@ -95,6 +104,8 @@ pub struct OAuthLogin {
95104
pub client_id: Option<String>,96105
pub device_id: Option<OwnedDeviceId>,97106
pub application_type: OAuthApplicationType,107
+ /// let plain http through, for loopback hosts only108
+ pub allow_insecure_loopback: bool,98109
}99110
100111
impl OAuthLogin {@@ -113,6 +124,7 @@ impl OAuthLogin {
113124
client_id: None,114125
device_id: None,115126
application_type,127
+ allow_insecure_loopback: false,116128
}117129
}118130
@@ -134,6 +146,12 @@ impl OAuthLogin {
134146
self.device_id = Some(device_id);135147
self136148
}149
+150
+ #[must_use]151
+ pub fn allow_insecure_loopback(mut self, allow: bool) -> Self {152
+ self.allow_insecure_loopback = allow;153
+ self154
+ }137155
}138156
139157
#[derive(Clone, Copy, Debug, Eq, PartialEq)]@@ -341,6 +359,7 @@ impl SoftLogout {
341359
pub fn password_login(&self, password: impl Into<String>) -> PasswordLogin {342360
PasswordLogin::new(self.homeserver.clone(), self.user_id.as_str(), password)343361
.with_device_id(self.device_id.clone())362
+ .allow_insecure_loopback(true)344363
}345364
}346365
@@ -360,6 +379,9 @@ pub enum AccountError {
360379
UnsupportedAuthentication,361380
#[error("the OAuth callback was already submitted")]362381
OAuthCallbackAlreadySubmitted,382
+ /// what was typed can't name a homeserver, so nothing was sent anywhere383
+ #[error("that isn't a usable homeserver, {0}")]384
+ InvalidHomeserver(&'static str),363385
/// an IndexedDB delete is waiting on the store's other connections. it goes through364386
/// once they close, and opening the store again waits behind it365387
#[error("the store is still open elsewhere, it will be deleted once that closes")]@@ -431,9 +453,10 @@ impl Session {
431453
login: PasswordLogin,432454
sessions: Arc<dyn SessionStore>,433455
) -> Result<Self, AccountError> {456
+ let homeserver = homeserver_input(&login.homeserver, login.allow_insecure_loopback)?;434457
let store_lock = sessions.store().lock().await?;435458
let client = matrix_client_builder(sessions.store())436
- .server_name_or_homeserver_url(login.homeserver.trim())459
+ .server_name_or_homeserver_url(homeserver)437460
.build()438461
.await439462
.map_err(|error| account_error("building the Matrix client", error))?;@@ -464,9 +487,10 @@ impl Session {
464487
.map_err(|error| account_error("parsing the OAuth redirect URI", error))?;465488
let client_uri = Url::parse(login.client_uri.trim())466489
.map_err(|error| account_error("parsing the OAuth client URI", error))?;490
+ let homeserver = homeserver_input(&login.homeserver, login.allow_insecure_loopback)?;467491
let store_lock = sessions.store().lock().await?;468492
let client = matrix_client_builder(sessions.store())469
- .server_name_or_homeserver_url(login.homeserver.trim())493
+ .server_name_or_homeserver_url(homeserver)470494
.build()471495
.await472496
.map_err(|error| account_error("building the Matrix client", error))?;@@ -693,6 +717,43 @@ fn matrix_client_builder(store: &StoreConfig) -> ClientBuilder {
693717
}694718
}695719
720
+/// a bare server name goes through untouched, since that's what makes the sdk try `.well-known`721
+fn homeserver_input(input: &str, allow_insecure_loopback: bool) -> Result<&str, AccountError> {722
+ let input = input.trim();723
+ let invalid = |reason| Err(AccountError::InvalidHomeserver(reason));724
+ if input.is_empty() {725
+ return invalid("it's empty");726
+ }727
+ if input.contains(char::is_whitespace) {728
+ return invalid("it has spaces in it");729
+ }730
+ if !input.contains("://") {731
+ return match <&ruma::ServerName>::try_from(input) {732
+ Ok(_) => Ok(input),733
+ Err(_) => invalid("it's neither a server name nor a url"),734
+ };735
+ }736
+ let Ok(url) = Url::parse(input) else {737
+ return invalid("it isn't a valid url");738
+ };739
+ if !url.username().is_empty() || url.password().is_some() {740
+ return invalid("it has a username in it");741
+ }742
+ let loopback = match url.host() {743
+ None => return invalid("it has no host"),744
+ Some(url::Host::Domain(domain)) => domain.eq_ignore_ascii_case("localhost"),745
+ Some(url::Host::Ipv4(address)) => address.is_loopback(),746
+ Some(url::Host::Ipv6(address)) => address.is_loopback(),747
+ };748
+ match url.scheme() {749
+ "https" => Ok(input),750
+ "http" if allow_insecure_loopback && loopback => Ok(input),751
+ "http" if allow_insecure_loopback => invalid("plain http only reaches loopback hosts"),752
+ "http" => invalid("it isn't https"),753
+ _ => invalid("only http and https are spoken"),754
+ }755
+}756
+696757
pub(crate) fn account_error(697758
context: &'static str,698759
source: impl std::error::Error + SendOutsideWasm + SyncOutsideWasm + 'static,@@ -36,6 +36,7 @@ async fn password_session_round_trips_and_discards_with_its_store() {
3636
let config = EngineConfig::default().with_initial_history_limit(0);3737
let session = Session::login_password(3838
PasswordLogin::new(server.uri(), "example", "wordpass")39
+ .allow_insecure_loopback(true)3940
.with_device_display_name("macltok test"),4041
Arc::new(sessions()),4142
)@@ -89,7 +90,7 @@ async fn a_restart_after_a_soft_logout_still_knows_the_device() {
8990
)9091
};9192
Session::login_password(92
- PasswordLogin::new(server.uri(), "example", "wordpass"),93
+ PasswordLogin::new(server.uri(), "example", "wordpass").allow_insecure_loopback(true),9394
Arc::new(sessions()),9495
)9596
.await@@ -140,7 +141,7 @@ async fn a_login_whose_first_save_fails_can_still_be_saved() {
140141
std::fs::set_permissions(&records, read_only).unwrap();141142
142143
let login = Session::login_password(143
- PasswordLogin::new(server.uri(), "example", "wordpass"),144
+ PasswordLogin::new(server.uri(), "example", "wordpass").allow_insecure_loopback(true),144145
Arc::new(sessions()),145146
)146147
.await;@@ -169,7 +170,7 @@ async fn a_store_takes_one_session_at_a_time() {
169170
)170171
};171172
let (engine, handle, events) = Session::login_password(172
- PasswordLogin::new(server.uri(), "example", "wordpass"),173
+ PasswordLogin::new(server.uri(), "example", "wordpass").allow_insecure_loopback(true),173174
Arc::new(sessions()),174175
)175176
.await@@ -279,7 +280,7 @@ async fn a_refresh_is_saved_before_anyone_hears_of_it() {
279280
StoreConfig::sqlite(root.join("store"), "test store passphrase"),280281
));281282
let session = Session::login_password(282
- PasswordLogin::new(server.uri(), "example", "wordpass"),283
+ PasswordLogin::new(server.uri(), "example", "wordpass").allow_insecure_loopback(true),283284
sessions.clone(),284285
)285286
.await@@ -300,6 +301,36 @@ async fn a_refresh_is_saved_before_anyone_hears_of_it() {
300301
std::fs::remove_dir_all(root).unwrap();301302
}302303
304
+#[tokio::test]305
+async fn a_homeserver_that_cant_be_one_is_refused_before_anything_is_sent() {306
+ let root = temporary_path();307
+ let sessions: Arc<dyn SessionStore> = Arc::new(EncryptedSessionStore::file(308
+ root.join("session"),309
+ StoreConfig::sqlite(root.join("store"), "test store passphrase"),310
+ ));311
+ for (homeserver, loopback) in [312
+ ("", true),313
+ (" ", true),314
+ ("example .org", true),315
+ ("ftp://example.org", true),316
+ ("https://user@example.org", true),317
+ ("https://", true),318
+ ("example.org/path", true),319
+ ("http://example.org", false),320
+ ("http://example.org", true),321
+ ("http://localhost:8008", false),322
+ ] {323
+ let login =324
+ PasswordLogin::new(homeserver, "example", "wordpass").allow_insecure_loopback(loopback);325
+ let refused = Session::login_password(login, sessions.clone()).await;326
+ assert!(327
+ matches!(refused, Err(AccountError::InvalidHomeserver(_))),328
+ "{homeserver:?} got through"329
+ );330
+ }331
+ assert!(!root.exists());332
+}333
+303334
fn record(homeserver: &str) -> SessionRecord {304335
serde_json::from_value(serde_json::json!({305336
"homeserver": homeserver,