Bezpečnostní díra v Nette (?) – v session zůstávají i administrátorem odebrané role*

Upozornění: Tohle vlákno je hodně staré a informace nemusí být platné pro současné Nette.
enumag
Člen | 2118
+
0
-

Vezměme v úvahu následující proces:

  1. Uživatel XYZ se přihlásí do systému.
  2. Chvíli po něm se přihlásí administrátor a uživateli XYZ změní heslo či odebere nějakou roli.
  3. Uživatel XYZ provede akci, na kterou by po odebrání role již neměl mít právo.

Pokud mi něco neuniklo tak se to uživateli XYZ povede! Jeho identita včetně rolí je totiž uložena v Session již z doby kdy se přihlásil a systém nikde neověřuje, že se něco změnilo. Ze stejného důvodu případná změna hesla administrátorem nezpůsobí automatické odhlášení.

V kombinaci se session slidingem si tak může uživatel hypoteticky držet svá původní oprávnění po neomezeně dlouhou dobu.

Problém s rolemi lze řešit Majklovou FakeIdentity, ale to je trochu hack. Problém s (ne)odhlášením navíc i tak zůstává.

Jan Tvrdík
Nette guru | 2595
+
0
-

Tohle není bug, ale feature se kterou prostě musíš počítat. Pokud to potřebuješ řešit, tak při každém požadavku si načti uživatele z DB a ověř aktuálnost dat.

enumag
Člen | 2118
+
0
-

Vyřešit to umím, jen se mi nelíbí, že něco takového je přímo v Nette. Nemám problém připravit pull request s opravou (psát to budu tak jako tak), ale musím vědět, že ten pull request má smysl vytvářet – což dle tvých slov zřejmě nemá. Inu… jak je ctěná libost, mějte si aplikace děravé. :-)

Editoval enumag (23. 1. 2013 14:22)

Filip Procházka
Moderator | 4668
+
0
-

Tak ještě jednou: to není chyba, ale feature. Pokud to v tvé konkrétní aplikaci představuje problém, implementuj si vlastní UserStorage.

enumag
Člen | 2118
+
0
-

Když pořád opakujete, že je to feature, tak jaké to má výhody? Kromě jednoho ušetřeného dotazu na databázi nevidím žádné.

Patrik Votoček
Člen | 2221
+
0
-

feature je právě a přesně ten ušetřený dotaz. A hlavně to nemůžeš pořešit jinak aniž by jsi neudělal závyslost na datové (resp. databázové) vrstvě kterou používá každý jinou.

enumag
Člen | 2118
+
0
-

@Patrik Votoček: Ono by úplně stačilo kdyby IAuthorizator měl druhou metodu isIdentityValid(IIdentity $identity); a User by ji používal na ověření zda se identita nezměnila. Komu by vyhovovalo stávající chování mohl by do té metody klidně napsat return TRUE;. Kdo by to chtěl řešit tak by si ji implementoval dle své databázové vrstvy.

Editoval enumag (23. 1. 2013 16:23)

hrach
Člen | 1822
+
0
-

Divas se na to dost spatne. Vetsina aplikace zase takovou spravu roli nema, neresi zmenu hesla administratorem, ci podobne. V aplikaci, kde toto pouzivame, to samozrejme je implementovane spravne. Takze zatim to vypada, ze to mas derave jen ty :D

Filip Procházka
Moderator | 4668
+
0
-

enumag napsal(a):

@Patrik Votoček: Ono by úplně stačilo kdyby IAuthorizator měl druhou metodu isIdentityValid(IIdentity $identity);

To by teda nestačilo, protože od toho je UserStorage.

cabadaj
Člen | 8
+
0
-

Já souhlasím s @enumag. Nette se, pokud vím, snaží defaultně chovat tak, aby nevznikaly bezpečnostní problémy. A tady, když použiju Nette tak jak je doporučováno, tak mi to může vytvořit díru v aplikaci. Stačí jedoduchý příklad, kdy se uživatel přihlásí a pak je administrátorem například zabanován. Pokud se sám neodhlásí, tak zabanování moc nepomohlo.

