mirror of
https://github.com/fusion44/blitz_api.git
synced 2026-08-13 11:52:45 +02:00
fix(system): stop leaking passwords via change-password endpoint
The change-password endpoint accepted old_password/new_password as bare str parameters, i.e. query parameters, so the passwords ended up in access logs, proxy logs and browser history. Accept them in a ChangePasswordInput request body instead. Also mark the RaspiBlitz blitz.passwords.sh check/set invocations sensitive=True so the plaintext passwords are not written to the debug log, and guard against a missing password type (was an AttributeError -> 500). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
9912ff0bfa
commit
c4afca50e9
4 changed files with 67 additions and 18 deletions
|
|
@ -261,11 +261,11 @@ class RaspiBlitzSystem(SystemBase):
|
|||
|
||||
async def change_password(self, type: str, old_password: str, new_password: str):
|
||||
# check just allowed type values
|
||||
type = type.lower()
|
||||
if type not in ["a", "b", "c"]:
|
||||
if not type or type.lower() not in ["a", "b", "c"]:
|
||||
raise HTTPException(
|
||||
status.HTTP_400_BAD_REQUEST, detail=f"unknown password type: {type}"
|
||||
)
|
||||
type = type.lower()
|
||||
|
||||
# check password formatting
|
||||
if not password_valid(old_password):
|
||||
|
|
@ -279,7 +279,8 @@ class RaspiBlitzSystem(SystemBase):
|
|||
|
||||
# first check if old password is correct
|
||||
result = await exec_bash_command(
|
||||
f'/home/admin/config.scripts/blitz.passwords.sh check {type} "{old_password}"' # noqa: E501
|
||||
f'/home/admin/config.scripts/blitz.passwords.sh check {type} "{old_password}"', # noqa: E501
|
||||
sensitive=True,
|
||||
)
|
||||
data = {}
|
||||
match result:
|
||||
|
|
@ -300,7 +301,7 @@ class RaspiBlitzSystem(SystemBase):
|
|||
if type == "c":
|
||||
# will set password c of both lnd & core lightning if installed/activated
|
||||
script_call = f'/home/admin/config.scripts/blitz.passwords.sh set c "{old_password}" "{new_password}"' # noqa: E501
|
||||
result = await exec_bash_command(script_call)
|
||||
result = await exec_bash_command(script_call, sensitive=True)
|
||||
data = {}
|
||||
match result:
|
||||
case Ok(in_data):
|
||||
|
|
|
|||
|
|
@ -16,6 +16,13 @@ class LoginInput(BaseModel):
|
|||
] = None
|
||||
|
||||
|
||||
class ChangePasswordInput(BaseModel):
|
||||
old_password: constr(min_length=1)
|
||||
new_password: constr(min_length=1)
|
||||
# RaspiBlitz only: which password (a, b or c) to change
|
||||
type: Optional[str] = None
|
||||
|
||||
|
||||
class APIPlatform(str, Enum):
|
||||
RASPIBLITZ = "raspiblitz"
|
||||
NATIVE_PYTHON = "native_python"
|
||||
|
|
|
|||
|
|
@ -1,5 +1,3 @@
|
|||
from typing import Optional
|
||||
|
||||
from fastapi import APIRouter, HTTPException, Request, Response, status
|
||||
from fastapi.params import Depends, Query
|
||||
|
||||
|
|
@ -14,6 +12,7 @@ from app.system.docs import (
|
|||
get_hw_info_json,
|
||||
)
|
||||
from app.system.models import (
|
||||
ChangePasswordInput,
|
||||
ConnectionInfo,
|
||||
LoginInput,
|
||||
RawDebugLogData,
|
||||
|
|
@ -74,18 +73,10 @@ def refresh_token():
|
|||
response_description="if 200 OK - password change worked",
|
||||
dependencies=[Depends(JWTBearer())],
|
||||
)
|
||||
async def change_password_impl(
|
||||
old_password: str,
|
||||
new_password: str,
|
||||
type: Optional[str] = Query(
|
||||
None,
|
||||
description=(
|
||||
"ℹ️ Used in **RaspiBlitz only**. Password A, B or C. "
|
||||
'Must be one of `["a", "b", "c"]`'
|
||||
),
|
||||
),
|
||||
):
|
||||
return await change_password(type, old_password, new_password)
|
||||
async def change_password_impl(data: ChangePasswordInput):
|
||||
return await change_password(
|
||||
data.type, data.old_password, data.new_password
|
||||
)
|
||||
|
||||
|
||||
@router.get(
|
||||
|
|
|
|||
50
tests/test_change_password.py
Normal file
50
tests/test_change_password.py
Normal file
|
|
@ -0,0 +1,50 @@
|
|||
"""
|
||||
Regression tests for the change-password endpoint.
|
||||
|
||||
- Passwords were accepted as query parameters, so they leaked into access
|
||||
logs, proxy logs and browser history. They must travel in the request body.
|
||||
- The RaspiBlitz password scripts were called without sensitive=True, so the
|
||||
plaintext passwords were written to the debug log.
|
||||
"""
|
||||
|
||||
from app.api.models import ProcessResult
|
||||
from app.external.result_type.src.result.result import Ok
|
||||
|
||||
|
||||
def test_change_password_uses_request_body_not_query():
|
||||
from app.main import app
|
||||
|
||||
spec = app.openapi()
|
||||
path = next(p for p in spec["paths"] if p.endswith("change-password"))
|
||||
operation = spec["paths"][path]["post"]
|
||||
|
||||
param_names = {p["name"] for p in operation.get("parameters", [])}
|
||||
assert "old_password" not in param_names, "password must not be a query param"
|
||||
assert "new_password" not in param_names, "password must not be a query param"
|
||||
assert "requestBody" in operation, "passwords must be sent in the request body"
|
||||
|
||||
|
||||
async def test_raspiblitz_change_password_calls_are_sensitive(monkeypatch):
|
||||
from app.system.impl import raspiblitz as rb
|
||||
|
||||
# the shell-script existence check would exit(1) in the test env
|
||||
monkeypatch.setattr(
|
||||
rb.RaspiBlitzSystem, "_check_shell_scripts_status", lambda self: None
|
||||
)
|
||||
system = rb.RaspiBlitzSystem()
|
||||
calls = []
|
||||
|
||||
async def fake_exec(command, **kwargs):
|
||||
calls.append(kwargs)
|
||||
# 'check' -> correct=1; 'set' -> no error key
|
||||
return Ok(ProcessResult(0, "correct=1\n", ""))
|
||||
|
||||
monkeypatch.setattr(rb, "exec_bash_command", fake_exec)
|
||||
monkeypatch.setattr(rb, "password_valid", lambda p: True)
|
||||
|
||||
await system.change_password("a", "oldpass12", "newpass12")
|
||||
|
||||
assert len(calls) == 2, "expected a check call and a set call"
|
||||
assert all(c.get("sensitive") is True for c in calls), (
|
||||
"password script calls must be marked sensitive so they aren't logged"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue