aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormetamuffin <metamuffin@disroot.org>2023-12-11 01:19:51 +0100
committermetamuffin <metamuffin@disroot.org>2023-12-11 01:19:51 +0100
commit36d7fb2790774c53415c96f8c6955be42bad952f (patch)
tree4481dac53a6d0896e90ff72b9b68665e59e159db
parent767d6c4c7b8518198b0343781128027051b94ae5 (diff)
downloadjellything-36d7fb2790774c53415c96f8c6955be42bad952f.tar
jellything-36d7fb2790774c53415c96f8c6955be42bad952f.tar.bz2
jellything-36d7fb2790774c53415c96f8c6955be42bad952f.tar.zst
(partially) fix security problem with federated session
-rw-r--r--Cargo.lock1
-rw-r--r--client/Cargo.toml1
-rw-r--r--client/src/lib.rs26
-rw-r--r--common/src/user.rs24
-rw-r--r--server/src/federation.rs15
-rw-r--r--server/src/routes/api/mod.rs28
-rw-r--r--server/src/routes/stream.rs17
-rw-r--r--server/src/routes/ui/account/mod.rs28
-rw-r--r--server/src/routes/ui/account/session/token.rs18
-rw-r--r--server/src/routes/ui/account/settings.rs8
10 files changed, 109 insertions, 57 deletions
diff --git a/Cargo.lock b/Cargo.lock
index c13139d..ea124d3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1366,6 +1366,7 @@ dependencies = [
"jellycommon",
"log",
"reqwest",
+ "serde",
"serde_json",
"tokio",
]
diff --git a/client/Cargo.toml b/client/Cargo.toml
index 05dd2d4..bece4c5 100644
--- a/client/Cargo.toml
+++ b/client/Cargo.toml
@@ -9,4 +9,5 @@ log = { workspace = true }
reqwest = { version = "0.11.20", features = ["json"] }
anyhow = "1.0.75"
serde_json = "1.0.107"
+serde = { version = "1.0.188", features = ["derive"] }
tokio = { workspace = true }
diff --git a/client/src/lib.rs b/client/src/lib.rs
index 92545a9..eca27fc 100644
--- a/client/src/lib.rs
+++ b/client/src/lib.rs
@@ -4,13 +4,14 @@
Copyright (C) 2023 metamuffin <metamuffin.org>
*/
use anyhow::Result;
+use jellycommon::user::UserPermission;
use log::debug;
use reqwest::{
header::{HeaderMap, HeaderValue},
Client,
};
-use serde_json::json;
-use std::time::Duration;
+use serde::Serialize;
+use std::collections::HashSet;
use stream::StreamSpec;
use tokio::io::AsyncWriteExt;
@@ -22,6 +23,14 @@ pub struct Instance {
pub tls: bool,
}
+#[derive(Serialize)]
+pub struct LoginDetails {
+ pub username: String,
+ pub password: String,
+ pub expire: Option<i64>,
+ pub drop_permissions: Option<HashSet<UserPermission>>,
+}
+
impl Instance {
pub fn new(host: String, tls: bool) -> Self {
Self { host, tls }
@@ -33,20 +42,11 @@ impl Instance {
self.host
)
}
- pub async fn login(
- self,
- username: String,
- password: String,
- expire: Duration,
- ) -> anyhow::Result<Session> {
+ pub async fn login(self, data: LoginDetails) -> anyhow::Result<Session> {
let session_token = Client::builder()
.build()?
.post(format!("{}/api/create_session", self.base()))
- .json(&json!({
- "expire": expire.as_secs(),
- "password": password,
- "username": username,
- }))
+ .json(&data)
.send()
.await?
.json()
diff --git a/common/src/user.rs b/common/src/user.rs
index ae9a757..f33ca71 100644
--- a/common/src/user.rs
+++ b/common/src/user.rs
@@ -38,18 +38,26 @@ pub struct PermissionSet(pub HashMap<UserPermission, bool>);
#[serde(rename_all = "snake_case")]
pub enum UserPermission {
Admin,
+ // ManageUsers,
+ // GenerateInvite,
+ ManageSelf,
+
+ AccessNode(String),
+ StreamFormat(StreamFormat),
Transcode,
- ManageUsers,
FederatedContent,
- GenerateInvite,
- StreamFormat(StreamFormat),
- AccessNode(String),
}
impl UserPermission {
pub const ALL_ENUMERABLE: &'static [UserPermission] = {
use UserPermission::*;
- &[Admin, Transcode, StreamFormat(user::StreamFormat::Original)]
+ &[
+ Admin,
+ Transcode,
+ ManageSelf,
+ FederatedContent,
+ StreamFormat(user::StreamFormat::Original),
+ ]
};
pub fn default_value(&self) -> bool {
use user::StreamFormat::*;
@@ -57,6 +65,7 @@ impl UserPermission {
matches!(
self,
Transcode
+ | ManageSelf
| FederatedContent
| StreamFormat(Jhls | HlsMaster | HlsVariant | Matroska | Segment | Webvtt)
)
@@ -66,6 +75,7 @@ impl UserPermission {
impl Display for UserPermission {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&match self {
+ UserPermission::ManageSelf => format!("manage self (password, display name, etc.)"),
UserPermission::FederatedContent => format!("access to federated content"),
UserPermission::Admin => format!("administrative rights"),
UserPermission::StreamFormat(StreamFormat::Original) => {
@@ -78,8 +88,8 @@ impl Display for UserPermission {
format!("downloading media via {x:?}")
}
UserPermission::Transcode => format!("transcoding"),
- UserPermission::ManageUsers => format!("management of all users"),
- UserPermission::GenerateInvite => format!("inviting new users"),
+ // UserPermission::ManageUsers => format!("management of all users"),
+ // UserPermission::GenerateInvite => format!("inviting new users"),
UserPermission::AccessNode(s) => format!("access to library node {s:?}"),
})
}
diff --git a/server/src/federation.rs b/server/src/federation.rs
index 38863e2..eb2a1ac 100644
--- a/server/src/federation.rs
+++ b/server/src/federation.rs
@@ -5,8 +5,8 @@
*/
use anyhow::anyhow;
use jellybase::CONF;
-use jellyclient::{Instance, Session};
-use std::{collections::HashMap, sync::Arc, time::Duration};
+use jellyclient::{Instance, LoginDetails, Session};
+use std::{collections::HashMap, sync::Arc};
use tokio::sync::RwLock;
pub struct Federation {
@@ -46,11 +46,12 @@ impl Federation {
let s = Arc::new(
self.get_instance(host)?
.to_owned()
- .login(
- username.to_owned(),
- password.to_owned(),
- Duration::from_secs(60 * 60 * 24 * 356),
- )
+ .login(LoginDetails {
+ username: username.to_owned(),
+ password: password.to_owned(),
+ expire: None,
+ drop_permissions: None,
+ })
.await?,
);
w.insert(host.to_owned(), s.clone());
diff --git a/server/src/routes/api/mod.rs b/server/src/routes/api/mod.rs
index 615c836..87ed0e9 100644
--- a/server/src/routes/api/mod.rs
+++ b/server/src/routes/api/mod.rs
@@ -4,12 +4,12 @@
Copyright (C) 2023 metamuffin <metamuffin.org>
*/
use super::ui::{
- account::{login_logic, session::AdminSession, LoginForm},
+ account::{login_logic, session::AdminSession},
error::MyResult,
};
use crate::database::Database;
use anyhow::{anyhow, Context};
-use jellycommon::Node;
+use jellycommon::{user::UserPermission, Node};
use rocket::{
get,
http::MediaType,
@@ -20,8 +20,9 @@ use rocket::{
serde::json::Json,
Request, State,
};
+use serde::Deserialize;
use serde_json::{json, Value};
-use std::ops::Deref;
+use std::{collections::HashSet, ops::Deref};
#[get("/api")]
pub fn r_api_root() -> Redirect {
@@ -33,9 +34,26 @@ pub fn r_api_version() -> &'static str {
"2"
}
+#[derive(Deserialize)]
+pub struct CreateSessionParams {
+ username: String,
+ password: String,
+ expire: Option<i64>,
+ drop_permissions: Option<HashSet<UserPermission>>,
+}
+
#[post("/api/create_session", data = "<data>")]
-pub fn r_api_account_login(database: &State<Database>, data: Json<LoginForm>) -> MyResult<Value> {
- let token = login_logic(database, &data.username, &data.password)?;
+pub fn r_api_account_login(
+ database: &State<Database>,
+ data: Json<CreateSessionParams>,
+) -> MyResult<Value> {
+ let token = login_logic(
+ database,
+ &data.username,
+ &data.password,
+ data.expire,
+ data.drop_permissions.clone(),
+ )?;
Ok(json!(token))
}
diff --git a/server/src/routes/stream.rs b/server/src/routes/stream.rs
index 5944ace..279a621 100644
--- a/server/src/routes/stream.rs
+++ b/server/src/routes/stream.rs
@@ -10,6 +10,7 @@ use jellybase::{
permission::{NodePermissionExt, PermissionSetExt},
CONF,
};
+use jellyclient::LoginDetails;
use jellycommon::{stream::StreamSpec, user::UserPermission, MediaSource};
use log::{info, warn};
use rocket::{
@@ -19,7 +20,7 @@ use rocket::{
response::{self, Redirect, Responder},
Either, Request, Response, State,
};
-use std::{ops::Range, time::Duration};
+use std::{collections::HashSet, ops::Range};
use tokio::io::{duplex, DuplexStream};
#[head("/n/<_id>/stream?<spec>")]
@@ -71,11 +72,15 @@ pub async fn r_stream(
info!("creating session on {host}");
let instance = federation.get_instance(&host)?.to_owned();
let session = instance
- .login(
- username.to_owned(),
- password.to_owned(),
- Duration::from_secs(60),
- )
+ .login(LoginDetails {
+ username: username.to_owned(),
+ password: password.to_owned(),
+ expire: Some(60),
+ drop_permissions: Some(HashSet::from_iter([
+ UserPermission::ManageSelf,
+ UserPermission::Admin, // in case somebody federated the admin :)))
+ ])),
+ })
.await?;
let uri = session.stream(&remote_id, &spec);
diff --git a/server/src/routes/ui/account/mod.rs b/server/src/routes/ui/account/mod.rs
index 88f6f45..1c5b19a 100644
--- a/server/src/routes/ui/account/mod.rs
+++ b/server/src/routes/ui/account/mod.rs
@@ -6,6 +6,8 @@
pub mod session;
pub mod settings;
+use std::collections::HashSet;
+
use super::{error::MyError, layout::LayoutPage};
use crate::{
database::Database,
@@ -16,7 +18,7 @@ use anyhow::anyhow;
use argon2::{password_hash::Salt, Argon2, PasswordHasher};
use chrono::Duration;
use jellybase::CONF;
-use jellycommon::user::{PermissionSet, Theme, User};
+use jellycommon::user::{PermissionSet, Theme, User, UserPermission};
use rocket::{
form::{Contextual, Form},
get,
@@ -162,7 +164,7 @@ pub fn r_account_login_post(
jar.add(
Cookie::build(
"session",
- login_logic(database, &form.username, &form.password)?,
+ login_logic(database, &form.username, &form.password, None, None)?,
)
.permanent()
.finish(),
@@ -177,11 +179,17 @@ pub fn r_account_logout_post(jar: &CookieJar) -> MyResult<Redirect> {
Ok(Redirect::found(rocket::uri!(r_home())))
}
-pub fn login_logic(database: &Database, username: &str, password: &str) -> MyResult<String> {
+pub fn login_logic(
+ database: &Database,
+ username: &str,
+ password: &str,
+ expire: Option<i64>,
+ drop_permissions: Option<HashSet<UserPermission>>,
+) -> MyResult<String> {
// hashing the password regardless if the accounts exists to prevent timing attacks
let password = hash_password(username, password);
- let user = database
+ let mut user = database
.user
.get(&username.to_string())?
.ok_or(anyhow!("invalid password"))?;
@@ -190,9 +198,17 @@ pub fn login_logic(database: &Database, username: &str, password: &str) -> MyRes
Err(anyhow!("invalid password"))?
}
+ if let Some(ep) = drop_permissions {
+ // remove all grant perms that are in `ep`
+ user.permissions
+ .0
+ .retain(|p, val| if *val { !ep.contains(p) } else { true })
+ }
+
Ok(session::token::create(
- &user,
- Duration::days(CONF.login_expire),
+ user.name,
+ user.permissions,
+ Duration::days(CONF.login_expire.min(expire.unwrap_or(i64::MAX))),
))
}
diff --git a/server/src/routes/ui/account/session/token.rs b/server/src/routes/ui/account/session/token.rs
index b6c22f7..969207d 100644
--- a/server/src/routes/ui/account/session/token.rs
+++ b/server/src/routes/ui/account/session/token.rs
@@ -12,7 +12,7 @@ use anyhow::anyhow;
use base64::Engine;
use chrono::{Duration, Utc};
use jellybase::CONF;
-use jellycommon::user::User;
+use jellycommon::user::PermissionSet;
use log::warn;
use std::sync::LazyLock;
@@ -29,11 +29,11 @@ static SESSION_KEY: LazyLock<[u8; 32]> = LazyLock::new(|| {
}
});
-pub fn create(user: &User, expire: Duration) -> String {
+pub fn create(username: String, permissions: PermissionSet, expire: Duration) -> String {
let session_data = SessionData {
expire: Utc::now() + expire,
- username: user.name.to_owned(),
- permissions: user.permissions.clone(),
+ username: username.to_owned(),
+ permissions,
};
let mut plaintext =
bincode::serde::encode_to_vec(&session_data, bincode::config::standard()).unwrap();
@@ -73,14 +73,8 @@ pub fn validate(token: &str) -> anyhow::Result<String> {
#[test]
fn test() {
let tok = create(
- &User {
- name: "blub".to_string(),
- display_name: "blub".to_owned(),
- password: vec![],
- admin: false,
- permissions: jellycommon::user::PermissionSet::default(),
- theme: jellycommon::user::Theme::Dark,
- },
+ "blub".to_string(),
+ jellycommon::user::PermissionSet::default(),
Duration::days(1),
);
validate(&tok).unwrap();
diff --git a/server/src/routes/ui/account/settings.rs b/server/src/routes/ui/account/settings.rs
index 2192d43..90dcf37 100644
--- a/server/src/routes/ui/account/settings.rs
+++ b/server/src/routes/ui/account/settings.rs
@@ -13,7 +13,8 @@ use crate::{
},
uri,
};
-use jellycommon::user::Theme;
+use jellybase::permission::PermissionSetExt;
+use jellycommon::user::{Theme, UserPermission};
use rocket::{
form::{self, validate::len, Contextual, Form},
get,
@@ -95,6 +96,11 @@ pub fn r_account_settings_post(
database: &State<Database>,
form: Form<Contextual<SettingsForm>>,
) -> MyResult<DynLayoutPage<'static>> {
+ session
+ .user
+ .permissions
+ .assert(&UserPermission::ManageSelf)?;
+
let form = match &form.value {
Some(v) => v,
None => return Ok(settings_page(session, Some(Err(format_form_error(form))))),