Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port register, login and update services to the new ECS style #923

Merged
merged 25 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
54ce170
Move VPN routes to its own file
LDiazN Jan 15, 2025
f079cea
Refactor VPN Routes into its own file
LDiazN Jan 15, 2025
38f3b8a
Add probe_login_post endpoint copied and pasted from the probe_servic…
LDiazN Jan 15, 2025
481f9af
Removed useless example model
LDiazN Jan 15, 2025
61ae79e
Add register service
LDiazN Jan 15, 2025
769a0e7
Added informational comment; Formatting endpoint docs string
LDiazN Jan 16, 2025
d1a8137
Fix broken tests
LDiazN Jan 16, 2025
f3785be
Changed the path to match the same path as the old version
LDiazN Jan 16, 2025
c52c208
Fixing wrong schema in ProbeRegister model for /register
LDiazN Jan 16, 2025
be3fb93
Porting probe_services tests related to probe login
LDiazN Jan 16, 2025
6cd9426
Fixing broken test; handling case with empty username and password
LDiazN Jan 17, 2025
c69b37d
Removing unnecessary import
LDiazN Jan 17, 2025
ddc93da
Set username and password optional to deliver a 401 error when they'r…
LDiazN Jan 17, 2025
eb4cf8d
Add simple test for fake /update endpoint
LDiazN Jan 17, 2025
dc48d33
Fix spacing
LDiazN Jan 17, 2025
c5a05ea
Adding logging to update function
LDiazN Jan 17, 2025
b493e1f
Add client_id path argument to update to make it consistent with the …
LDiazN Jan 17, 2025
884cb70
Add usage of actual token into the test
LDiazN Jan 17, 2025
a76a17c
Add login token to update testing
LDiazN Jan 17, 2025
21a166c
Remove useless global log statement
LDiazN Jan 22, 2025
dcf48d5
Black reformat
LDiazN Jan 22, 2025
cc0342d
Add comment with warning about encryption key
LDiazN Jan 22, 2025
c164783
improve comment writing
LDiazN Jan 22, 2025
3679d4e
Add warning about the encryption key to ooniauth
LDiazN Jan 22, 2025
b7bb39f
Add warning about the encryption key to ooniauth v1
LDiazN Jan 22, 2025
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
10 changes: 10 additions & 0 deletions ooniapi/services/ooniauth/src/ooniauth/routers/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,16 @@ async def user_login(
settings: Settings = Depends(get_settings),
):
"""Auth Services: login using a registration/login link"""

# **IMPORTANT** You have to compute this token using a different key
# to the one used in ooniprobe service, because you could allow
# a login bypass attack if you don't.
#
# The token used in ooniprobe is generated regardless of any authentication,
# because it's a toy token to please old probes.
#
# We set this up in terraform