Chápu, že zatěžovat aplikaci při každém requestu zbytečným dotazem je nehezké. Ale nedeklaroval bych to jako feature. Bug to je, mělo by se to vyřešit. Teď je jen otázka jak na to.

Nebo by měl být aspoň někde návod, jak tuhle situaci v Nette co nejlépe vyřešit.

uestla
Backer | 785
+
0
-

Nette nabízí super nástroje, ale tohle jsem od něj nikdy neočekával – už z důvodu, že by mi nebylo jasné, jakým způsobem by vůbec byl schopen zjistit, že došlo ke změně práv. To je prostě součástí byznys logiky, do které by framework neměl zasahovat.

Filip Procházka
Moderator | 4668
+
0
-

Nemělo by se to vyřešit, protože to není chyba. Jakékoliv ověřování čehokoliv proti databázi je úkol pro tvůj kód, nikoliv pro Nette. Poděď si UserStorage a implementuj logiku v něm.

MartinitCZ
Člen | 580
+
0
-

@**enumag**, @**cabadaj**: Tak navrhněte řešení ;)
Osobně si myslim, že tohle je věc, kterou Nette řešit nemá a ani nemůže. :)

Milo
Nette Core | 1257
+
0
-

Podívejte se na to tak, že role jsou obyčejná data vytažená z databáze. A vy je ve svém kódu předáte do Identity jako pole hodnot.

Když tohle uděláte ve své aplikaci s jinými daty, očekáváte, že se automaticky změní když je změníte v databázi? Ne. Budete přemýšlet nad tím, jak je zinvalidovat a obnovit. Stejným způsobem přemýšlejte i nad rolemi.

Milo
Nette Core | 1257
+
0
-

Btw. nemělo by stačit pouze podědit Identity na MyIdentity, přetížit metodu getRoles() a v autentikátoru vracet tuto identitu?

Beru zpět. Nestačí.

Editoval Milo (25. 1. 2013 14:12)

enumag
Člen | 2118
+
0
-

@**enumag**, @**cabadaj**: Tak navrhněte řešení ;)
Osobně si myslim, že tohle je věc, kterou Nette řešit nemá a ani nemůže. :)

Tak jsem si to implementoval. Jak by to podle mne mělo být:

  1. Konstanta IDENTITY_CHANGED by měla být součástí \Nette\Security\IUserStorage.
  2. Ta podstatná část metody getSessionSection by mohla být přímo v \Nette\Http\UserStorage.
  3. Protože Nette to řešit opravdu nemůže, výchozí implementace metody isIdentityValid by byla return TRUE;.

Kdo by chtěl tento bezpečnostní nedostatek řešit, měl by ušetřenou práci plus předdefinovanou log-out reason konstantu. Stačilo by implementovat tu metodu (+ konstruktor kvůli závislosti).

Editoval enumag (4. 2. 2013 21:10)

Honza Marek
Člen | 1664
+
0
-

Nějaká metoda isIdentityValid je opravdu jen nesmyslná komplikace. Jak už bylo psáno výše, správné řešení je podědit UserStorage tak, aby metoda setIdentity uložila do session id uživatele a metoda getIdentity uživatele načetla podle id z databáze.

enumag
Člen | 2118
+
0
-

@Honza Marek: To není to o co mi jde. Já nechci původní identitu uživatele vždy nahradit tou aktuální, tím bych nijak neošetřil změnu uživatelského jména nebo hesla. Chci uživatele odhlásit a sdělit mu důvod odhlášení. Kromě toho i pokud můj případ řešit nechceš tak potřebuješ uživatele odhlásit pokud v databázi už není, protože v tom případě nemůžeš načíst jeho identitu.

Honza Kuchař
Člen | 1661
+
0
-

Tak z getIdentity vyhazuj při změně informací v databázi výjimku.

pekelnik
Člen | 462
+
0
-

Tato diskuse se toci v kruhu…

enumag
Člen | 2118
+
0
-

Souhlasím. Do Nette se to evidentně v žádné podobě nedostane. Kdo bude chtít, může použít mou implementaci z Gistu. Další diskuse je myslím zbytečná.