Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# v154.0 (In progress)

## ✨ What's Changed ✨

### Firefox Accounts

- Session-token authenticated requests to the FxA auth-server now use the typed-Bearer token scheme (`Authorization: Bearer fxs_<token_id>`) instead of Hawk. This is an internal change with no consumer-facing API impact; production routes accept both schemes. See the [authentication schemes reference](https://mozilla.github.io/ecosystem-platform/reference/authentication-schemes). ([#PRNUM](https://github.com/mozilla/application-services/pull/PRNUM))

[Full Changelog](In progress)

### Glean
Expand Down
2 changes: 1 addition & 1 deletion components/fxa-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ sync15 = { path = "../sync15", default-features=false, features=["crypto"] }
url = "2.2"
viaduct = { path = "../viaduct" }
jwcrypto = { path = "../support/jwcrypto" }
rc_crypto = { path = "../support/rc_crypto", features = ["ece", "hawk"] }
rc_crypto = { path = "../support/rc_crypto", features = ["ece"] }
error-support = { path = "../support/error" }
thiserror = "2"
anyhow = "1.0"
Expand Down
4 changes: 0 additions & 4 deletions components/fxa-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use error_support::{ErrorHandling, GetErrorHandling};
use rc_crypto::hawk;
use std::string;

/// Public error type thrown by many [`FirefoxAccount`] operations.
Expand Down Expand Up @@ -190,9 +189,6 @@ pub enum Error {
#[error("Sync15 error: {0}")]
SyncError(#[from] sync15::Error),

#[error("HAWK error: {0}")]
HawkError(#[from] hawk::Error),

#[error("Integer conversion error: {0}")]
IntegerConversionError(#[from] std::num::TryFromIntError),

Expand Down
158 changes: 103 additions & 55 deletions components/fxa-client/src/internal/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ use super::{config::Config, util};
use crate::{trace, Error, Result};
use error_support::breadcrumb;
use parking_lot::Mutex;
use rc_crypto::{
digest,
hawk::{Credentials, Key, PayloadHasher, RequestBuilder, SHA256},
hkdf, hmac,
};
use rc_crypto::{digest, hkdf, hmac};
use serde_derive::{Deserialize, Serialize};
use serde_json::json;
use std::{
Expand All @@ -28,8 +24,12 @@ use sync15::DeviceType;
use url::Url;
use viaduct::{header_names, status_codes, Method, Request, Response};

const HAWK_HKDF_SALT: [u8; 32] = [0b0; 32];
const HAWK_KEY_LENGTH: usize = 32;
const AUTH_HKDF_SALT: [u8; 32] = [0b0; 32];
const AUTH_KEY_LENGTH: usize = 32;
// Auth-server typed-Bearer prefix for sessionToken (matches KIND_PREFIXES in
// the auth-server's bearer-fxa-token scheme). See:
// https://mozilla.github.io/ecosystem-platform/reference/authentication-schemes
const BEARER_PREFIX_SESSION_TOKEN: &str = "fxs";
const RETRY_AFTER_DEFAULT_SECONDS: u64 = 10;
// Devices older than this many days will not appear in the devices list
const DEVICES_FILTER_DAYS: u64 = 21;
Expand Down Expand Up @@ -237,14 +237,14 @@ impl FxAClient for Client {
scopes: &[&str],
) -> Result<OAuthTokenResponse> {
let url = config.token_endpoint()?;
let key = derive_auth_key_from_session_token(session_token)?;
let auth = derive_bearer_token_from_session_token(session_token)?;
let body = json!({
"client_id": config.client_id,
"scope": scopes.join(" "),
"grant_type": "fxa-credentials",
"access_type": "offline",
});
let request = HawkRequestBuilder::new(Method::Post, url, &key)
let request = BearerRequestBuilder::new(Method::Post, url, &auth)
.body(body)
.build()?;
Ok(self.make_request(request)?.json()?)
Expand Down Expand Up @@ -279,9 +279,9 @@ impl FxAClient for Client {
"grant_type": "fxa-credentials",
"scope": scopes.join(" ")
});
let key = derive_auth_key_from_session_token(session_token)?;
let auth = derive_bearer_token_from_session_token(session_token)?;
let url = config.token_endpoint()?;
let request = HawkRequestBuilder::new(Method::Post, url, &key)
let request = BearerRequestBuilder::new(Method::Post, url, &auth)
.body(parameters)
.build()?;
self.make_request(request)?.json().map_err(Into::into)
Expand All @@ -308,9 +308,9 @@ impl FxAClient for Client {
auth_params: AuthorizationRequestParameters,
) -> Result<OAuthAuthResponse> {
let parameters = serde_json::to_value(auth_params)?;
let key = derive_auth_key_from_session_token(session_token)?;
let auth = derive_bearer_token_from_session_token(session_token)?;
let url = config.auth_url_path("v1/oauth/authorization")?;
let request = HawkRequestBuilder::new(Method::Post, url, &key)
let request = BearerRequestBuilder::new(Method::Post, url, &auth)
.body(parameters)
.build()?;

Expand All @@ -336,11 +336,11 @@ impl FxAClient for Client {
session_token: &str,
) -> Result<DuplicateTokenResponse> {
let url = config.auth_url_path("v1/session/duplicate")?;
let key = derive_auth_key_from_session_token(session_token)?;
let auth = derive_bearer_token_from_session_token(session_token)?;
let duplicate_body = json!({
"reason": "migration"
});
let request = HawkRequestBuilder::new(Method::Post, url, &key)
let request = BearerRequestBuilder::new(Method::Post, url, &auth)
.body(duplicate_body)
.build()?;

Expand Down Expand Up @@ -449,8 +449,8 @@ impl FxAClient for Client {
session_token: &str,
) -> Result<Vec<GetAttachedClientResponse>> {
let url = config.auth_url_path("v1/account/attached_clients")?;
let key = derive_auth_key_from_session_token(session_token)?;
let request = HawkRequestBuilder::new(Method::Get, url, &key).build()?;
let auth = derive_bearer_token_from_session_token(session_token)?;
let request = BearerRequestBuilder::new(Method::Get, url, &auth).build()?;
Ok(self.make_request(request)?.json()?)
}

Expand All @@ -466,8 +466,8 @@ impl FxAClient for Client {
"scope": scope,
});
let url = config.auth_url_path("v1/account/scoped-key-data")?;
let key = derive_auth_key_from_session_token(session_token)?;
let request = HawkRequestBuilder::new(Method::Post, url, &key)
let auth = derive_bearer_token_from_session_token(session_token)?;
let request = BearerRequestBuilder::new(Method::Post, url, &auth)
.body(body)
.build()?;
self.make_request(request)?.json().map_err(|e| e.into())
Expand Down Expand Up @@ -518,8 +518,8 @@ impl Client {
) -> Result<OAuthTokenResponse> {
let url = config.token_endpoint()?;
if let Some(session_token) = session_token {
let key = derive_auth_key_from_session_token(session_token)?;
let request = HawkRequestBuilder::new(Method::Post, url, &key)
let auth = derive_bearer_token_from_session_token(session_token)?;
let request = BearerRequestBuilder::new(Method::Post, url, &auth)
.body(body)
.build()?;

Expand Down Expand Up @@ -613,13 +613,21 @@ fn kw(name: &str) -> Vec<u8> {
.to_vec()
}

pub fn derive_auth_key_from_session_token(session_token: &str) -> Result<Vec<u8>> {
/// Derive the typed-Bearer `Authorization` header value for a sessionToken.
///
/// Returns the complete `Bearer fxs_<token_id>` value the auth-server expects,
/// where `token_id` is the hex-encoded first 32 bytes of the HKDF output (the
/// same value Hawk used as `id=`).
pub fn derive_bearer_token_from_session_token(session_token: &str) -> Result<String> {
let session_token_bytes = hex::decode(session_token)?;
let context_info = kw("sessionToken");
let salt = hmac::SigningKey::new(&digest::SHA256, &HAWK_HKDF_SALT);
let mut out = vec![0u8; HAWK_KEY_LENGTH * 2];
let salt = hmac::SigningKey::new(&digest::SHA256, &AUTH_HKDF_SALT);
let mut out = vec![0u8; AUTH_KEY_LENGTH * 2];
hkdf::extract_and_expand(&salt, &session_token_bytes, &context_info, &mut out)?;
Ok(out)
let token_id = hex::encode(&out[0..AUTH_KEY_LENGTH]);
Ok(bearer_token(&format!(
"{BEARER_PREFIX_SESSION_TOKEN}_{token_id}"
)))
}

#[derive(Serialize, Deserialize)]
Expand All @@ -633,21 +641,20 @@ pub struct AuthorizationRequestParameters {
pub keys_jwe: Option<String>,
}

struct HawkRequestBuilder<'a> {
struct BearerRequestBuilder<'a> {
url: Url,
method: Method,
body: Option<String>,
hkdf_sha256_key: &'a [u8],
auth_header: &'a str,
}

impl<'a> HawkRequestBuilder<'a> {
pub fn new(method: Method, url: Url, hkdf_sha256_key: &'a [u8]) -> Self {
rc_crypto::ensure_initialized();
HawkRequestBuilder {
impl<'a> BearerRequestBuilder<'a> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afact, every user of this passes BEARER_PREFIX_SESSION_TOKEN except for one test. Is there a future where this might be different? And every single user of the builder is calling derive_auth_key_from_session_token(). The few callers who don't use a session token then end up calling bearer_token() and manually setting up the header themselves.

I wonder if maybe we should replace derive_auth_key_from_session_token() with, say, derive_bearer_token_from_session_token(), which returns the complete header with the prefix already applied. The existing key param just gets renamed as it is now the full bearer token ready to use as-is You could also imagine a derive_bearer_token_from_refresh_token() and those requests could maybe start using this constructor too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to derive_bearer_token_from_session_token 👍🏽, I didn't make any changes to refresh token since this pr is just scoped to session tokens though.

pub fn new(method: Method, url: Url, auth_header: &'a str) -> Self {
BearerRequestBuilder {
url,
method,
body: None,
hkdf_sha256_key,
auth_header,
}
}

Expand All @@ -658,30 +665,9 @@ impl<'a> HawkRequestBuilder<'a> {
self
}

fn make_hawk_header(&self) -> Result<String> {
// Make sure we de-allocate the hash after hawk_request_builder.
let hash;
let method = format!("{}", self.method);
let mut hawk_request_builder = RequestBuilder::from_url(method.as_str(), &self.url)?;
if let Some(ref body) = self.body {
hash = PayloadHasher::hash("application/json", SHA256, body)?;
hawk_request_builder = hawk_request_builder.hash(&hash[..]);
}
let hawk_request = hawk_request_builder.request();
let token_id = hex::encode(&self.hkdf_sha256_key[0..HAWK_KEY_LENGTH]);
let hmac_key = &self.hkdf_sha256_key[HAWK_KEY_LENGTH..(2 * HAWK_KEY_LENGTH)];
let hawk_credentials = Credentials {
id: token_id,
key: Key::new(hmac_key, SHA256)?,
};
let header = hawk_request.make_header(&hawk_credentials)?;
Ok(format!("Hawk {}", header))
}

pub fn build(self) -> Result<Request> {
let hawk_header = self.make_hawk_header()?;
let mut request =
Request::new(self.method, self.url).header(header_names::AUTHORIZATION, hawk_header)?;
let mut request = Request::new(self.method, self.url)
.header(header_names::AUTHORIZATION, self.auth_header)?;
if let Some(body) = self.body {
request = request
.header(header_names::CONTENT_TYPE, "application/json")?
Expand Down Expand Up @@ -1021,6 +1007,68 @@ mod tests {
use super::*;
use mockito::mock;

const TEST_AUTH_HEADER: &str = "Bearer fxs_deadbeef";

#[test]
fn builder_sets_authorization_header() {
let req = BearerRequestBuilder::new(
Method::Get,
Url::parse("https://example.com/v1/account/attached_clients").unwrap(),
TEST_AUTH_HEADER,
)
.build()
.unwrap();
assert_eq!(
req.headers.get(header_names::AUTHORIZATION),
Some(TEST_AUTH_HEADER)
);
}

#[test]
fn builder_with_body_sets_content_type() {
let req = BearerRequestBuilder::new(
Method::Post,
Url::parse("https://example.com/v1/session/duplicate").unwrap(),
TEST_AUTH_HEADER,
)
.body(json!({"a": "b"}))
.build()
.unwrap();
assert_eq!(
req.headers.get(header_names::CONTENT_TYPE),
Some("application/json")
);
}

#[test]
fn builder_without_body_omits_content_type() {
let req = BearerRequestBuilder::new(
Method::Get,
Url::parse("https://example.com/v1/account/attached_clients").unwrap(),
TEST_AUTH_HEADER,
)
.build()
.unwrap();
assert_eq!(req.headers.get(header_names::CONTENT_TYPE), None);
}

// Backward-compat guard for already-logged-in users: a session token issued
// under the old (Hawk) client is unchanged by this migration, so it must run
// through the same HKDF derivation and produce the same token id the server
// already extracts. The id is the first 32 bytes of the HKDF output, the exact
// value Hawk sent as `id=`, now carried as `Bearer fxs_<token_id>`. The pinned
// value below catches any derivation drift that would break existing sessions.
#[test]
fn derive_bearer_token_from_existing_session_token() {
nss_as::ensure_initialized();
let session_token = "0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f";
let auth = derive_bearer_token_from_session_token(session_token).unwrap();
assert_eq!(
auth,
"Bearer fxs_923ee512d6153af6796ecb3aaddb1e8a48a713ef79cf0dbcfcc124456fd3ed6a"
);
}

#[test]
#[allow(non_snake_case)]
fn check_OAauthTokenRequest_serialization() {
Expand Down
13 changes: 13 additions & 0 deletions examples/fxa-client/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ enum Command {
#[clap(long = "scope", required = true)]
scopes: Vec<String>,
},
/// List the clients attached to the account (uses session-token auth).
AttachedClients,
Disconnect,
}

Expand Down Expand Up @@ -117,6 +119,17 @@ fn main() -> Result<()> {
let tok = account.get_access_token(&scope, !ignore_cache)?;
println!("Success: {tok:?}");
}
Command::AttachedClients => {
for client in account.get_attached_clients()? {
println!(
"{} (client_id: {}, current: {}, last access: {:?})",
client.name.as_deref().unwrap_or("<unnamed>"),
client.client_id.as_deref().unwrap_or("<none>"),
client.is_current_session,
client.last_access_time,
);
}
}
Command::Disconnect => {
account.disconnect();
}
Expand Down
Loading