HTTP Basic login + Http\Url + Nette 3 – security issue?

Jakub Bouček
Člen | 54
+
+1
-

Po aktualizaci balíčku nette\http na verzi 3.0 nás nepříjemně vypekla nenápadná změna z commitu c8dbe23, který mění chování třídy Http\Url. Ta nově při vypsání URL u http(s) requestů do stringu generovaného URL vkládá nyní i HTTP citlivé přihlašovací údaje z HTTP Basic autorizace.

Bohužel to mělo pro nás velmi nepříjemný dopad. V rámci aplikace generujememe relativní odkazy, kde se to neprojeví. Ale jakmile generujeme odkazy pro prostředí, které opouští aplikaci (e-maily, webhooky, atd.), generuje se celá adresa a nyní se na e-maily posílaly URL s včetně jména a hesla (vynechme prosím diskuzi na téma, proč ještě HTTP Basic používat).

Related jsou k tomu diskuze:

Nepovažuju tu změnu za šťastnou – sice formálně je pravda, že to URL se pak skutečně vygeneruje dle zadaných parametrů, ale z pohledu security je pak potřeba u každého výpisu URL hlídat, zda jsem tyto údaje u objektu Http\Url nezapomněl odstranit (a to i preventivně pro případ, že by se v budoucnu aplikace například zapouzdřila pod HTTP Basic auth na straně serveru).

Myslím, že by Nette mělo bezpečnější cestou brát v potaz riziko, že spíše než žádoucí výpis těchto proměnných je mnohem větší pravděpodobnost, že vypsání credentials bude nežádoucí s potenciálem narušit bezpečnost. Ostatně například podobně jako u Cookies, i HTTP Basic má výhodu, že jej browser po zadání uživatelem skrývá, nejsou uvedeny v URL liště a jen se automaticky posílají v HTTP hlavičkách.

Můžeme se bavit o tom, co je formálně správné – tedy má-li v sobě Http\Url objekt definované credentials, měl by je tedy při serializaci do URL vypsat, aby se dokázal obnovit do původního stavu? Formálně je to asi v pořádku, ale prakticky považuji HTTP Basic auth za dosti specifickou problematiku, kterou je třeba řešit individuálně.

Vnímám problematiku podobně, jako když se kdysi Sessions ID pushovalo do URL. Dneska se to považuje za chybu a nemýlím-li se, Nette toto ani nepodporuje, protože je to známá bezpečnostní díra.

Tuto situaci vnímám velmi podobně.

Dávalo by mi smysl, kdyby se při běžném výpisu URL tyto informace nevypisovat a pro výslovný požadavek vytvořit metody:

  • \Nette\Http\Url::getAuthorityWithCredentials()
  • \Nette\Http\Url::getHostUrlWithCredentials()
  • \Nette\Http\Url::getAbsoluteUrlWithCredentials()
CZechBoY
Člen | 3608
+
0
-

