Bezpečnostní díra v Nette (?) – v session zůstávají i administrátorem odebrané role*
- enumag
- Člen | 2118
Vezměme v úvahu následující proces:
- Uživatel XYZ se přihlásí do systému.
- Chvíli po něm se přihlásí administrátor a uživateli XYZ změní heslo či odebere nějakou roli.
- 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
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
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
Tak ještě jednou: to není chyba, ale feature. Pokud to v tvé konkrétní aplikaci představuje problém, implementuj si vlastní UserStorage.
- Patrik Votoček
- Člen | 2221
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
@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)
- Filip Procházka
- Moderator | 4668
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
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.
- Filip Procházka
- Moderator | 4668
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
@**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 | 1283
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.
- enumag
- Člen | 2118
@**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:
- Konstanta IDENTITY_CHANGED by měla být součástí \Nette\Security\IUserStorage.
- Ta podstatná část metody getSessionSection by mohla být přímo v \Nette\Http\UserStorage.
- 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
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
@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.
