Skip to content

Commit

Permalink
refactor: Remove Tokenserver support for per-node secrets (#1211)
Browse files Browse the repository at this point in the history
Closes #1208
  • Loading branch information
ethowitz authored Feb 4, 2022
1 parent aa93312 commit eac6b55
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 101 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ commands:
- run:
name: Core Python Checks
command: |
flake8 src/tokenserver
flake8 src/tokenserver/verify.py
flake8 tools/integration_tests
flake8 tools/tokenserver
rust-clippy:
Expand Down
24 changes: 2 additions & 22 deletions src/tokenserver/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ use super::db::models::Db;
use super::db::params::{GetNodeId, PostUser, PutUser, ReplaceUsers};
use super::error::TokenserverError;
use super::extractors::TokenserverRequest;
use super::support::{self, Tokenlib};
use super::support::Tokenlib;
use super::NodeType;
use crate::server::metrics::Metrics;
use crate::tokenserver::support::MakeTokenPlaintext;

#[derive(Debug, Serialize)]
Expand All @@ -32,31 +31,12 @@ pub struct TokenserverResult {
pub async fn get_tokenserver_result(
req: TokenserverRequest,
db: Box<dyn Db>,
mut metrics: Metrics,
) -> Result<HttpResponse, Error> {
let updates = update_user(&req, db).await?;

let (token, derived_secret) = {
// Get the plaintext that will be used to derive the token and secret to be returned to
// the client
let token_plaintext = get_token_plaintext(&req, &updates)?;

// Derive the node-specific secret that will be used to derive the token and secret to be
// returned to the client
let secrets = {
metrics.start_timer("tokenserver.node_secret_derivation", None);

support::derive_node_secrets(vec![&hex::encode(req.shared_secret)], &req.user.node)
.map_err(|_| {
error!("⚠️ Failed to derive node secret");

TokenserverError::internal_error()
})?
};

metrics.start_timer("tokenserver.token_creation", None);
// Get the token and secret
Tokenlib::get_token_and_derived_secret(token_plaintext, &secrets[secrets.len() - 1])?
Tokenlib::get_token_and_derived_secret(token_plaintext, &req.shared_secret)?
};

let result = TokenserverResult {
Expand Down
24 changes: 0 additions & 24 deletions src/tokenserver/secrets.py

This file was deleted.

31 changes: 0 additions & 31 deletions src/tokenserver/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,25 +76,6 @@ impl Tokenlib {
}
}

pub fn derive_node_secrets(secrets: Vec<&str>, node: &str) -> Result<Vec<String>, Error> {
const FILENAME: &str = "secrets.py";

Python::with_gil(|py| {
let code = include_str!("secrets.py");
let module = PyModule::from_code(py, code, FILENAME, FILENAME)?;

module
.getattr("derive_secrets")?
.call((secrets, node), None)
.map_err(|e| {
e.print_and_set_sys_last_vars(py);
e
})
.and_then(|x| x.extract())
})
.map_err(pyerr_to_actix_error)
}

#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
pub struct TokenData {
pub user: String,
Expand Down Expand Up @@ -270,16 +251,4 @@ mod tests {

assert_eq!(expected_claims, decoded_claims);
}

#[test]
fn test_derive_secret_success() {
let secrets = vec!["deadbeefdeadbeefdeadbeefdeadbeef"];
let node = "https://node";
let derived_secrets = derive_node_secrets(secrets, node).unwrap();

assert_eq!(
derived_secrets,
vec!["a227eb0deb5fb4fd8002166f555c9071".to_owned()]
);
}
}
27 changes: 4 additions & 23 deletions tools/integration_tests/tokenserver/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.
from base64 import urlsafe_b64decode
import binascii
import hashlib
import hmac
import json
import jwt
Expand All @@ -21,7 +19,6 @@
from fxa.oauth import Client as OAuthClient
from fxa.tests.utils import TestEmailAccount
from hashlib import sha256
from tokenlib import HKDF

from tokenserver.test_support import TestCase

Expand Down Expand Up @@ -124,19 +121,6 @@ def _fxa_metrics_hash(self, value):
hasher.update(value.encode('utf-8'))
return hasher.hexdigest()

def _derive_secret(self, master_secret):
info = "services.mozilla.com/mozsvc/v1/node_secret/%s" % self.NODE_URL
hkdf_params = {
"salt": None,
"info": info.encode("utf-8"),
"hashmod": hashlib.sha256,
}
size = len(master_secret) // 2
derived_secret = HKDF(master_secret.encode("utf-8"), size=size,
**hkdf_params)

return binascii.b2a_hex(derived_secret).decode()

def test_unauthorized_error_status(self):
# Totally busted auth -> generic error.
headers = {
Expand Down Expand Up @@ -202,19 +186,16 @@ def test_valid_request(self):
payload = raw[:-32]
signature = raw[-32:]
payload_dict = json.loads(payload.decode('utf-8'))

signing_secret = binascii.b2a_hex(
self.TOKEN_SIGNING_SECRET.encode("utf-8")).decode()
node_specific_secret = self._derive_secret(signing_secret)
signing_secret = self.TOKEN_SIGNING_SECRET
expected_token = tokenlib.make_token(payload_dict,
secret=node_specific_secret)
secret=signing_secret)
expected_signature = urlsafe_b64decode(expected_token)[-32:]
# Using the #compare_digest method here is not strictly necessary, as
# this is not a security-sensitive situation, but it's good practice
self.assertTrue(hmac.compare_digest(expected_signature, signature))
# Check that the given key is a secret derived from the hawk ID
expected_secret = tokenlib.get_derived_secret(
res.json['id'], secret=node_specific_secret)
expected_secret = tokenlib.get_derived_secret(res.json['id'],
secret=signing_secret)
self.assertEqual(res.json['key'], expected_secret)
# Check to make sure the remainder of the fields are valid
self.assertEqual(res.json['uid'], user['uid'])
Expand Down

0 comments on commit eac6b55

Please sign in to comment.