Skip to content

Commit e13aa2d

Browse files
committed
Review changes
1 parent c5291a1 commit e13aa2d

File tree

2 files changed

+109
-96
lines changed

2 files changed

+109
-96
lines changed

psst-core/src/session/login5.rs

Lines changed: 83 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ use crate::error::Error;
44
use crate::session::client_token::ClientTokenProvider;
55
use crate::session::token::Token;
66
use crate::session::SessionService;
7+
use crate::system_info::{CLIENT_ID, DEVICE_ID};
78
use crate::util::{default_ureq_agent_builder, solve_hash_cash};
89
use librespot_protocol::login5::login_response::Response;
910
use librespot_protocol::{
1011
client_info::ClientInfo,
11-
credentials::{StoredCredential},
12+
credentials::StoredCredential,
1213
hashcash::HashcashSolution,
1314
login5::{
14-
login_request::Login_method, ChallengeSolution, LoginError, LoginRequest,
15-
LoginResponse,
15+
login_request::Login_method, ChallengeSolution, LoginError, LoginRequest, LoginResponse,
1616
},
1717
};
1818
use parking_lot::Mutex;
@@ -21,58 +21,61 @@ use protobuf::{Message, MessageField};
2121
use std::fmt::Formatter;
2222
use std::time::{Duration, Instant};
2323
use std::{error, fmt, thread};
24-
use crate::system_info::{CLIENT_ID, DEVICE_ID};
2524

2625
const MAX_LOGIN_TRIES: u8 = 3;
2726
const LOGIN_TIMEOUT: Duration = Duration::from_secs(3);
2827

28+
#[derive(Debug)]
29+
pub enum ChallengeError {
30+
Unsupported,
31+
NoneFound,
32+
}
33+
2934
#[derive(Debug)]
3035
enum Login5Error {
31-
FaultyRequest(LoginError),
32-
CodeChallenge,
33-
NoStoredCredentials,
34-
RetriesFailed(u8),
36+
/// The server denied the request with a specific error code.
37+
RequestDenied(LoginError),
38+
/// The server issued a challenge that we could not solve.
39+
Challenge(ChallengeError),
40+
/// The operation could not be performed due to invalid local state.
41+
InvalidState(String),
42+
/// The login attempt failed after multiple retries.
43+
RetriesExceeded(u8),
44+
/// The server's response was malformed or missing expected fields.
45+
MalformedResponse,
3546
}
3647

3748
impl error::Error for Login5Error {}
3849

3950
impl fmt::Display for Login5Error {
4051
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
4152
match self {
42-
Login5Error::FaultyRequest(e) => {
43-
write!(f, "Login request was denied {:?}", e)
44-
}
45-
Login5Error::CodeChallenge => {
46-
write!(f, "Login5 code challenge is not supported")
47-
}
48-
Login5Error::NoStoredCredentials => {
49-
write!(f, "Tried to acquire access token without stored credentials")
50-
}
51-
Login5Error::RetriesFailed(u8) => {
52-
write!(f, "Couldn't successfully authenticate after {:?} times", u8)
53+
Login5Error::RequestDenied(e) => write!(f, "Login request was denied: {:?}", e),
54+
Login5Error::Challenge(c) => match c {
55+
ChallengeError::Unsupported => write!(f, "Login5 code challenge is not supported"),
56+
ChallengeError::NoneFound => write!(f, "No challenges found in response"),
57+
},
58+
Login5Error::InvalidState(s) => write!(f, "Invalid state: {}", s),
59+
Login5Error::RetriesExceeded(n) => {
60+
write!(f, "Couldn't successfully authenticate after {} times", n)
5361
}
62+
Login5Error::MalformedResponse => write!(f, "Missing response from login server"),
5463
}
5564
}
5665
}
5766

5867
impl From<Login5Error> for Error {
5968
fn from(err: Login5Error) -> Self {
6069
match err {
61-
Login5Error::NoStoredCredentials | Login5Error::RetriesFailed(_) | Login5Error::FaultyRequest(_) => {
62-
Error::InvalidStateError(err.into())
63-
}
64-
Login5Error::CodeChallenge => Error::UnimplementedError(err.into()),
70+
Login5Error::RequestDenied(_)
71+
| Login5Error::InvalidState(_)
72+
| Login5Error::RetriesExceeded(_)
73+
| Login5Error::MalformedResponse => Error::InvalidStateError(err.into()),
74+
Login5Error::Challenge(_) => Error::UnimplementedError(err.into()),
6575
}
6676
}
6777
}
6878

