protego-safe / backend Goto Github PK

• Admin może zobaczyć nowych pacjentów i oznaczyć ich jako zweryfikowany / niezweryfikowany
• Admin może zobaczyć jakich użytkowników napotkał pacjent, kiedy, jak często i jaka była siła sygnału spotkani
• Admin wybiera i oznacza wybranych użytkowników jako red

• w specs/api.md mamy opis w jaki sposób backend zwraca informacje o błędach (HTTP status code i messsage)
• message jest wszędzie w dwóch językach w zależności od parametru lang
• wszystkie funkcje API są poprawione zgodnie z powyższym

Po zainstalowaniu projektu i uruchomieniu go w GPC testy się nie ruchamiają.

python -m unittest

Ran 0 tests in 0.000s

OK

Is your feature request related to a problem? Please describe.
Czy zostanie wprowadzony Exposure Notifications Express, update na IOS już jest
Describe the solution you'd like
Wprowadzenie exposure notification express
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
b/d
Additional context
b/d

Uproszczone wersja rozwiązania dla #5
Zwracanie losowych 24x7 losowych beacond_ids . Rozwiązanie tymczasowe, bez zapisywania w bazie.

"Twój kod" wyświetla się nieprawidłowo.

Na razie logika tej funkcji niech będzie hardcoded.

Czesc,

zostawie wam tu mala notke na prosbe czlonkow zespolu z ktorym pracowalem nad prawie tym samym tematem. Ciut nas wyprzedziliscie wiec zamykamy temat, z ta jedna roznica ze..

Rozwazcie synchronizacje z danymi GISu ktory identyfikuje zarazonych na podstawie numeru telefonu. Mechanizm juz jest bo korzysta z niego Aplikacja Kwarantanna domowa.

Majac informacje na urzadzeniu o tym ze dany pacjent jest chory bedziecie w stanie policzyc i powiedziec innym uzytkownikom ze mogli byc narazeni na kontakt z chorym, oraz pomoc GISowi w sledzeniu epidemii co defakto chcielismy my zrobic.

Chetnie odpowiem na pytania jesli sie takowe pojawia i bedzie wola dzialan z waszej strony bo dosc mocno juz rozkminilismy temat jak to powinno dzialac.

Pozdrawiam

/register i /confirm_registration

Zapraszam do przeczytania ogłoszenia

Jeśli API byłoby dostępne, wysyłając odpowiedniego requesta na /register można komuś zrobić psikusa, gdyż nie ma możliwości walidacji numeru telefonu, który poda się w zgłoszeniu o rejestrację.

Rozważyć więc należy możliwość dodania pola adresu IP (wiem, kwestia prywatności..), z którego poszedł request rejestracji,
co da możliwość zastosowania jakiegoś throthlingu, a w razie czego blokowania/zgłoszenia dowcipnisia. Można też korzystać z unikalnego identyfikatora urządzenia/instalacji aplikacji, ale
to jest nieweryfikowalne i wiadomo, że to co leży na urządzeniu jest raczej "jawne" i do zmanipulowania poza aplikacją.

Będzie nowa bramka SMS. Jak tylko dowiem się jaka, dam znać.

Wywołania get i put w confirm_registration nie znajdują się wewnątrz transakcji. Oznacza to, że przy wielokrotnym wywołaniu funkcji dla tych samych parametrów może dojść do stworzenia dwóch lub więcej instancji użytkownika w datastore. Przykład:

Podczas rejestracji i zapewne nie tylko adres klienta pobierany jest tak:
ip = request.headers.get('X-Forwarded-For')

Jest to nieprawidłowe, gdyż, gdy w zapytaniu występuje już nagłowek X-Forwarded-For z jakakolwiek wartoscia np fake_ip - proxy googlowe po drodze doda kolejny adres do tego nagłóowka. Bedzie on wygladal tak:
fake_ip, client_ip
Wtedy wzięcie całego stringa do porównania spowoduje, że trywialnie łatwo bedzie można spoofować ten nagłowek - bez wysiłku zmiany prawdziwego adresu IP.
Jesli w naglowku jest lista adresow - trzeba brac ostatni (jeśli podczas testow w naglowku dostawaliscie jeden IP).

DOC: https://cloud.google.com/load-balancing/docs/https , https://tools.ietf.org/html/rfc7239

wg. specyfikacji https://github.com/ProteGO-app/specs/blob/master/specs/api.md#funkcja-send_encounters

Każde wywołanie /get_status nadpisuje w bazie danych dane użytkownika przesłane w żądaniu: platform, os_version, device_name, app_version, lang
Pole last_status_requested ustawiane jest na now()