try:
dec = decode_jwt(
token=token, key=settings.jwt_encryption_key, audience="register"
Expand Down
10 changes: 10 additions & 0 deletions ooniapi/services/ooniauth/src/ooniauth/routers/v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,16 @@ async def create_user_session(
settings: Settings = Depends(get_settings),
):
"""Auth Services: login using a registration/login link"""

# **IMPORTANT** You have to compute this token using a different key
# to the one used in ooniprobe service, because you could allow
# a login bypass attack if you don't.
#
# The token used in ooniprobe is generated regardless of any authentication,
# because it's a toy token to please old probes.
#
# We set this up in terraform

if req and req.login_token:
user_session = get_user_session_from_login_token(
login_token=req.login_token,
Expand Down
12 changes: 6 additions & 6 deletions ooniapi/services/ooniprobe/src/ooniprobe/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
from prometheus_fastapi_instrumentator import Instrumentator

from . import models
from .routers import v2
from .routers.v2 import vpn
from .routers.v1 import probe_services

from .dependencies import get_postgresql_session
from .common.dependencies import get_settings
Expand Down Expand Up @@ -45,7 +46,8 @@ async def lifespan(app: FastAPI):
allow_headers=["*"],
)

app.include_router(v2.router, prefix="/api")
app.include_router(vpn.router, prefix="/api")
app.include_router(probe_services.router, prefix="/api")


@app.get("/version")
Expand Down Expand Up @@ -93,7 +95,5 @@ async def health(
@app.get("/")
async def root():
# TODO(art): fix this redirect by pointing health monitoring to /health
#return RedirectResponse("/docs")
return {
"msg": "hello from ooniprobe"
}
# return RedirectResponse("/docs")
return {"msg": "hello from ooniprobe"}
Empty file.
144 changes: 144 additions & 0 deletions ooniapi/services/ooniprobe/src/ooniprobe/routers/v1/probe_services.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import logging
from datetime import datetime, timezone, timedelta
import time
from typing import List

from fastapi import APIRouter, Depends, HTTPException, Response

from ...common.dependencies import get_settings
from ...common.routers import BaseModel
from ...common.auth import create_jwt, decode_jwt, jwt
from ...common.config import Settings
from ...common.utils import setnocacheresponse

router = APIRouter(prefix="/v1")

log = logging.getLogger(__name__)


class ProbeLogin(BaseModel):
# Allow None username and password
# to deliver informational 401 error when they're missing
username: str | None = None
# not actually used but necessary to be compliant with the old API schema
password: str | None = None


class ProbeLoginResponse(BaseModel):
token: str
expire: str


@router.post("/login", tags=["ooniprobe"], response_model=ProbeLoginResponse)
def probe_login_post(
probe_login: ProbeLogin,
response: Response,
settings: Settings = Depends(get_settings),
) -> ProbeLoginResponse:

if probe_login.username is None or probe_login.password is None:
raise HTTPException(status_code=401, detail="Missing credentials")

token = probe_login.username
# TODO: We have to find a way to explicitly log metrics with prometheus.
# We're currently using the instrumentator default metrics, like http response counts
# Maybe using the same exporter as the instrumentator?
try:
dec = decode_jwt(token, audience="probe_login", key=settings.jwt_encryption_key)
registration_time = dec["iat"]
log.info("probe login: successful")
# metrics.incr("probe_login_successful")
except jwt.exceptions.MissingRequiredClaimError:
log.info("probe login: invalid or missing claim")
# metrics.incr("probe_login_failed")
raise HTTPException(status_code=401, detail="Invalid credentials")
except jwt.exceptions.InvalidSignatureError:
log.info("probe login: invalid signature")
# metrics.incr("probe_login_failed")
raise HTTPException(status_code=401, detail="Invalid credentials")
except jwt.exceptions.DecodeError:
# Not a JWT token: treat it as a "legacy" login
# return jerror("Invalid or missing credentials", code=401)
log.info("probe login: legacy login successful")
# metrics.incr("probe_legacy_login_successful")
registration_time = None

exp = datetime.now(timezone.utc) + timedelta(days=7)
payload = {"registration_time": registration_time, "aud": "probe_token"}
token = create_jwt(payload, key=settings.jwt_encryption_key)
# expiration string used by the probe e.g. 2006-01-02T15:04:05Z
expire = exp.strftime("%Y-%m-%dT%H:%M:%SZ")
login_response = ProbeLoginResponse(token=token, expire=expire)
setnocacheresponse(response)

return login_response


class ProbeRegister(BaseModel):
# None of this values is actually used, but I add them
# to keep it compliant with the old api
password: str
platform: str
probe_asn: str
probe_cc: str
software_name: str
software_version: str
supported_tests: List[str]


class ProbeRegisterResponse(BaseModel):
client_id: str


@router.post("/register", tags=["ooniprobe"], response_model=ProbeRegisterResponse)
def probe_register_post(
probe_register: ProbeRegister,
response: Response,
settings: Settings = Depends(get_settings),
) -> ProbeRegisterResponse:
"""Probe Services: Register

Probes send a random string called password and receive a client_id

The client_id/password tuple is saved by the probe and long-lived

Note that most of the request body arguments are not actually
used but are kept here to use the same API as the old version

"""

# **IMPORTANT** You have to compute this token using a different key
# to the one used in ooniauth service, because you could allow
# a login bypass attack if you don't.
#
# Note that this token is generated regardless of any authentication,
# so if you use the same jwt_encryption_key for ooniauth, you give users
# an auth token for free
#
# We set this up in the terraform level

# client_id is a JWT token with "issued at" claim and
# "audience" claim. The "issued at" claim is rounded up.
issued_at = int(time.time())
payload = {"iat": issued_at, "aud": "probe_login"}
client_id = create_jwt(payload, key=settings.jwt_encryption_key)
Copy link
Member

Choose a reason for hiding this comment

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

We should take note of the fact the jwt_encryption_key is being shared for between probes and user authenticating via email in the auth service.

I don't think there is necessary a problem, given that the aud field is different, but we should make sure we don't forget to always check aud filed to avoid authentication bypasses.

Copy link
Member

Choose a reason for hiding this comment

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

Following a discussion with @LDiazN we decided to actually change this to a dedicated JWT token which we generate and inject via terraform: https://github.com/ooni/devops/blob/main/tf/environments/dev/main.tf#L187

log.info("register successful")

register_response = ProbeRegisterResponse(client_id=client_id)
setnocacheresponse(response)

return register_response


class ProbeUpdate(BaseModel):
Copy link
Member

Choose a reason for hiding this comment

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

The ProbeUpdate request contains some useful metadata that we could use for the purpose of the anonymous credential anomaly detection to establish a baseline for comparing the probe_cc and probe_asn inconsistencies (see: https://github.com/ooni/orchestra/blob/master/registry/registry/handler/registry.go#L25).

Basically what I'm thinking of is that, if we assume that the update requests are not going to be tampered with as much as the measurement submit ones, we can use this to determine which inconsistencies are due to our geoip being off compared to the probe one, vs those which are due to the measurement metadata being inconsistent due to malice.

We can do parsing and logging of this as a follow up PR though.

pass


class ProbeUpdateResponse(BaseModel):
status: str


@router.put("/update/{client_id}", tags=["ooniprobe"])
def probe_update_post(probe_update: ProbeUpdate) -> ProbeUpdateResponse:
log.info("update successful")
return ProbeUpdateResponse(status="ok")
Empty file.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import datetime, timedelta, timezone, date
from datetime import datetime, timedelta, timezone
import random
from typing import Dict, List
import logging
Expand All @@ -7,22 +7,22 @@
from sqlalchemy.orm import Session
from fastapi import APIRouter, Depends, HTTPException

from .. import models
from ... import models

from ..utils import (
from ...utils import (
fetch_openvpn_config,
fetch_openvpn_endpoints,
format_endpoint,
upsert_endpoints,
)
from ..common.routers import BaseModel
from ..common.dependencies import get_settings
from ..dependencies import get_postgresql_session
from ...common.routers import BaseModel
from ...common.dependencies import get_settings
from ...dependencies import get_postgresql_session

router = APIRouter(prefix="/v2")

log = logging.getLogger(__name__)

router = APIRouter()
log = logging.getLogger(__name__)


class VPNConfig(BaseModel):
Expand Down Expand Up @@ -113,7 +113,7 @@ def get_or_update_riseupvpn(
raise HTTPException(status_code=500, detail="error updating provider")


@router.get("/v2/ooniprobe/vpn-config/{provider_name}", tags=["ooniprobe"])
@router.get("/ooniprobe/vpn-config/{provider_name}", tags=["ooniprobe"])
def get_vpn_config(
provider_name: str,
db=Depends(get_postgresql_session),
Expand Down
49 changes: 31 additions & 18 deletions ooniapi/services/ooniprobe/src/ooniprobe/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

Insert VPN credentials into database.
"""

import base64
from datetime import datetime, timezone
import itertools
Expand All @@ -27,11 +28,13 @@ class OpenVPNConfig(TypedDict):
cert: str
key: str


class OpenVPNEndpoint(TypedDict):
address: str
protocol: str
transport: str


def fetch_riseup_ca() -> str:
r = httpx.get(RISEUP_CA_URL)
r.raise_for_status()
Expand All @@ -50,6 +53,7 @@ def fetch_openvpn_config() -> OpenVPNConfig:
key, cert = pem.parse(pem_cert)
return OpenVPNConfig(ca=ca, cert=cert.as_text(), key=key.as_text())


def fetch_openvpn_endpoints() -> List[OpenVPNEndpoint]:
endpoints = []

Expand All @@ -59,38 +63,47 @@ def fetch_openvpn_endpoints() -> List[OpenVPNEndpoint]:
for ep in j["gateways"]:
ip = ep["ip_address"]
# TODO(art): do we want to store this metadata somewhere?
#location = ep["location"]
#hostname = ep["host"]
# location = ep["location"]
# hostname = ep["host"]
for t in ep["capabilities"]["transport"]:
if t["type"] != "openvpn":
continue
for transport, port in itertools.product(t["protocols"], t["ports"]):
endpoints.append(OpenVPNEndpoint(
address=f"{ip}:{port}",
protocol="openvpn",
transport=transport
))
endpoints.append(
OpenVPNEndpoint(
address=f"{ip}:{port}", protocol="openvpn", transport=transport
)
)
return endpoints


def format_endpoint(provider_name: str, ep: OONIProbeVPNProviderEndpoint) -> str:
return f"{ep.protocol}://{provider_name}.corp/?address={ep.address}&transport={ep.transport}"

def upsert_endpoints(db: Session, new_endpoints: List[OpenVPNEndpoint], provider: OONIProbeVPNProvider):
new_endpoints_map = {f'{ep["address"]}-{ep["protocol"]}-{ep["transport"]}': ep for ep in new_endpoints}

def upsert_endpoints(
db: Session, new_endpoints: List[OpenVPNEndpoint], provider: OONIProbeVPNProvider
):
new_endpoints_map = {
f'{ep["address"]}-{ep["protocol"]}-{ep["transport"]}': ep
for ep in new_endpoints
}
for endpoint in provider.endpoints:
key = f'{endpoint.address}-{endpoint.protocol}-{endpoint.transport}'
key = f"{endpoint.address}-{endpoint.protocol}-{endpoint.transport}"
if key in new_endpoints_map:
endpoint.date_updated = datetime.now(timezone.utc)
new_endpoints_map.pop(key)
else:
db.delete(endpoint)

for ep in new_endpoints_map.values():
db.add(OONIProbeVPNProviderEndpoint(
date_created=datetime.now(timezone.utc),
date_updated=datetime.now(timezone.utc),
protocol=ep["protocol"],
address=ep["address"],
transport=ep["transport"],
provider=provider
))
db.add(
OONIProbeVPNProviderEndpoint(
date_created=datetime.now(timezone.utc),
date_updated=datetime.now(timezone.utc),
protocol=ep["protocol"],
address=ep["address"],
transport=ep["transport"],
provider=provider,
)
)
10 changes: 9 additions & 1 deletion ooniapi/services/ooniprobe/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,21 @@ def client_with_bad_settings():
yield client


JWT_ENCRYPTION_KEY = "super_secure"


@pytest.fixture
def client(alembic_migration):
app.dependency_overrides[get_settings] = make_override_get_settings(
postgresql_url=alembic_migration,
jwt_encryption_key="super_secure",
jwt_encryption_key=JWT_ENCRYPTION_KEY,
prometheus_metrics_password="super_secure",
)

client = TestClient(app)
yield client


@pytest.fixture
def jwt_encryption_key():
return JWT_ENCRYPTION_KEY
Loading
Loading