Skip to content

Commit

Permalink
feat: Puts pyo3 behind feature flag and derives tokens directly in Ru…
Browse files Browse the repository at this point in the history
…st (#1513)

* Removes pyo3 and derives tokens directly in Rust

* Adds tests for JWT verifying

* Adds tests for token generation

* Adds metrics for oauth verify error cases

* Updates jsonwebtoken to not include default features (including pem loading)

* Adds context and logs errors during oauth verify

* Uses ring for cryptographic rng

* Adds back python impl under feature flag

* Uses one cached http client for reqwest
  • Loading branch information
Tarik Eshaq authored Feb 12, 2024
1 parent d544a0e commit 1b11684
Show file tree
Hide file tree
Showing 29 changed files with 1,321 additions and 295 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ commands:
- run:
name: Rust Clippy MySQL
command: |
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/mysql -- -D warnings
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/mysql --features=py_verifier -- -D warnings
rust-clippy-spanner:
steps:
- run:
name: Rust Clippy Spanner
command: |
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/spanner -- -D warnings
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/spanner --features=py_verifier -- -D warnings
cargo-build:
steps:
- run:
Expand Down
26 changes: 26 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ docopt = "1.1"
env_logger = "0.10"
futures = { version = "0.3", features = ["compat"] }
hex = "0.4"
hkdf = "0.12"
hmac = "0.12"
http = "0.2"
jsonwebtoken = { version = "9.2", default-features = false }
lazy_static = "1.4"
protobuf = "=2.25.2" # pin to 2.25.2 to prevent side updating
rand = "0.8"
Expand All @@ -62,6 +65,7 @@ slog-scope = "4.3"
slog-stdlog = "4.1"
slog-term = "2.6"
tokio = "1"
thiserror = "1.0.26"

[profile.release]
# Enables line numbers in Sentry reporting
Expand Down
7 changes: 4 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ RUN \
apt-get -q install -y --no-install-recommends libmysqlclient-dev cmake

COPY --from=planner /app/recipe.json recipe.json
RUN cargo chef cook --release --no-default-features --features=syncstorage-db/$DATABASE_BACKEND --recipe-path recipe.json
RUN cargo chef cook --release --no-default-features --features=syncstorage-db/$DATABASE_BACKEND --features=py_verifier --recipe-path recipe.json

FROM chef as builder
ARG DATABASE_BACKEND=spanner
Expand All @@ -46,7 +46,7 @@ ENV PATH=$PATH:/root/.cargo/bin
RUN \
cargo --version && \
rustc --version && \
cargo install --path ./syncserver --no-default-features --features=syncstorage-db/$DATABASE_BACKEND --locked --root /app && \
cargo install --path ./syncserver --no-default-features --features=syncstorage-db/$DATABASE_BACKEND --features=py_verifier --locked --root /app && \
if [ "$DATABASE_BACKEND" = "spanner" ] ; then cargo install --path ./syncstorage-spanner --locked --root /app --bin purge_ttl ; fi

FROM docker.io/library/debian:bullseye-slim
Expand All @@ -56,11 +56,12 @@ COPY --from=builder /app/requirements.txt /app
# have to set this env var to prevent the cryptography package from building
# with Rust. See this link for more information:
# https://pythonshowcase.com/question/problem-installing-cryptography-on-raspberry-pi
ENV CRYPTOGRAPHY_DONT_BUILD_RUST=1

RUN \
apt-get -q update && apt-get -qy install wget

ENV CRYPTOGRAPHY_DONT_BUILD_RUST=1

RUN \
groupadd --gid 10001 app && \
useradd --uid 10001 --gid 10001 --home /app --create-home app && \
Expand Down
11 changes: 6 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ PYTHON_SITE_PACKGES = $(shell $(SRC_ROOT)/venv/bin/python -c "from distutils.sys

clippy_mysql:
# Matches what's run in circleci
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/mysql -- -D warnings
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/mysql --features=py_verifier -- -D warnings

clippy_spanner:
# Matches what's run in circleci
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/spanner -- -D warnings
cargo clippy --workspace --all-targets --no-default-features --features=syncstorage-db/spanner --features=py_verifier -- -D warnings

clean:
cargo clean
Expand Down Expand Up @@ -47,14 +47,15 @@ python:
python3 -m venv venv
venv/bin/python -m pip install -r requirements.txt


run_mysql: python
PATH="./venv/bin:$(PATH)" \
# See https://github.com/PyO3/pyo3/issues/1741 for discussion re: why we need to set the
# below env var
PYTHONPATH=$(PYTHON_SITE_PACKGES) \
RUST_LOG=debug \
RUST_LOG=debug \
RUST_BACKTRACE=full \
cargo run --no-default-features --features=syncstorage-db/mysql -- --config config/local.toml
cargo run --no-default-features --features=syncstorage-db/mysql --features=py_verifier -- --config config/local.toml

run_spanner: python
GOOGLE_APPLICATION_CREDENTIALS=$(PATH_TO_SYNC_SPANNER_KEYS) \
Expand All @@ -65,7 +66,7 @@ run_spanner: python
PATH="./venv/bin:$(PATH)" \
RUST_LOG=debug \
RUST_BACKTRACE=full \
cargo run --no-default-features --features=syncstorage-db/spanner -- --config config/local.toml
cargo run --no-default-features --features=syncstorage-db/spanner --features=py_verifier -- --config config/local.toml

test:
SYNC_SYNCSTORAGE__DATABASE_URL=mysql://sample_user:sample_password@localhost/syncstorage_rs \
Expand Down
2 changes: 1 addition & 1 deletion syncserver-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ serde_json.workspace = true
slog.workspace = true
slog-scope.workspace = true
actix-web.workspace = true
hkdf.workspace = true

hkdf = "0.12"
2 changes: 1 addition & 1 deletion syncserver-db-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ edition.workspace=true
backtrace.workspace=true
futures.workspace=true
http.workspace=true
thiserror.workspace=true

deadpool = { git = "https://github.com/mozilla-services/deadpool", tag = "deadpool-v0.7.0" }
diesel = { version = "1.4", features = ["mysql", "r2d2"] }
diesel_migrations = { version = "1.4.0", features = ["mysql"] }
syncserver-common = { path = "../syncserver-common" }
thiserror = "1.0.26"
9 changes: 5 additions & 4 deletions syncserver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ slog-mozlog-json.workspace = true
slog-scope.workspace = true
slog-stdlog.workspace = true
slog-term.workspace = true
hmac.workspace = true
thiserror.workspace = true

actix-http = "3"
actix-rt = "2"
Expand All @@ -40,7 +42,6 @@ async-trait = "0.1.40"
dyn-clone = "1.0.4"
hostname = "0.3.1"
hawk = "5.0"
hmac = "0.12"
mime = "0.3"
reqwest = { workspace = true, features = [
"json",
Expand All @@ -53,8 +54,7 @@ syncserver-settings = { path = "../syncserver-settings" }
syncstorage-db = { path = "../syncstorage-db" }
syncstorage-settings = { path = "../syncstorage-settings" }
time = "^0.3"
thiserror = "1.0.26"
tokenserver-auth = { path = "../tokenserver-auth" }
tokenserver-auth = { path = "../tokenserver-auth", default-features = false}
tokenserver-common = { path = "../tokenserver-common" }
tokenserver-db = { path = "../tokenserver-db" }
tokenserver-settings = { path = "../tokenserver-settings" }
Expand All @@ -65,7 +65,8 @@ validator_derive = "0.16"
woothee = "0.13"

[features]
default = ["mysql"]
default = ["mysql", "py_verifier"]
no_auth = []
py_verifier = ["tokenserver-auth/py"]
mysql = ["syncstorage-db/mysql"]
spanner = ["syncstorage-db/spanner"]
5 changes: 3 additions & 2 deletions syncserver/src/tokenserver/extractors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,8 @@ impl FromRequest for AuthData {
let mut tags = HashMap::default();
tags.insert("token_type".to_owned(), "BrowserID".to_owned());
metrics.start_timer("token_verification", Some(tags));
let verify_output = state.browserid_verifier.verify(assertion).await?;
let verify_output =
state.browserid_verifier.verify(assertion, &metrics).await?;

// For requests using BrowserID, the client state is embedded in the
// X-Client-State header, and the generation and keys_changed_at are extracted
Expand Down Expand Up @@ -487,7 +488,7 @@ impl FromRequest for AuthData {
let mut tags = HashMap::default();
tags.insert("token_type".to_owned(), "OAuth".to_owned());
metrics.start_timer("token_verification", Some(tags));
let verify_output = state.oauth_verifier.verify(token).await?;
let verify_output = state.oauth_verifier.verify(token, &metrics).await?;

// For requests using OAuth, the keys_changed_at and client state are embedded
// in the X-KeyID header.
Expand Down
28 changes: 28 additions & 0 deletions syncserver/src/tokenserver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use serde::{
Serialize,
};
use syncserver_common::{BlockingThreadpool, Metrics};
#[cfg(not(feature = "py_verifier"))]
use tokenserver_auth::JWTVerifierImpl;
use tokenserver_auth::{browserid, oauth, VerifyToken};
use tokenserver_common::NodeType;
use tokenserver_db::{params, DbPool, TokenserverPool};
Expand Down Expand Up @@ -40,6 +42,32 @@ impl ServerState {
metrics: Arc<StatsdClient>,
blocking_threadpool: Arc<BlockingThreadpool>,
) -> Result<Self, ApiError> {
#[cfg(not(feature = "py_verifier"))]
let oauth_verifier = {
let mut jwk_verifiers: Vec<JWTVerifierImpl> = Vec::new();
if let Some(primary) = &settings.fxa_oauth_primary_jwk {
jwk_verifiers.push(
primary
.clone()
.try_into()
.expect("Invalid primary key, should either be fixed or removed"),
)
}
if let Some(secondary) = &settings.fxa_oauth_secondary_jwk {
jwk_verifiers.push(
secondary
.clone()
.try_into()
.expect("Invalid secondary key, should either be fixed or removed"),
);
}
Box::new(
oauth::Verifier::new(settings, jwk_verifiers)
.expect("failed to create Tokenserver OAuth verifier"),
)
};

#[cfg(feature = "py_verifier")]
let oauth_verifier = Box::new(
oauth::Verifier::new(settings, blocking_threadpool.clone())
.expect("failed to create Tokenserver OAuth verifier"),
Expand Down
2 changes: 1 addition & 1 deletion syncstorage-db-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ lazy_static.workspace=true
http.workspace=true
serde.workspace=true
serde_json.workspace=true
thiserror.workspace=true

async-trait = "0.1.40"
diesel = { version = "1.4", features = ["mysql", "r2d2"] }
diesel_migrations = { version = "1.4.0", features = ["mysql"] }
syncserver-common = { path = "../syncserver-common" }
syncserver-db-common = { path = "../syncserver-db-common" }
thiserror = "1.0.26"
2 changes: 1 addition & 1 deletion syncstorage-mysql/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ base64.workspace=true
futures.workspace=true
http.workspace=true
slog-scope.workspace=true
thiserror.workspace=true

async-trait = "0.1.40"
diesel = { version = "1.4", features = ["mysql", "r2d2"] }
Expand All @@ -20,7 +21,6 @@ syncserver-common = { path = "../syncserver-common" }
syncserver-db-common = { path = "../syncserver-db-common" }
syncstorage-db-common = { path = "../syncstorage-db-common" }
syncstorage-settings = { path = "../syncstorage-settings" }
thiserror = "1.0.26"
url = "2.1"

[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion syncstorage-spanner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ env_logger.workspace = true
futures.workspace = true
http.workspace = true
slog-scope.workspace = true
thiserror.workspace = true

async-trait = "0.1.40"
google-cloud-rust-raw = { version = "0.16.1", features = ["spanner"] }
Expand All @@ -30,7 +31,6 @@ syncserver-common = { path = "../syncserver-common" }
syncserver-db-common = { path = "../syncserver-db-common" }
syncstorage-db-common = { path = "../syncstorage-db-common" }
syncstorage-settings = { path = "../syncstorage-settings" }
thiserror = "1.0.26"
tokio = { workspace = true, features = [
"macros",
"sync",
Expand Down
17 changes: 16 additions & 1 deletion tokenserver-auth/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,30 @@ edition.workspace = true
futures.workspace = true
serde.workspace = true
serde_json.workspace = true
hex.workspace = true
hkdf.workspace = true
hmac.workspace = true
jsonwebtoken.workspace = true
base64.workspace = true
sha2.workspace = true
thiserror.workspace = true
slog-scope.workspace = true

async-trait = "0.1.40"
dyn-clone = "1.0.4"
pyo3 = { version = "0.20", features = ["auto-initialize"] }
reqwest = { workspace = true, features = ["json", "rustls-tls"] }
ring = "0.17"
syncserver-common = { path = "../syncserver-common" }
tokenserver-common = { path = "../tokenserver-common" }
tokenserver-settings = { path = "../tokenserver-settings" }
tokio = { workspace = true }
pyo3 = { version = "0.20", features = ["auto-initialize"], optional = true}


[dev-dependencies]
mockito = "0.30.0"
tokio = { workspace = true, features = ["macros"]}

[features]
default = ["py"]
py = ["pyo3"]
Loading

0 comments on commit 1b11684

Please sign in to comment.