Můžeš poslat nějaký repro kod? Negeneruješ náhodou odkazy přes `$presenter->link()· apd.?

Felix
Nette Core | 1183
+
-2
-

Nepomohlo by ti toto?

$this->httpRequest->getUrl()->withoutUserInfo()

Nebo by jsi radeji chtel zmenit default generovani?

Jakub Bouček
Člen | 54
+
+2
-

@CZechBoY V tomto případě nejde přímo o Nette aplikaci, ale pouze o používání Nette balíčků, jako je např. nette/http a jeho Http\Request.

@Felix Jako pomohlo a nepomohlo. Jde o to, že by IMHO framework, který si buduje pověst nejbezpečnějšího frameworku mohl jít naproti situaci jako je shora popsaná – tedy aby výchozí rendering Url objektu byl secure, protože s ohledem na praxi používání HTTP Basic auth je běžně žádoucí, aby se citlivé user info nedostávalo tak snadno ven.

Proto mi přijde nutnost volání ->withoutUserInfo() v každém místě výpisu nežádoucí a naopak je žádoucí, aby se informace vypsaly pouze na jednoznačně sdělenou žádost. BTW: Díky, o této metodě jsem nevěděl, použil jsem ->withUser('')->withPassword('')

Editoval Jakub Bouček (10. 3. 2020 0:12)

David Grudl
Nette Core | 8082
+
+1
-

Ta změna vznikla proto, aby objekt Url/UrlImmutable, když mu nastavím credentials, je i vypsal, viz to PR #63. To je myslím zcela legitimní požadavek a na tom bych nic neměnil.

Další věc je, jestli v rámci objektu Http\Request mají být credentials dostupné přes URL. Jako API se k tomu vyloženě nabízí a userinfo v URL funguje v řadě klientů.

Na druhou stranu podle RFC https://tools.ietf.org/html/rfc3986#… je userinfo deprecated a prohlížeče to dnes už ignorují.

Asi řešením by mohlo být tyto informace do Url nepředávat a místo toho přidat do Http\Request nějakou metodu getUserInfo() nebo getCredentials()

Jakub Bouček
Člen | 54
+
+2
-

Přemýšlím nad tím už několikátý den a moc se mi navržené řešení nelíbí. Zejména s ohledem na bezpečnost a předvídatelnost.

Snažím si uspořádat si, jak by to mělo být správně a narážím na to, že celém návrhu se userinfo tyčí jako něco nesystémového a vytváří to slabá místa.

Předně si vezměme URL. Praxe v používání HTTP je taková, že do URL by se neměly vkládat citlivé údaje, proto se všechny citlivé údaje posílají přes POST data a nebo přes Cookies – v obou případech existují pokročilé mechanismy, které jsou vytvořeny, aby tyto mechanismy chránily (HttpOnly, SameSite, excludování v logách, ap.).

Takže existuje pochopitelný předpoklad, že co je v URL, je de-facto veřejné a nevyžaduje zvláštní pozornost (nechme stranou oprávněnost takového předpokladu a přijměme fakt, že takto může hodně vývojářů smýšlet).

userinfo ale vyniká tím, že přestože je ve skutečnosti posíláno HTTP hlavičkou a je zjevně velmi citlivým údajem, tak je ale z historických důvodů interpretováno v URL. To je právě ten point, v němž shledávám nesystémovost a proto si myslím, že v úvahách vyžaduje více individuální a méně formální přístup.

David Grudl napsal:
Další věc je, jestli v rámci objektu Http\Request mají být credentials dostupné přes URL. Jako API se k tomu vyloženě nabízí a userinfo v URL funguje v řadě klientů.
Na druhou stranu podle RFC https://tools.ietf.org/html/rfc3986#… je userinfo deprecated a prohlížeče to dnes už ignorují.
Asi řešením by mohlo být tyto informace do Url nepředávat…

Tak tomu bylo, než se to v PR nette/http#161 změnilo. Tu změnu si vyžádala potřeba nějakého use-case, kdy bylo potřeba userinfo do Url objektu zpropagovat. Otázkou je, zda autor inicujícího issue nette/http#160 vnímal aspekt, že se credentials na druhém konci aplikaci začne propisovat do všech URL.

Ostatně je vidět, že na to záhy reaguje commit d1378d4 v nette/application, který v rámci Nette aplikaci blokuje propagování userinfo do presenterů a šablon. Dá se říci, že při použití celé Nette aplikace je problém vyřešen.

Nicméně sám narážím na to u aplikací, které nejsou postaveny nad Nette, kde potřebuji sestavit URL request, aby aplikace mohla odkazovat sama na sebe – čisté PHP neposkytuje žádnou přímou cestu, než si URl složitě skládat z parametrů, proto používám \Nette\Http\RequestFactory::fromGlobals()->getUrl(). A jde o to, že tímto způsobem mi vlastně v aplikaci může vzniknout nežádoucí zranitelnost. Současně ale API žádného z použitých objektů nenapovídá, že by něco takového mohlo vzniknout a já musím myslet na to, abych zavolal withoutUserInfo().

Další problém je, že se s tím vývojář vůbec nemusí v aplikaci potkat do doby, než vypíše absolutní URL v requestu autorizovaném přes HTTP Basic auth a tak se problém může v aplikaci ukrývat velmi dlouho.

Pokud se nemýlím, tak jak uvedený commit v nette/application, tak i můj problém spočívá především v tom, že je nežádoucí, aby se credentials vypsaly na výstup. Proto mi dává smysl ta správná opatření posunout co nejblíže k místu, kde je problém patrný – tedy tam, kde se objekt Url překládá na string, tedy tam, kde skutečně dochází v vypsání userinfo.

Pro srovnání: vnímám tu problematiku podobně jako escapování HTML. Taky bychom neměli escapovat POST data při vstupu, ani při ukládání do databáze, nebo při čtení z ní, ale až v okamžiku, kdy se hodnota skutečně propisuje do HTML, aby bylo na první pohled zřejmé, že jsou data použita ve správném kontextu.

Současně mi přijde můj návrh podobný návrhu Fetch API v browserech, kdy funkce fetch() také ořezává credentials, dokud vývojář výslovně jejich poslání nevynutí.

Safe-first.

Felix napsal:
Nebo by jsi radeji chtel zmenit default generovani?

Přesně tak. Upravit defaultní generování, aby nehrozil neočekávaný leak hesel, ledaže by si to vývojář vyloženě vyžádal.

Editoval Jakub Bouček (15. 3. 2020 0:26)

David Grudl
Nette Core | 8082
+
0
-

Teď vůbec nevím, které navržené řešení se ti nelíbí.

Jakub Bouček
Člen | 54
+
+1
-

Moc se omlouvám, že jsem se odmlčel, nevím už ani proč. Bohužel jsem opět narazil na tento problém znovu, kdy aplikací proběhl Basic Auth request a vypsal se kam neměl, protože někdo v kódu potřeboval URL aktuální stránky to získal takto:

echo $httpRequest->getUrl();

// https://example.com@username:password/index.php

Znovu jsem si celé vlákno pročetl a tentokrát jsem dospěl k jinému názoru než posledně a souhlasím s @DavidGrudl, tedy aby se credentials nepropisovaly do objektu Http\Url v Requestu a místo toho se tyto informace uložily pouze do objektu Http\Request.

Jakub Bouček
Člen | 54
+
0
-

Poslal jsem PR: https://github.com/…ttp/pull/211

Ale nenapadá mě, jak rozumně zajistit zpětnou kompatibilit, aby Url vyhodilo warning, když se vývojář pokusí číst credentials z $httpRequest->getUrl()->getUser().