@DariuszAniszewski ja popieram ten pomysł, brakuje tutaj jeszcze definicji modeli zgrupowanych w jednym miejscu lub w funkcjach. W planach jest aplikacja backoffice it tam bardzo by się to przydało.

Originally posted by @rafbara in #63 (comment)

Automatyczny skrypt który testuje wszystkie najważniejsze API czy działają poprawnie. Do uruchamiania po każdym wdrożeniu.

uuid's (nawet uuid4()) nie są odpowiednie do generowanie kluczy:
https://stackoverflow.com/questions/816882/how-to-predict-the-next-guid-from-a-given-guid?noredirect=1&lq=1

Używajmy secrets.token_hex(16) albo odpowiednika:
https://docs.python.org/3/library/secrets.html

Trzeba też zmienić w dokumentacji wszystkie uuid na string

To repozytorium zawiera kod starej wersji aplikacji (od której funkcjonalnie nowa wprawdzie nie różni się prawie niczym), ale nie jest to kod, który jest odpalony na właściwym serwerze produkcyjnym ProteGO Safe.

Repozytorium powinno zostać zaktualizowane o nową wersję kodu, tak jak stało się to z repozytoriami android i ios.

Jeżeli kod backendu i alogrytmy określajace stan zagrożenia nie mają być publiczne - powinno być to wprost napisane w https://github.com/ProteGO-Safe/specs a to repozytorium powinno zostać usunięte, bo teraz wprowadza tylko w błąd.

Zgodnie ze zmianam w tym PR: #52

Z tego co rozumiem, zakładamy, że ostatni człon w X-Forwarded-For jest wystarczająco precyzyjnie powiązany z człowiekiem, że warto go używać do karania spamerów.
(Zakładam, że wiemy to, bo sami ustawiamy ten nagłówek na krawędzi systemu by był równy IP z którego przychodzi request)
Następnie w oparciu o ten ip budujemy string używany jako klucz pod którym w Redisie trzymamy sobie ile requestów robiono do danej funkcji w danym okresie z danego ip:

    key = f"{function}:{period}:{ip}"

Wygląda na to, że dekoratora limit_request używamy w dość sensytywnej sądząc po nazwie funkcji send_encounters - dopiero zapoznaję się z projektem, więc mogę się mylić, ale nazwa brzmi jak coś czego się używa gdy się jest chorym i chce się przesłać na serwer listę spotkanych idików.

A zatem, jeśli ktoś zajrzy do Redisa i poszuka sobie kluczy zaczynających się od prefixu send_encounters:day:, to czy przypadkiem nie dostanie na tacy adresów IP osób, które są z dużym ppb są chore, a przynajmniej sądzą, że są, a przynajmniej korzystały z endpointu send_encounters, a przynajmniej jakieś IP które uważamy za dostatecznie precyzyjnie powiązane z takimi ludźmi?

Z tego co rozumiem, nie ma potrzeby by klucz key był czytelny dla ludzi czy ogólnie "odwracalny" - wystarczy tylko by istniała funkcja różnowartościowa z krotek (function,period,ip) w takie stringi.
Oczywiście liczba różnych function, różnych period a nawet różnych ip jest bardzo mała, więc zwyczajnie zahashowanie tego nie wystarczy, bo mogę sobie bruteforcem przeglądnąć wszystkie takie krotki i sprawdzić, które z nich po zahashowaniu są kluczami w Redisie.

Nie wiem ile "instancji" tego serwera mamy i jak długowieczne one są, ale może wystarczyłoby, żeby serwer przy starcie losował sobie jakiś salt, znany tylko jemu tj. trzymany w jakiejś pythonowej zmiennej, którego będzie używał przez cały czas aż padnie, by solić hashe?

Alternatywnie, może mógłby ów salt być jakoś wstrzykiwany z zewnątrz przez jakiś Env - to nieco mniej bezpieczne, ale zakładam, że jak ktoś ma dostęp do tej części serwera ORAZ do Redisa, to jest to sytuacja o oczko gorsza niż jeśli ma dostęp tylko do Redisa (czy jego dumpów, backupów, komunikacji, logów?), więc jest postęp.

Testy nie przechodzą jeśli z tego ip / nr były wcześniej requesty.

Napisanie testów funkcji

W pierwszej wersji zwraca zawsze orange

