fix(types,blahd): reject timestamps > i64::MAX

This commit is contained in:
oxalica 2024-09-24 20:36:27 -04:00
parent c4fbf1294b
commit a38b59da84
5 changed files with 27 additions and 12 deletions

View file

@ -29,6 +29,8 @@ pub struct VerifyError(#[from] VerifyErrorImpl);
enum VerifyErrorImpl { enum VerifyErrorImpl {
#[error("profile id_key mismatch")] #[error("profile id_key mismatch")]
ProfileIdKeyMismatch, ProfileIdKeyMismatch,
#[error("act_key[{0}] has invalid expiring timestamp")]
ActKeyTimestamp(usize),
#[error("act_key[{0}] not signed by id_key")] #[error("act_key[{0}] not signed by id_key")]
ActKeySigner(usize), ActKeySigner(usize),
#[error("invalid act_key[{0}] signature: {1}")] #[error("invalid act_key[{0}] signature: {1}")]
@ -66,6 +68,9 @@ impl UserIdentityDesc {
{ {
return Err(VerifyErrorImpl::ActKeySigner(i).into()); return Err(VerifyErrorImpl::ActKeySigner(i).into());
} }
if i64::try_from(kdesc.expire_time).is_err() {
return Err(VerifyErrorImpl::ActKeyTimestamp(i).into());
}
signed_kdesc signed_kdesc
.verify() .verify()
.map_err(|err| VerifyErrorImpl::ActKeySignature(i, err))?; .map_err(|err| VerifyErrorImpl::ActKeySignature(i, err))?;
@ -335,6 +340,19 @@ mod tests {
VerifyErrorImpl::ActKeySigner(1), VerifyErrorImpl::ActKeySigner(1),
); );
// Timestamp overflows i64.
id_desc.act_keys[1] = UserActKeyDesc {
act_key: act_pub.clone(),
expire_time: u64::MAX,
comment: String::new(),
}
.sign_msg_with(&id_key, &id_priv, TIMESTAMP, rng)
.unwrap();
assert_err!(
id_desc.verify(None, TIMESTAMP),
VerifyErrorImpl::ActKeyTimestamp(1),
);
// OK act key. // OK act key.
id_desc.act_keys[1] = UserActKeyDesc { id_desc.act_keys[1] = UserActKeyDesc {
act_key: act_pub.clone(), act_key: act_pub.clone(),

View file

@ -41,6 +41,8 @@ max_page_len = 1024
max_request_len = 4096 max_request_len = 4096
# The maximum timestamp tolerance in seconds for request validation. # The maximum timestamp tolerance in seconds for request validation.
# NB. This should be small enough to reject timestamps overflowing `i64`,
# otherwise it may cause panics.
timestamp_tolerance_secs = 90 timestamp_tolerance_secs = 90
[server.feed] [server.feed]

View file

@ -432,8 +432,7 @@ pub trait TransactionOps {
stmt.execute(named_params! { stmt.execute(named_params! {
":uid": uid, ":uid": uid,
":act_key": kdesc.signee.payload.act_key, ":act_key": kdesc.signee.payload.act_key,
// FIXME: Other `u64` that will be stored in database should also be range checked. ":expire_time": i64::try_from(kdesc.signee.payload.expire_time).expect("verified timestamp"),
":expire_time": kdesc.signee.payload.expire_time.min(i64::MAX as _),
})?; })?;
} }
@ -537,7 +536,7 @@ pub trait TransactionOps {
fn add_room_chat_msg(&self, rid: Id, uid: i64, cid: Id, chat: &SignedChatMsg) -> Result<()> { fn add_room_chat_msg(&self, rid: Id, uid: i64, cid: Id, chat: &SignedChatMsg) -> Result<()> {
let conn = self.conn(); let conn = self.conn();
let act_key = &chat.signee.user.act_key; let act_key = &chat.signee.user.act_key;
let timestamp = chat.signee.timestamp; let timestamp = i64::try_from(chat.signee.timestamp).expect("verified timestamp");
let nonce = chat.signee.nonce; let nonce = chat.signee.nonce;
let rich_text = &chat.signee.payload.rich_text; let rich_text = &chat.signee.payload.rich_text;
let sig = &chat.sig; let sig = &chat.sig;

View file

@ -1,6 +1,6 @@
use std::num::NonZero; use std::num::NonZero;
use std::sync::Arc; use std::sync::Arc;
use std::time::{Duration, SystemTime}; use std::time::Duration;
use anyhow::Result; use anyhow::Result;
use axum::extract::{ws, OriginalUri}; use axum::extract::{ws, OriginalUri};
@ -11,8 +11,8 @@ use axum::routing::{get, post};
use axum::{Json, Router}; use axum::{Json, Router};
use axum_extra::extract::WithRejection as R; use axum_extra::extract::WithRejection as R;
use blah_types::{ use blah_types::{
ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload, DeleteRoomPayload, Id, get_timestamp, ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload, DeleteRoomPayload,
MemberPermission, RoomAdminOp, RoomAdminPayload, RoomAttrs, RoomMetadata, ServerPermission, Id, MemberPermission, RoomAdminOp, RoomAdminPayload, RoomAttrs, RoomMetadata, ServerPermission,
Signed, SignedChatMsg, UserKey, UserRegisterPayload, WithMsgId, X_BLAH_DIFFICULTY, Signed, SignedChatMsg, UserKey, UserRegisterPayload, WithMsgId, X_BLAH_DIFFICULTY,
X_BLAH_NONCE, X_BLAH_NONCE,
}; };
@ -103,11 +103,7 @@ impl AppState {
fn verify_signed_data<T: Serialize>(&self, data: &Signed<T>) -> Result<(), ApiError> { fn verify_signed_data<T: Serialize>(&self, data: &Signed<T>) -> Result<(), ApiError> {
api_ensure!(data.verify().is_ok(), "signature verification failed"); api_ensure!(data.verify().is_ok(), "signature verification failed");
let timestamp_diff = SystemTime::now() let timestamp_diff = get_timestamp().abs_diff(data.signee.timestamp);
.duration_since(SystemTime::UNIX_EPOCH)
.expect("after UNIX epoch")
.as_secs()
.abs_diff(data.signee.timestamp);
api_ensure!( api_ensure!(
timestamp_diff <= self.config.timestamp_tolerance_secs, timestamp_diff <= self.config.timestamp_tolerance_secs,
"invalid timestamp", "invalid timestamp",

View file

@ -1190,7 +1190,7 @@ async fn register_flow(server: Server) {
// Sign using id_key. // Sign using id_key.
let act_key = UserActKeyDesc { let act_key = UserActKeyDesc {
act_key: CAROL.pubkeys.act_key.clone(), act_key: CAROL.pubkeys.act_key.clone(),
expire_time: u64::MAX, expire_time: i64::MAX as _,
comment: "comment".into(), comment: "comment".into(),
} }
.sign_msg(&CAROL.pubkeys.id_key, &CAROL.id_priv) .sign_msg(&CAROL.pubkeys.id_key, &CAROL.id_priv)