69-
#[derive(Debug)]
70-
pub struct Login5Request {
71-
pub uri: String,
72-
pub method: String,
73-
pub payload: Vec<Vec<u8>>,
74-
}
75-
7679
pub struct Login5 {
7780
auth_token: Mutex<Option<Token>>,
7881
client_token_provider: ClientTokenProvider,
@@ -88,10 +91,14 @@ impl Login5 {
8891
/// used for it.
8992
///
9093
/// returns: Login5
91-
pub fn new(client_token_provider: Option<ClientTokenProvider>, proxy_url: Option<&str>) -> Self {
94+
pub fn new(
95+
client_token_provider: Option<ClientTokenProvider>,
96+
proxy_url: Option<&str>,
97+
) -> Self {
9298
Self {
9399
auth_token: Mutex::new(None),
94-
client_token_provider: client_token_provider.unwrap_or_else(|| ClientTokenProvider::new(proxy_url)),
100+
client_token_provider: client_token_provider
101+
.unwrap_or_else(|| ClientTokenProvider::new(proxy_url)),
95102
agent: default_ureq_agent_builder(proxy_url).build().into(),
96103
}
97104
}
@@ -111,7 +118,7 @@ impl Login5 {
111118
Ok(vec)
112119
}
113120

114-
fn request_new_access_token(&self, login: Login_method) -> Result<Token, Error> {
121+
fn request_new_token(&self, login: Login_method) -> Result<Token, Error> {
115122
let mut login_request = LoginRequest {
116123
client_info: MessageField::some(ClientInfo {
117124
client_id: String::from(CLIENT_ID),
@@ -128,40 +135,47 @@ impl Login5 {
128135
loop {
129136
count += 1;
130137

131-
let message = LoginResponse::parse_from_bytes(&response)?;
132-
if let Some(Response::Ok(ok)) = message.response {
133-
return Ok(Token {
134-
access_token: ok.access_token,
135-
expires_in: Duration::from_secs(ok.access_token_expires_in.try_into().unwrap_or(3600)),
136-
token_type: "Bearer".to_string(),
137-
scopes: vec![],
138-
timestamp: Instant::now(),
139-
});
140-
}
141-
142-
if message.has_error() {
143-
match message.error() {
144-
LoginError::TIMEOUT | LoginError::TOO_MANY_ATTEMPTS => {
138+
let mut message = LoginResponse::parse_from_bytes(&response)?;
139+
match message.response.take() {
140+
Some(Response::Ok(ok)) => {
141+
let expiry_secs = ok.access_token_expires_in.try_into().unwrap_or(3600);
142+
return Ok(Token {
143+
access_token: ok.access_token,
144+
expires_in: Duration::from_secs(expiry_secs),
145+
token_type: "Bearer".to_string(),
146+
scopes: vec![],
147+
timestamp: Instant::now(),
148+
});
149+
}
150+
Some(Response::Error(err)) => match err.enum_value() {
151+
Ok(LoginError::TIMEOUT) | Ok(LoginError::TOO_MANY_ATTEMPTS) => {
145152
log::debug!("Too many login5 requests... timeout!");
146153
thread::sleep(LOGIN_TIMEOUT)
147154
}
148-
others => {
155+
Ok(other) => {
149156
log::debug!("Login5 request failed!");
150-
151-
return Err(Login5Error::FaultyRequest(others).into());
157+
return Err(Login5Error::RequestDenied(other).into());
152158
}
159+
Err(other) => {
160+
log::warn!("Unknown login error: {}", other);
161+
}
162+
},
163+
Some(Response::Challenges(_)) => {
164+
// handles the challenges, and updates the login context with the response
165+
Self::handle_challenges(&mut login_request, message)?;
166+
}
167+
None => {
168+
return Err(Login5Error::MalformedResponse.into());
169+
}
170+
_ => {
171+
log::warn!("Unhandled login response");
153172
}
154-
}
155-
156-
if message.has_challenges() {
157-
// handles the challenges, and updates the login context with the response
158-
Self::handle_challenges(&mut login_request, message)?;
159173
}
160174

161175
if count < MAX_LOGIN_TRIES {
162176
response = self.request(&login_request)?;
163177
} else {
164-
return Err(Login5Error::RetriesFailed(MAX_LOGIN_TRIES).into());
178+
return Err(Login5Error::RetriesExceeded(MAX_LOGIN_TRIES).into());
165179
}
166180
}
167181
}
@@ -176,12 +190,14 @@ impl Login5 {
176190
challenges.challenges.len()
177191
);
178192

193+
if challenges.challenges.is_empty() {
194+
return Err(Login5Error::Challenge(ChallengeError::NoneFound).into());
195+
}
196+
179197
for challenge in &challenges.challenges {
180-
if challenge.has_code() {
181-
return Err(Login5Error::CodeChallenge.into());
182-
} else if !challenge.has_hashcash() {
183-
log::debug!("Challenge was empty, skipping...");
184-
continue;
198+
if challenge.has_code() || !challenge.has_hashcash() {
199+
// We only solve hashcash challenges.
200+
return Err(Login5Error::Challenge(ChallengeError::Unsupported).into());
185201
}
186202

187203
let hash_cash_challenge = challenge.hashcash();
@@ -193,7 +209,6 @@ impl Login5 {
193209
hash_cash_challenge.length,
194210
&mut suffix,
195211
)?;
196-
197212
let (seconds, nanos) = (duration.as_secs() as i64, duration.subsec_nanos() as i32);
198213
log::debug!("Solving hashcash took {seconds}s {nanos}ns");
199214

@@ -228,16 +243,16 @@ impl Login5 {
228243
/// the client token (see also `Login5::new(...)`). For example, if you previously generated
229244
/// stored credentials with an android client-id, they won't work within login5 using a desktop
230245
/// client-id.
231-
pub fn get_access_token(
232-
&self,
233-
session: &SessionService,
234-
) -> Result<Token, Error> {
246+
pub fn get_access_token(&self, session: &SessionService) -> Result<Token, Error> {
235247
let mut cur_token = self.auth_token.lock();
236248

237249
let login_creds = session.config.lock().as_ref().unwrap().login_creds.clone();
238250
let auth_data = login_creds.auth_data.clone();
239251
if auth_data.is_empty() {
240-
return Err(Login5Error::NoStoredCredentials.into());
252+
return Err(Login5Error::InvalidState(
253+
"Tried to acquire access token without stored credentials".to_string(),
254+
)
255+
.into());
241256
}
242257

243258
if let Some(auth_token) = &*cur_token {
@@ -249,7 +264,6 @@ impl Login5 {
249264
log::debug!("Auth token expired");
250265
}
251266

252-
253267
log::debug!("Requesting new auth token");
254268

255269
// Conversion from psst protocol structs to librespot protocol structs
@@ -259,7 +273,7 @@ impl Login5 {
259273
..Default::default()
260274
});
261275

262-
let new_token = self.request_new_access_token(method)?;
276+
let new_token = self.request_new_token(method)?;
263277

264278
log::debug!("Successfully requested new auth token");
265279

psst-core/src/util.rs

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use std::{io, io::SeekFrom, mem, time::Duration};
2-
use std::time::Instant;
1+
use crate::error::Error;
32
use byteorder::{BigEndian, ByteOrder};
43
use num_traits::{One, WrappingAdd};
54
use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer};
65
use sha1::{Digest, Sha1};
7-
use crate::error::Error;
6+
use std::time::Instant;
7+
use std::{io, io::SeekFrom, mem, time::Duration};
88

99
pub const NET_CONNECT_TIMEOUT: Duration = Duration::from_millis(8 * 1000);
1010

@@ -33,39 +33,38 @@ pub fn solve_hash_cash(
3333
length: i32,
3434
dst: &mut [u8],
3535
) -> Result<Duration, Error> {
36-
// after a certain number of seconds, the challenge expires
37-
const TIMEOUT: u64 = 5; // seconds
38-
let now = Instant::now();
36+
const TIMEOUT: Duration = Duration::from_secs(5);
37+
// SHA-1 produces a 20-byte hash, we check the trailing 8 bytes.
38+
const OFFSET_LEN: usize = 8;
39+
const CHECK_OFFSET: usize = 20 - OFFSET_LEN;
3940

40-
let md = Sha1::digest(ctx);
41-
42-
let mut counter: i64 = 0;
43-
let target: i64 = BigEndian::read_i64(&md[12..20]);
41+
let now = Instant::now();
42+
let initial_digest = Sha1::digest(ctx);
43+
let target = BigEndian::read_i64(&initial_digest[CHECK_OFFSET..]);
4444

45-
let suffix = loop {
46-
if now.elapsed().as_secs() >= TIMEOUT {
47-
return Err(Error::InvalidStateError(
48-
format!("{TIMEOUT} seconds expired").into(),
49-
));
50-
}
45+
let mut suffix = [0u8; 16];
46+
let mut counter = 0i64;
5147

52-
let suffix = [(target + counter).to_be_bytes(), counter.to_be_bytes()].concat();
48+
while now.elapsed() < TIMEOUT {
49+
suffix[..OFFSET_LEN].copy_from_slice(&target.wrapping_add(counter).to_be_bytes());
50+
suffix[OFFSET_LEN..].copy_from_slice(&counter.to_be_bytes());
5351

54-
let mut hasher = Sha1::new();
55-
hasher.update(prefix);
56-
hasher.update(&suffix);
57-
let md = hasher.finalize();
52+
let final_digest = Sha1::new()
53+
.chain_update(prefix)
54+
.chain_update(&suffix)
55+
.finalize();
5856

59-
if BigEndian::read_i64(&md[12..20]).trailing_zeros() >= (length as u32) {
60-
break suffix;
57+
if BigEndian::read_i64(&final_digest[CHECK_OFFSET..]).trailing_zeros() >= (length as u32) {
58+
dst.copy_from_slice(&suffix);
59+
return Ok(now.elapsed());
6160
}
6261

6362
counter += 1;
64-
};
65-
66-
dst.copy_from_slice(&suffix);
63+
}
6764

68-
Ok(now.elapsed())
65+
Err(Error::InvalidStateError(
66+
format!("{TIMEOUT:?} expired").into(),
67+
))
6968
}
7069

7170
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord, Default)]

0 commit comments

Comments
 (0)