Ze względu na to, że numer telefonu będzie opcjonalny (ProteGO-Safe/specs#34 (comment)) a co za tym idzie będzie można swobodnie generować user_id, potrzebujemy zrobić rate-limiting na wszystkie API. Należy go też użyć przy zabezpieczeniu naszej rejestracji SMS.

Inspiracje:
https://cloud.google.com/community/tutorials/cloud-functions-rate-limiting
https://pypi.org/project/ratelimit/

Zapraszam do dyskusji.

https://github.com/ProteGO-Safe/backend/blob/master/README.md

Ze względu na to, że numer telefonu nie będzie obowiązkowy (ProteGO-Safe/specs#34 (comment)) użytkownicy mogą zarejestrować się bez nr telefonu.

Potrzebujemy albo nowego endpoint'u /register_no_msisdn albo istniejący endpoint /register nie powinien wymagać msisdn i od razu zwracać user_id. Jak lepiej?

Częścią tego issue jest też aktualizacja dokumentacji.

Proces rejestracji jest najbardziej newralgicznym elementem naszego systemu. Musimy zabezpieczyć się przed następującymi próbami nadużyć:

• ktoś spamuje różne numery telefonu naszymi kodami (i naraża nas na koszty)
• ktoś spamuje jeden numer telefonu naszymi kodami (i naprzykrza się)
• ktoś próbuję uniemożliwić komuś rejestrację wysyłając /register tuż po tym jak rejestrujący się rozpoczął rejestrację, tym samym zmieniając jego kod

Jak osiągnąć powyższe:

• jeśli z jakiegoś IP przychodzą żądania rejestracji, które nie kończą się powodzeniem, pozwalamy zrobić kolejne (wysłać SMS) po odczekaniu jakiegoś timeout'u
• nie wysyłamy SMSa na ten sam numer częściej niż 1 na minutę
• nie wysyłamy SMSa na ten sam numer częściej niż 4 na godzinę
• kod weryfikacyjny jest taki sam przez 10 minut. To znaczy, że jeśli ktoś w ciągu 10 minut jeszcze raz wywoła /register dla takiego samego numeru dostanie taki sam kod (!)
• kod weryfikacyjny jest ważny przez 10 minut. Użytkownik ma 10 minut na wpisanie poprawnego kodu
• zapisujemy każde wywołanie /register do tabeli Registrations. czyścimy starsze niż 24h
• zapisujemy do Registrations czy wysłaliśmy wiadomość SMS
• zapisujemy do Registrations czy zakończyło się powodzeniem
• limitujemy /verify_registration do 3 razy na godzinę dla tego samego numeru.

Zwracamy aplikacjom za ile czasu mogą ponowić zapytanie.

Kod do tego musi być super klarowny i wszystkie oczy na pokład.

Treści wiadomości:
pl: Twój kod dla NAZWA to: 123-456
en: Your NAZWA code is: 123-456

Źródła i inspiracje:
https://www.twilio.com/docs/verify/developer-best-practices
https://stackoverflow.com/questions/20839638/how-do-you-prevent-verification-code-attack-to-server

Obecnie wiele funkcji zawiera duplikacje kodu - walidacja standardowych parametrów, pobieranie/aktualizowanie użytkowników w Datastore itp.

Wynika to ze specyfiki Cloud Functions które muszą być niezależne od siebie oraz posiadać plik main.py w głównym katalogu z funkcją. Nie ma możliwości importowania z plików będących poza katalogiem z plikiem main.py.

Proponuję rozwiązanie polegające na utworzeniu pliku utils.py (przykładowo) w nadrzędnym folderze repozytorium. Plik ten zawierałby wspólne metody dla wszystkich funkcji. Plik byłby linkowany na poziomie deploymentu przez terraforma poprzez

1. utworzenie definicji pliku

data "local_file" "utils" {
  filename = "${path.module}/utils.py"
}

2. dodanie go przy tworzeniu pliku ZIP, np:

// START check_version
[...]
data "archive_file" "check_version" {
  type        = "zip"
  output_path = "${path.module}/files/check_version.zip"
  [...]
  source {
    content  = "${file("${data.local_file.utils.filename}")}"
    filename = "utils.py"
  }
}

W każdej funkcji wystarczy zaimportować metody z modułu zwykłym importem:
from utils import ...

Co ważne, lokalne środowisko nie musi być zmieniane - ustawiając katalog nadrzędny z tego repozytorium jako katalog nadrzędny projektu, wszystkie importy działają lokalnie, a zmiana jest tylko na poziomie terraforma do wdrożenia.

Klient wysyłając odpowiednio spreparowane last_beacon_date jest w stanie zmusić system do utworzenia mu wielu identyfikatorów na daną godziną.

Aby nie wprowadzać ludzi w błąd proponujemy archiwizację obecnego master'a. Nowy układ będzie zawierał konfigurację proxy.

It would be beneficial to have type hinting described by PEP484, PEP526 and similar ones.
With that, IDE could help developers and we can catch potential bugs faster using for example mypy in GitHub Actions

Ze względu na to, że numer telefonu nie będzie obowiązkowy (ProteGO-Safe/specs#34 (comment)) użytkownicy będą proszeni o podanie jakiegoś tekstowego identyfikatora (TBD) przed wysłaniem historii spotkań (encounters). Potrzebujemy przygotować na to API, zapisywać w bazie i poprawić dokumentację. Sugeruję pole proof : string.

Dodanie parametru send_sms do requestu register_device

Serwer ma zwrócić listę identyfikatorów, które telefon ma używać do rozgłaszania przez Bluetooth. Każdy identyfikator to UUID (16 bajtów). Każdy identyfikator ma być ważny przez jedną konkretną godzinę czasu lokalnego. Serwer powinien zwrócić listę identyfikatorów na 7 dni do przodu (czyli 7x24 = 168 identyfikatorów). Serwer generuje brakujące identyfikatory, zapisuje je w bazie Beacons i zwraca telefonowi w postaci:

beacon_ids : [ 
{"date":"2020032715", "beacon_id": "685632a0-3f03-4221-bd95-5a3fab6ff77f"},
{"date":"2020032716", "beacon_id": "0537d93b-7c7a-489c-abbc-690f03889b09"},
...
]

data jest w postaci YYYYmmddhh

Funkcja i endpoint. Trzeba się zgrać z mobilkami

Według dokumentacji Google Cloud:

X-Forwarded-For: [CLIENT_IP(s)], [global forwarding rule IP]
A comma-delimited list of IP addresses through which the client request has been routed. The first IP in this list is generally the IP of the client that created the request. The subsequent IPs provide information about proxy servers that also handled the request before it reached the application server. For example:

X-Forwarded-For: clientIp, proxy1Ip, proxy2Ip

Oznacza to, że wartość X-Forwarded-For może być w niektórych przypadkach listą adresów IP. Wygląda na to, że można w łatwy sposób oszukać poniższy kod:

https://github.com/ProteGO-app/backend/blob/ce9a500aefacc180e3a90134fa8e82fc8c72b399/functions/register/main.py#L89-L92

ponieważ z reguły serwery doklejają adres IP jeśli klient wyśle ten nagłówek (nie testowałem na Google Cloud).

Na stronie Quotas and Limits można znaleźć informację o maksymalnym rozmiarze requestu HTTP w przypadku Streaming inserts (używane przez insert_rows):

HTTP request size limit: 10 MB (see Note)
Exceeding this value will cause invalid errors.
Note: Internally the request is translated from HTTP json into an internal data structure. The translated data structure has its own enforced size limit. It's hard to predict the size of the resulting internal data structure, but the chance of hitting the internal limit is very low if you keep your HTTP requests to 5 MB or less.

Oczywiście 5 MB to sporo i jest szansa że to nigdy się nie wydarzy (zależy jak działa aplikacja mobilna) ale myślę, że z uwagi na fakt, że te dane są niezwykle ważne i nie wiadomo kiedy i czy nastąpi retry warto rozważyć trzymanie się limitu 5 MB przy dodawaniu tych danych. W praktyce trzeba wywołać insert_rows kilka razy z mniejszymi paczkami.

Originally posted by @bartekn in https://github.com/ProteGO-app/backend/pull/34/files

Request:

:method: POST
:scheme: https
:path: /confirm_registration
:authority: europe-west3-anna-dev-272212.cloudfunctions.net
accept: */*
content-type: application/json
accept-encoding: gzip, deflate, br
user-agent: ProteGO%20Dev/1 CFNetwork/1121.2.1 Darwin/18.7.0
content-length: 177
accept-language: en-us

{
	"appVersion": "1.0",
	"lang": "en",
	"code": "SYYAN",
	"registration_id": "6c870d3f82a8f33d0b1ab0c1fd0a8d09",
	"osVersion": "13.3",
	"deviceType": "iPhone 8",
	"platform": "ios",
	"apiVersion": "1"
}

Response:

:status: 500
x-cloud-trace-context: bcd60228726dd4b472df3091e188a597;o=1
date: Fri, 03 Apr 2020 14:03:51 GMT
content-type: text/html; charset=UTF-8
server: Google Frontend
content-length: 323
alt-svc: quic=":443"; ma=2592000; v="46,43",h3-Q050=":443"; ma=2592000,h3-Q049=":443"; ma=2592000,h3-Q048=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,h3-T050=":443"; ma=2592000


<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>Please try again in 30 seconds.</h2>
<h2></h2>
</body></html>