Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Added secured signals #469

Closed
wants to merge 2 commits into from
@hrach

Pouze jedna poznamka: kvuli nevyreseni validace parametru: http://forum.nette.org/cs/6819-validace-parametru-v-presenterech vypada metoda getCsrfToken tak, jak je. Po teto uprave bude mozne pouzit bezne serialize.


Reseni: https://github.com/nextras/secured-links

Nette/Application/UI/Presenter.php
@@ -990,6 +1005,27 @@ public function lastModified($lastModified, $etag = NULL, $expire = NULL)
/**
+ * Returns unique token for method and params
+ * @param string
+ * @param string
+ * @param array
+ * @return string
+ */
+ protected function getCsrfToken($control, $method, $params)
+ {
+ $session = $this->getSession('Nette.Application.Presenter/CSRF');
+ if (!isset($session->token)) {
+ $session->token = Nette\Utils\Strings::random();

Jeden token po celou dobu přihlášení? Jak moc je to bezpečné? (otazník znamená otázku, opravdu nevím)

$session->setExpiration('+ 10 minutes');
@hrach
hrach added a note

taky sem nad tim premyslel, nicmene predstava, ze si zobrazim stranku a nejaky odkaz mi na ni po 10 minutach nebude fungovat je hruza;
navic, ochrana funguej vzdy nejlip tak, kdyz nikde neni dira, kdyz nekde bude dira, ani tech 10 minut nebude dostatecny fix ...

V tom by neměl být žádný problém. Ten token leží na serveru a nelze ho nijak ukrást. Pokud lze, tak je díra jinde.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fprochazka

Btw moc pěkný commit ;) +1 :P

@dg
Owner
dg commented

Díky za request. Raději bych to asi nechal do verze 2.x

@hrach

prijde mi to jako hodne zasadni vec, zvlaste ve chvili, kdy se nette chlubi svou bezpecnosti. Feature je to jiz dlouho pozadovana, na foru se vali 3 roky... v kontextu nekolika experimental commitu nevidim duvod odkladu. Nebo ano?

@JanTvrdik

Díky @hrach­ovi za dokončení, ale mohl jsi mi dát aspoň mention :)
@dg +1

Nette/Application/UI/Presenter.php
@@ -990,6 +1005,27 @@ public function lastModified($lastModified, $etag = NULL, $expire = NULL)
/**
+ * Returns unique token for method and params
+ * @param string
+ * @param string
+ * @param array
+ * @return string
+ */
+ protected function getCsrfToken($control, $method, $params)
+ {
+ $session = $this->getSession('Nette.Application.Presenter/CSRF');

Nette.Application.UI.Presenter/CSRF :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@NoxArt

Zrovna Security věcem bych udělil velkou výjimku a do kódu je uvedl ještě do současné verze. Navíc když je v Hlavní přednosti uvedená Bezpečnost jako první a jestli kód k tomu už je...

A jinak hezká práce JanTvrdik, hrach

@hrach

Ano ano, prakticky je to prace jenom @JanTvrdik. (krom testu, nejakeho poladeni a otestovani :D) Timto se omlouvam. Sve sem k tomu rekl. Ohledne "Application.UI" - verim, ze to uz si David pripadne pri mergi poresi :)

@hrach

Dovolil jsem si opravit Application.UI namespace v session a prepushnout s autorstvim @JanTvrdik ;)

@JanTvrdik

Proč ty parametry vlastně nejde serializovat?

@hrach

Protoze z PresenterRequest prijde $id jako string, ale pro link(..., array('id' => 2)) dostane getCsrf $id jako integer.

@dg
Owner
dg commented

Protože tohle není kompletní řešení (chybí podpora pro akce nebo AJAX) a jde o bezpečnost, tak bych to za každou cenu neuspěchával.

@hrach
  • podpora pro ajax: normálně to v ajaxu funguje. Proč by nemělo?
  • podpora pro akce: akce by neměla měnit stav aplikace, pokud ano, je to imo špatně udělaný návrh, tam bych tuto funkcionalitu záměrně nepřidával.
@JanTvrdik

@dg: S AJAXem nám analogické řešení velmi dlouho a velmi spolehlivě funguje. Pro akce to není naimplementované pouze proto, že se bojím o výkon (netestoval jsem to). Ale souhlasím s tebou, že co se týká bezpečnosti, tak se rozhodně spěchat nevyplatí.

@Aurielle

Protlačme to ještě do Nette 2 final, bump!

@dg
Owner
dg commented

Po verzi 2.0 budou i další verze :-)

@JanTvrdik

V to mi doufáme. Ideálně aby mezi nimi byla menší pauza, než mezi 0.9.7 a 2.0 :)

@dg
Owner
dg commented

Poměrně dlouho mi trvalo přijmout, že nová velká verze nemusí obsahovat všechny nápady, ale lze je nechat do verzí příštích. Třeba jako secured signals ;-)

@hrach

Co ted?

@JanTvrdik

Teď by to šlo :-)

@hrach

Tak me napada, to validovani parametru, vyresilo uz ten problem?

@JanTvrdik

Validace parametrů implementovaná není. Ale i kdyby implementovaná byla, tak si myslím, že ty hodnoty parametrů je lepší porovnávat netypově.

@JanTvrdik

Jinak zrovna ten algoritmus, kterým generuješ ten token se mi nelíbí, ale nic výrazně lepšího mě nenapadá.

@hrach

Tak co teď? n.2

@hrach

@HosipLan ne, chtel jsem do____(dopln dle vlastnich preferenci) rict, ze uz je to druha zadost, a 2 mesice starej pull.

@fprochazka

Já si taky myslím, že by se tomu mohla začít zase věnovat pozornost.

@JanTvrdik

Teď by to šlo. (2)

@dg
Owner
dg commented

Mám k řešení pár poznámek:

  • Aby to bylo skutečně bezpečné, nesmí nikdy dojít k tomu, že se zobrazí stránka, kde v URL je autorizační token. Musí se místo toho vždycky přesměrovat na stránku bez tokenu, jinak jde o falešný pocit bezpečí. Token se jinak prozradí skrze referery.
  • Životnost tokenu by se měla ukončit při přihlášení / odhlášení uživatele. To by bylo velmi žádoucí i v případě CSRF ochrany u formulářů, nicméně u odkazů je to naprosto nutné.
  • V souladu s plánovanou "destrukcí" presenterů by bylo fajn přijít s implementací, která nebude presenter ještě více zesložiťovat.
@hrach

Moje poznámky:

  • mas k tomu nejaky hint, napad?
  • ok
  • co to je za vymluvy? Toto jasne patri do impelentace jinych sluzeb (link, opravneni) a neni imo spravne to vyclenovat bokem jen kvuli tomu, ze to ma byt prvni vlastovka nejake restruktulizace. Spravnym postupem je to naopak implementovat do stavajici stuktury a posleze teprve vyclenit cele bloky, jako je tvorba odkazu, kontrola opravneni, apod. Klidne muzu s tim pomoct a potom tuto praci rozpracovat, ale odmitam ted michat jabka s hruskami.

Na mem skoleni to byla prvni otazka, kdyz jsem predvadel signaly/handle metody: Pry, jestli je to zabezpeceny proti csrf. Bylo mi opravdu trapne, kdyz jsem musel rict, ze neni. Par minut predtim mluvim o bezpecnosti, a pak toto. V tomto pripade si myslim, ze nedokonala ochrana je porad lepsi nez zadna, zvlaste v kontextu, kdyz se nette prohlasuje za framework eliminujici bezpecnostni chyby.

@dg
Owner
dg commented

Mám to chápat tak, že ti bylo trapně, že jsi nedodal dokonalé řešení? ;-)

@hrach

Pri dorzeni spravneho postupu je to dokonale bezpecne reseni. Po signalu proste presmerovavam.

@dg
Owner
dg commented

Rails way ;-)

@hrach

Nette way je pak poskytovat framework, ktery nema ochranu proti csrf.To jsme to vyhrali!!!

@juzna

Kdyz ale odesles formular a je v nem chyba, tak se zadne presmerovani nekona a formular se vykresli znovu, na URL pro zpracovani signalu (a tedy s tokenem). Obecne, kdyz behem zpracovani signalu neco neklapne. Bacha na to.

@dg
Owner
dg commented

Ano. Nette vždycky řešilo věci buď dobře, nebo vůbec.

Ale moc nechápu rozčilování nad tím, když poukážu na nedostatky nějaké implementace. Zabezpečení CSRF není žádná legrace a je potřeba si možné návrhy navzájem oponovat.

@hrach

@juzna Vsak ja souhlasim s tim, ze by toto melo byt doreseno. Jen me vytaci alibismus, ze aktualni stav je lepsi, nez ten, ze v 1 % pripadu nekomu zustane po signalu nepresmerovany stav a uzivatel prejde jinam, kde cilova stranka vyuzije referer a zneuzije ho na utok csrf. Misto toho nedame do nette moznost obrany pro 99 % zbylych pripadu.

Mimo to, je to velmi stara a pozadovana feature - min. 3 roky. http://forum.nette.org/cs/2384-widget-predani-parametru-komponente-potvrzeni-smazani#p17654 Celou dobu se na to peklo, potom ja z Honzovyho kodu udelam pull a napisu testy a objevi se tu neco jako, ze to prej nefunguje pro ajax. (WTF?) Potom dva mesice ticho a pak se sesype sada dalsich (edit: ale relevantnich) duvodu, proc zrovna dane reseni neni fajn. Mohli padnout pred dvemi mesici a dnes mohlo byt hotovo.

@NoxArt

Mám hloupý dotaz - "signálem" se tu myslí handle, nebo obecně request? Tím myslím - pokud by někdo něco dělal v action, bude to zabezpečené taky? (nepředpokládám, že by jakékoli akce musely vynuceně probíhat jen v handle)

@juzna

Zkusil jsem si to pullnout a zatim jsem spokojen, vypada to dobre. A ten divnej pocit, ze mi nekdo bude mazat polozky, kterej se porad vnucoval, se zacina ztracet. Ale jdu to jeste zkusit nejak hacknout ;) a overit, zda to ma teda nejake uskali.

Jinak drobnost, mozna by za to stalo sjednotit velikost pocatecniho pismene s ostatnima anotacema, ted mam napr u signalu:

/*
 * @User @secured
 */

a to vypada divne (neni to uplne ono)

@NoxArt

To znamená, že co bude v action a ne v handle nebude zabezpečené, resp. ani se neplánuje? Vzhledem k tomu, že dokumentace doslova uvádí, že action je na různé úkony, byla by to celkem zásadní změna (plus restrikce na aktuální view), když CSRF může klidně dojít na ten action.

Nebo mi něco uniká?

@hrach

@NoxArt mně uniká, co si chtěl vůbec říct? Jaká zásadní změna? Co jsou to různé úkony? Uf. Pročti to celý, pullni si to, otestuj si to. A kod měnicí data v db apod. piš do handle/formu.

@fprochazka

Výchozí CSRF i na akce by mohlo být dost kontraproduktivní. Potřebuješ snad CSRF ochranu na stránce s výpisem článku? Na tyhle úpravy jsou nejvhodnější signály :)

@juzna

@HosipLan tam neni nic vychozi nebo automaticke, musis to aktivovat anotaci @secured ;)
A IMO by se to hodilo i na akce, napr na vsechny akce tohoto presenteru by se mi to hodilo a nevidim nejak duvod predelavat je na signaly (zejmena, chci k nim jednoduche routy napr i z CLI).

Je mezi signalem a akci takovy rozdil? Jak rika treba dokumentace: "...presenter provede určitý úkon (přihlásí uživatele, zapíše data do databáze) a poté přesměruje jinam..."

@NoxArt

Myslel jsem to přesně jak to popsal juzna

Ještě teda k tomu @secured ... nadhodil bych jestli to nemá to být defaultně zapnuté. Třeba i viz http://zdrojak.root.cz/clanky/aktualne-tak-nam-hackli-github/nazory/21526/ a viz escapování v šablonách. Asi záleží na projektu, možná má někdo v action jen načítání z DB, ale někdo tam třeba má většinově zapisovací/měnící akce.
To stejné asi k formům, jestli by addProtection nemělo být přidáno defaultně a vynechat jen při explicitním zakázání - což by šlo, např. nějakým protected flagem, něco jako $autoCanonicalize v UI\Presenter

@dg
Owner
dg commented

ad bod 1: možná by životní cyklus měl být po fázi handle() násilně ukončen přesměrováním na aktutální view, pokud jej tedy už vývojář neukončil přesměrování sám

ad bod 2: asi by stačilo do CSRF klíče zahrnout user->getId().

ad ajax: tím, jak je CSRF klíč generován, nejsem schopen vytvořit AJAXový požadavek s vlastními parametry, tj. $.get(url, params). Tj. lze vytvářet pouze odkazy na straně serveru. Nejsem si jist, jestli tento způsob generování tokenu zvyšuje bezpečnost z hlediska CSRF (můžu poprosit o vysvětlení?) Pokud ne, tak bych generování tokenu změnil tak, aby se dalo používat na straně klienta.

@dg
Owner
dg commented

CSRF je bohužel útokem na nedostatek v návrhu HTTP a jakákoliv ochrana je jen workaroundem s vedlejšími efekty. Z toho důvodu nelze ochranu aktivovat zcela automaticky jako třeba v případě XSS nebo SQL injection. Pokud by nějaká defaultní ochrana měla existovat, tak by musela být implementována tak, aby neházela klacky pod nohy. Jinak by ji automaticky každý vypínal.

@dg
Owner
dg commented

Uff, teď jsem si omylem smazal komentář, tak stručně: zdá se mi jako nedomyšlenost, že o ochranu před CSRF se žádá v kódu komponenty. Musel bych pak komponenty třetích stran editovat. A nešlo by je použít v obou scénářích, tj s ochranou i bez.

@jasir

@dg zrovna jsem ten tvůj smazaný komentář četl, tak ho sem postnu:

Další věc k zamyšlení: je skutečně žádoucí, aby o ochranu před CSRF žádal kód komponenty? Neměl by to spíš dělat uživatel komponenty? Podobně, jako se komponenty stávají persistentní kódem presenteru, nikoliv komponenty. Při použiti cizí komponenty budu muset zasáhnout do jejího kódu, abych aktivoval ochranu?

@fprochazka

Mě by se naopak líbilo, mít implicitně všechny signály zabezpečené s tím, že by to šlo vypnout.

@hrach

Pokud bychom automaticky presmerovali, tak uvazme situaci: ajaxovy pozadavek vede na handle - provede se uspesne a my nepresmerujeme (ajax by byl totiz k nicemu). Doted vsechno ok, do url se nedostane token (->zadny referer) - az do chvile, nez pouzije nekdo html5 history api. Ha, a uz jsme zase v riti. Ne, automatika neni to nejlepsi reseni.

I kdybychom vymysleli super reseni na strane nette, zalezi zase na frontendu, co s tim udela a jestli to neprozradi. Proste je treba chapat co kdy a kde jak delam. Sebelepsi framework me neochrani, pokud nevim co delam.

Pres vyse zminene bych to dale asi neresil. Dokonalou ochranu, jak padlo od @dg, napsat nejde. A protoze Nette, jak uz vime, resi neco bud uplne, nebo vubec, je tento pull mrtvy.

@JanJakes

Zeptám se - kde přesně je token v refereru opravdu problém? Ono samotné vyzrazení tokenu by ještě neměl být problém, ne? Pokud je token vázán na uživatelskou session, jinému uživateli je k ničemu, nebo se pletu?

Nestačí tedy zajistit "It is important to state that this challenge token MUST be associated with the user session, otherwise an attacker may be able to fetch a valid token on their own and utilize it in an attack."?

@fprochazka

@jakes: můžeš okamžitě při odhalení přes referer vložit do stránky

<img src="http://zranitelny.web/admin/user/?id=10&do=delete&token=<?=$token?>" />

a ani vázání na session tě nezachrání

@NoxArt

Jenže z podstaty CSRF je iniciátorem v podstatě sám uživatel. A když získáš z refereru token, víš čí je, takže můžeš konkrétnímu návštěvníkovi předhazovat ten jeho token.

@dg
Owner
dg commented

@jakes při současné implementaci a předpokládaném způsobu použití je vyzrazení tokenu docela malý problém, protože se váže na jednu akci & parametry. Že je vázán na session nehraje roli, protože CSRF útok probíhá v prohlížeči napadeného s jeho session.

@JanJakes

Nojo, vždyť session je stejná, protože akci podrstčíme uživateli. Díky.

Každopádně pokud je token vázán na akci a parametry, jaká je hrozba při vyzrazení? Opakování stejné akce se stejnýmmi parametry v malém časovém horizontu? Jsou zde i jiné hrozby? Třeba by se dalo jít i touto cestou - při vyzrazeném tokenu se dostat na prakticky nulové riziko...

@dg
Owner
dg commented

@hrach tento pull není mrtvý, ale chybí jakákoliv analýza, takže je to spíš takové to domácí chránění před CSRF.

@dg
Owner
dg commented

ad AJAX: výhodou AJAXu je, že zachovává same origin policy, takže AJAXový požadavek je už z principu ověřený. Bohužel existuje způsob, jak využít chyby v Flash pluginu a provést na jiný server požadavek, který vypadá jako AJAXový. Z toho důvodu bych AJAXové požadavky chránil tak, že se pošle speciální HTTP hlavička s ověřovacím tokenem. Ten se může do stránky dodat třeba v <html data-nette-token="...">. Žádné další tokeny pro jednotlivá URL nebudou potřeba.

Taková ochrana by mohla být (časem?) defaultně zapnutá (ano, BC break).

@hrach

Cross domain requesty je mozne delat uplne standardne i v js. https://github.com/padolsey/jQuery-Plugins/tree/master/cross-domain-ajax/ Sic si nejsem jist, jestli dany skript zrovna zvladne i hlavicku simulujici ajax, ale predpokladam, ze bez problemu. A i kdyby ji nedokazal, handle metody zrovna vetsinou jsou dostupne i bez vynuceneho ajaxu.

Navrhovane data-nete-token smrdi magii. Ala zavinace! Snízena bezpecnost neunikatností tokenu pro handle/parametry pak utok povysuje na stejnou uroven jako ukradnuti session id.

@dg
Owner
dg commented

Cross domain requesty pochopitelně dělat lze, bez toho by nefungovalo třeba stahování jQuery z CDN, psal jsem o AJAXových požadavcích probíhajících v prohlížeči uživatele. Mícháš dohromady nesouvisející.

@hrach

Nevim, odkdy je Ajax neco jineho nez request. Ze to musi probihat v prohlizeci uzivatele je snad jasne, kdyz se bavime o CSRF. O tom, ze ten pozadavek musi byt spusten z jineho webu, je snad taky jasne, kdyz ten web mi ukradl token z refereru.

@JanJakes

A co tato cesta: Unikátní token pro konkrétní akci s konkrétními parametry (takže vlastně současné řešení) a ještě bych přidal "pro každé další provedení stejné akce jiný token", což by asi řešilo jednoduše smazání $session->token při provedení signálu. (A chování při AJAXu a bez bych nechal stejné.)

Jaké má toto řešení vady? Vyzrazení tokenu např. přes referer by vadit nemělo - už nebude platný.

@hrach

@jakes: no, to je dobry napad, pri overeni platnosti a splneni platnosti se stary token smaze. Ted uz tedy vyresit Daviduv pozadavek, ze ma byt dana vec pristupna pres ajax,aby si sam mohl generovat parametry. Zda se nekomu tento pozadavek (i)relevantni? Uz byste to nekdy pouzili? Cele signaly.cz to nepotrebovali ani jednou.

@dg
Owner
dg commented

@hrach když už o věci vím kulový, tak bývá lepší alespoň dočasně omezit aroganci

@jakes při každém provedení akce jiný token je dost zásadní implementační komplikace. Nevidím problém v tom přesměrování, protože akce, která povede ke kreslení stránky, by měla k témuž vést i po přesměrování bez tokenu.

@hrach

Muj posledni souhlas s jakes byl blbost, pac pak by mi prestal fungovat link v jinem tabu.

@dg to jsem rad ze to teda omezis, konecne se nekam dostaname. Zatim tu totiz je jenom rails way, kdy si hloupi vyvojari nezabezpecuji signaly a neni to chyba frameworku.

@dg
Owner
dg commented

Nevíte o nějakém frameworku, který CSRF ochranu odkazů řeší?

@JanTvrdik

Aha! Konečně jsem si vzpomněl, proč máme na signálech tu podmínku na přeskočení nepovinných parametrů :) Používá se to právě k tomu, že nepovinné parametry nejsou zahrnuty do generování tokenu. Tj. ty parametry, jejichž hodnoty neznám v době generování odkazu musí být označeny jako volitelné. Nutno podotknout, že se mi to řešení nelíbí.

Vynucení přesměrování považuji za dobrý nápad.

@JanTvrdik

Je skutečně žádoucí, aby o ochranu před CSRF žádal kód komponenty?

Existuje nějaký důvod, proč by tu anotaci neměly mít všechny komponenty? Stejně jako je věc autora komponenty zajistit, že komponenta není náchylná na XSS, tak by měl zajistit, že tam ta anotace bude, ne?

@JanTvrdik

ad zabezpečení běžných odkazů: jediný důvod (pokud si dobře vzpomínám), proč to není naimplementované i pro běžné odkazy je obava o rychlost. Teď se kvůli každému signálovému odkazu parsují anotace nad příslušnou handle metodou. Dokažte mi, že to nemá negativní dopad na rychlost aplikace a bude podpora i pro běžné odkazy.

@NoxArt

@Jan Tvrdík "Existuje nějaký důvod...?" Zajistit by měl, ale proč by mu s tím framework nepomohl? Když se podíváme, jaká se tu vede rozsáhlá diskuse, nevypadá to na triviální záležitost. A když už by v Nette toto bylo, proč to nemít defaultně zapnuté, stejně jako escapování, deny frames etc.? Kdo bude chtít, tak si to vypne... A kromě bezpečnosti to bude navíc DRY

@dg
Owner
dg commented

Zpravidla se snažím výkonný (ve smyslu exekutivní) kód do komponent nedávat, takže zda signál mění stav serveru a vyžaduje zabezpečení před CSRF není zřejmé ze samotného handle, ale teprve obsluhy nějaké události (např. https://github.com/nette/examples/blob/master/Fifteen/app/components/FifteenControl.php#L37 zabezpečení nevyžaduje, ale obsluha $onGameOver může psát do databáze).

Obdobně třeba u si u datagridu definuji u každého řádku "nějaké odkazy" a význam jako "link na detail" nebo "smazání" jim dávám mimo komponentu.

@dg
Owner
dg commented

Příkladem modifikování URL je třeba signál pro smazání položky: při odeslání JavaScriptem a po potvrzení „Skutečně chcete položku smazat“ je mu přidán parameter confirmed=1, který tam při generování odkazu není (nebo teoreticky být může s hodnotou 0).

@hrach

Jeste se mi nikdy nestalo, ze bych si stahl komponentu a pouzil bych ji tak, jak je. Stejne tak by se dalo argumentovat, ze komponenty maji vsude volat translator.

Moznost generovat odkazy bez parametru (respektive generovat parametry az v javascriptu) tedy resitelne je pridanim podminky pro nezahrnuti defaultnich parametru do hashe.

Diskuze by pak mohla byt "jen" o redirectu + pripadnou implementaci pro akce, nebo jeste neco?

Takže Davidův příklad by se řešil:

/**
 * @secured
 */
public function hadleDelete($postId, $confirmed = 0)
{
}

Je to tak?

@dg
Owner
dg commented

Vážně nikdo neví o frameworku, který tohle řeší? (Ani stydící se @hrach?) Chtělo by to totiž projít jejich security vulnerability reports a podle toho se zařídit, abychom neopakovali stejné chyby.

(Díval jsem na poslední Rails, Symfony, Django a tam jsem to nenašel)

@fprochazka

@dg já bych ještě chvilku počkal, nebo to napsal na Twitter. Koho jsem se ptal, tak ale neví. Všichni používají Nette ;)

@juzna

V zadnem jinem frameworku jsem to resene nevidel.

Kdyz jsem zkoumal treba GitHub, tak odkazy vedouci na akce (treba close issue) ma zAJAXovane a udelane pres post. Pokud odkaz provedete pres get (napr pri vypnutem JS, tak zkoncite na 404ce). Pokud udelate ajax post z jine domeny, tak se to taky neprovede, rekl bych ze se overuje pritomnost hlavicky Origin: kterou maji CORS ajaxovy requesty.

(Pozor, pri testovani CSRF v prohlizeci uvidite vzdycky error, protoze prohlizec vysledek zahodi neni-li pritomna nejaka magicka hlavicka. Ale na serveru se request zpracuje, neni-li nejak specialne osetren).

@juzna

Podle mě je ultimátní a 100% fungující ochrana proti CSRF neřešitelná, právě kvůli tomu jak HTTP protokol funguje. To ale není důvod ji neřešit vůbec. Běžně se používá princip vulnerability mitigation neboli snížení zranitelnosti - když mám bezpešnostní díry velké jako brána, dveře a okno, tak je raději zavřu a nechám otevřené jen malinkaté okénko do špajzu, do kterého se 90% záškodníkům nebude chtít lízt, a ze zbytku se jím 90% neprotáhne. Využije ho tak třeba méně než 1% záškodníků, což už je významný rozdíl a má důvod se tímto řešením zabývat.

Např. takový Suhosin je plugin do PHPka, který dělá právě vulnerability mitigation a podle mnoha diskusí má smysl (např. Debian řešil, zda ho mít v defaultu v distribuci, teď to řeší Ubuntu.)

U každého bezpečnostního rizika se musí brát v potaz dva faktory: velikost dopadu a pravděpodobnost využítí. Např. pravděpodobnost, že mi útočník přes Main-in-the-Middle útok ukradne token je zanedbatelné (při běžném webu). O trošku vyšší pravděpodobnost, ale stále zanedbatelnou, má únik tokenu přes referer hlavičku - to bych musel mít na svém webu např. obrázek z útočníkova webu (kdo by to dělal?) nebo odkaz na útočníkův web a uživatel by na něj musel kliknout. Samozřejmě že na některých webech má taková situace smysl, ale těch je minimum. Ve výsledku je tak velmi malá pravděpodobnost využití rizika, a to se stává zanedbatelným - bezpečnostní featura v 99% případů ochrání a tudíž má smysl.

@juzna

Django částečně ochranu řeší, a to pouze u ajaxových požadavů: přečte si token z cookies a přidá ho navíc do HTTP hlavičky. Útočník to není schopen realizovat, protože nedokáže cizí cookies číst (může pouze případnému XHR pomocí withCredentials říci, aby cookies poslal). Což je prakticky téměř to samé co tady navrhoval @dg.

@dg
Owner
dg commented

Pokud se nebavíme o tak dílčích jednotlivostech, jako je escapování řetězců, ale o komplexnějších úkolech, lze říct, že ultimativní a 100% fungující ochrana neexistuje proti ničemu. Bezpečnost proto není stav, ale míra. Tudíž snižování zranitelnosti je vyhovující postup.

Nicméně nelze podceňovat scénáře jako únik refereru přes hlavičku. Stačí mít blog a na něm komentáře, přihlášený uživatel může komentáře mazat (odkaz "Delete" vedle komentáře). Pokud bude možné do komentářů umisťovat obrázky nebo odkazy, je na problém zaděláno. Pokud se mi dostane do URL token, tak zobrazení obrázku nebo kliknutí na odkaz (což je standardní postup při kontrole cizích komentářů) token přenesou.

@hrach

Rebasnul jsem to a přidal kontrolu na přítomný redirect a nějaké další testy. Čekám na další připomínky a "komunitní vývoj".

@hrach

Open souce komunito, nejaka reakce?

@juzna

Stale jsme nedoresili pripominky, ktere mel @dg. Reseni je tedy takove polovicate.

Zkoumal jsem jine frameworky a sajty a nenasel jsem nikoho, kdo by to nejak poradne resil (viz moje drivejsi prispevky ohledne Django a GitHub; ale nejaky vycerpavajici pruzkum to nebyl).

@hrach

@juzna Ktere pripominky mas konkretne na mysli? Nevyzivam k tomu, abys je zminoval, ale spis implementoval :P

@juzna

@hrach nedoresene problemy:
1/ vyzrazeni tokenu - jak zminuje @dg, toto riziko tu existuje. Bud jej musime vyresit, nebo zanalyzovat a nasledne zduvodnit, proc jej neresit.
2/ ajaxove pozadavky - jak zminuje @dg, mely bychom verit pozadavkum sestavenym v javascriptu. Ty momentalne budou odmitnuty.

Dokud tyto problemy nebudou vyreseny (zodpovezeny), nema rebasovani nebo urgovani zadny vyznam ;)

@juzna

Ad problem vyzrazeni tokenu - ten by za normalnich okolnosti nemel nastat, pokud se programator zachova spravne a po provedeni akce udela presmerovani na URL bez tokenu. Pokud vsak nastane nejaka chyba a akci se nepovede provest, pak se presmerovani nekona, stranka s pripadnymy odkazy/obrazky se zobrazi token muze utect.

Reseni tohoto problemu nikoho nenapadlo ani jej nenasel. Sance zneuziti zde IMO neni zanedbatelna uplne, ale reseni nedokaze nikdo najit. Summary: zavedenim secured signalu se obrovske riziko snizi na hodne male. Reseni tedy ma cenu, avsak meli bychom upozornovat na mozny problem a stale hledat lepsi reseni.

@hrach

Samozrejme ze to vyznam ma. Protoze to nikdo jiny neudela.
1) Jinak nevim, jeslti sis vsiml, ze zakladni ochranu vyzrazeni tokenu jsem uz udelal.
2) to z principu reseni nema. A nefunkcni to taky neni, staci parametr uvest jako nepovinny. Coz je naprosto schudne pro nejaky confirm=1.

@juzna

Pokud provadis akci a ona selze, nemel bys presmerovat. Uzivatel ma zustat na stejne URL, aby mohl provest refresh a tim akci zopakovat. Vyhazovat v takovem pripade vyjimku je spatne.

Reseni ajaxu samozrejme mozne je. Pokud budeme mit pouze jeden token pro uzivatele, muzeme jej ulozit do cookies a ajax ho muze pridavat, at si vytvori parametry jake chce (jak to resi treba django, viz muj comment vyse). Mit jeden token pro session nebo jeden token pro konkretni akci uz zas takovy rozdil neni.

@hrach

"Pokud vsak nastane nejaka chyba a akci se nepovede provest"
Na tento usecase me napado jedno reseni: v pripade chyby (error presenter) smazat token v session.

@hrach

Nesouhlasim. V handle metode, ktera je get a jedna se o destruktivni akci, kterou je treba zabezpecit neni treba v pripade neuspechu zustavat na strance. Delete komentare? POST pozadavek je samo o sobe zabezpecen a tato vyjimka by se ho netykala. (Metoda neni oznacena @secured (ani nemuze, pac signal zpracova nette))

Upozornuji na dve fakta:

  • vyjimka je vyhozena jen pri pouziti secured A pritomnem tokenu. (tzn neni to zadne BC)
  • ajaxove zabezpeceni jsme nikdy nepotrebnovali pouzit na signaly.cz. A to je imo dle me uz velky projekt. Ruku na srdce - kdo z vas csrf resi u odkazu? Nebo nikdo nepouziva odkazy a vzdy pisete tovarnicku na form?
@juzna

Ja mam mazaci metody, ktere trvaji dyl a treba neco resi po siti. Obcas napisou "timeout expired"; clovek proste da F5 a zkusi to podruhe. To by mel radost, vracet se a hledat tu polozku znovu ;) Vyjimku bych tedy nevynucoval, pripadne jen nejaky warning co na produkci neshodi celou app.

To ze ty jsi to nepotreboval na signalech neznamena, ze ja nemam vic javascriptu na strance a neskladam si ajax requesty az na klientovi. Ale ok, ve sve aplikaci bych si prepsal metodu getCSRFToken a parametry tam nepouzival (i.e. velice jednoducha zmena)

@hrach

Prvni odstavec: nojo, ale to je problem trochu nekde jinde ;)
Druhy odstavec: jojo, to mas samozrejme pravdu.

@janci

V spojeni s tymto problemom mi napadla taka myslienka: Co keby sa v pripade vykonania metody s anotaciou @secured kontroloval referer ci ide odkaz ozaj z rovnakej domeny (stranky) na ktorej bol aj vygenerovany?

Akurat nie som si isty, ci nemoze nastat niektora situacia, kedy by bol referer nedefinovany. Chcelo by to asi komplexnejsiu analyzu. Ale mozno to aspon ako myslienka pomoze.

@jasir

Není náhodou možnost složit ajaxově dotaz právě proti logice ochrany parametrů před změnou uživatelem, k čemuž je imho tato vlastnost předurčena? Takový náš přihlášený uživatel hacker pak zjistí algoritmus generování linku v js a položí si tolik dotazů kolik bude chtít. Myslím tedy, že připomínka č.2 není relevantní, proto je také neřešitelná.

@jasir

A ještě k vyzrazení tokenu - uniklý token, svázaný na jednu akci, konkrétního uživatele, session a sadu parametrů, by útočníkovi dovolil právě tuto jednu jedinou akci, o kterou se uživatel pokusil. Je to opravdu riziko?

@hrach

Toz, rebasnul jsem kod na master, mohlo by se s tim zase po par mesicih pohnout.

@travisbot

This pull request passes (merged 2325a427 into d24962d).

@vojtech-dobes vojtech-dobes referenced this pull request from a commit in vojtech-dobes/nette.ajax.js
@vojtech-dobes vojtech-dobes Added support for CSRF protection (inspired by nette/nette#469) 04db0a2
@hrach hrach referenced this pull request
Closed

Database - Row Factory #805

@hrach

Tak, je to rok a 3 dny, co jsem udelal tento pullrequest. Dnes je ten slavny den:
Secured signaly pro každého s PHP 5.4+: https://github.com/nextras/application

@Vrtak-CZ

@hrach být tebou tak tam napíšu PHP 5.4+ někteří lidé to z examplu neodvodí a budou tě zbytečně týrat že jim to nefunguje...

@hrach

Díky za feedback, pridal jsem to sem do komentare i do raadme. Treba to nekoho donuti upgradovat, bude jenom dobre :)

@f3l1x

Je hruza jak se to nehejbe. Tohle uz melo v nette davno byt. Porad se jen resi BC break/nonBC break... Ja bych je tam nahazel, vsak oni si lidi zvyknou :smile:

@hrach hrach referenced this pull request from a commit in hrach/nette
@hrach hrach Database tests: removed redundant tests b662d49
@fabik

The RFC 2616, section 9.1.1 states:

In particular, the convention has been established that the GET and HEAD methods SHOULD NOT have the significance of taking an action other than retrieval. These methods ought to be considered "safe". This allows user agents to represent other methods, such as POST, PUT and DELETE, in a special way, so that the user is made aware of the fact that a possibly unsafe action is being requested.

So we should definitely use POST for secured signals (and ideally for all signals with side effects).

So what about adding a new JavaScript to Nette (e.g. netteSecured.js), that would send links like <a n:href="signal!" data-nette-csrf-token="{$token}"> using POST? We can also easily create a fallback method for users without JavaScript - if an user attempts to access a secured page using GET method, we will show him a confirmation dialog "Are you sure to delete XYZ?" and the Yes button in the dialog will execute the request with the CSRF token in POST param.

If you want to send signals using AJAX, you can just modify your JavaScript, so that it will send the token via POST.

I've implemented a proof of this concept and it seems to work great: https://fabik.org/sandbox-with-secured-signals/ (source code)

The syntax could look like <a n:href="signal!" n:secured>. (the n:secured macro will add the data-nette-csrf-token attribute). And the signal could be secured by adding $this->addProtection('Are you sure to delete XYZ?'); at the begging of the handle method or by adding an annotation @secured [optional question].

We should also figure out some way, how to customize the dialog. We can for example add a displayCsrfDialog($question) method to the Presenter, that would display the form. So the form can be easily customized by overriding this method.

I think that it would be great to include something like this in Nette. Do you think so?

If you agree, I will prepare a pull request. But I will need someone's help with making the netteSecured.js independent on jQuery.

@mishak87

Not good enough. It can actually allow for faster Breach Attack since are generated once from predictable input and only 8 characters of weak hashing function. I would suggest adding fatal error to turn off compression (gzip or deflate) on secured connection.

They should be different for each generated link.
Cannot be used twice (replay attack).
Have request rate limit (DDoS or guessing). Breach works better with more requests.
Any solution must mask argument and wrap token (or rather whole response) in garbage (random data).

Using current solution is good for security in 2010. First draft of this attack using compression in TSL was done around 1995 so NSA already knows ;)

TL;DR Sending token in response is like leaving hints to decode SSL communication. Does not matter it can't be guessed it is used for breaking the encryption it self and tokens help speedup the process.

@fabik

@mishak87 That's a goot point. Well, I think there exists a simple protection against this: We will split the token into two parts (A and B). The part A will be always a new random string of the same length as the token. The part B will be XOR of the part A and the token. (We can easily reconstruct the token as a XOR of the parts A and B.) So the attacker will not be able to find the CSRF token, because the parts A and B will be completely random strings on each request.

Btw. here's a nice explanation of the attack. ;)

@fabik fabik referenced this pull request from a commit in fabik/nette
@fabik fabik Application: added CSRF protection for signals [Closes #469] 7c7434c
@fabik fabik referenced this pull request from a commit in fabik/nette
@fabik fabik Application: added CSRF protection for signals [Closes #469] 1438512
@fabik fabik referenced this pull request from a commit in fabik/nette
@fabik fabik Application: added CSRF protection for signals [Closes #469] 52454ec
@fabik fabik referenced this pull request from a commit in fabik/nette
@fabik fabik Application: added CSRF protection for signals [Closes #469] 44ab451
@dg
Owner
dg commented

Už tu máme dvě issues. „A není to málo, Antone Pavloviči?“

Co říkáte na následující postup, který by byl implementačně snadný a univerzální.

Low level ochrana CSRF

  • Nette bude automaticky odesílat token v cookie __csrfs platností 0 (do konce sezení)
  • všechny POST formuláře budou vždy odesílat skryté pole __csrf s tímto tokenem + anti-breach maskováním
  • pokud nedojde ke spárování na úrovni RequestFactory, zobrazí se rovnou chyba 403
  • ochrana bude defaultně zapnutá, bude ji možné vypnout v configu (a do dokumentace nenapíšeme jak ;-)

Výhody:

  • zajistíme automatickou ochranu pro všechny POST formuláře, bez nutnosti volat addProtection
  • nebude to záviset na session
  • současná chybová hláška (Please submit this form again) vlastně nahrává útočníkovi
  • token v cookie (oproti třeba elementu META) nevyžaduje měnit šablony a je odolný vůči breach attack

Nevýhody:

  • manuálně vykreslené formuláře bez použití Latte nebudou fungovat (</form> i {/form} v Latte automaticky vykreslí všechny HiddenField, týká se to tedy hodně specifických případů)
  • větší náchylnost k vyzrazení tokenu při XSS, protože stačí udělat XSS i na stránce, kde není formulář.

Chráněné odkazy v Presenteru

Aktivace:

  • ochrana bude fungovat pro akce i signály a bude se aktivovat anotací u příslušné metody
  • bude možnost aktivovat ochranu pro celý presenter či komponentu v anotaci třídy
  • bude možné aktivovat ochranu pro jednotlivou komponentu presenteru, podobně, jako se určují persistentní komponenty
  • API zjišťující, zda cíl je chráněný, bude napsáno obecněji, aby jej bylo možné výhledově použít na ověřování i jiných anotací

Princip ochrany:

  • chráněné odkazy dostanou prázdný HTML atribut data-nette-secure
  • pomocí JavaScriptu budou tyto odkazy odesílány jako POST formulář na stejné URL s polem __csrf
  • výchozí implementace nebude využívat jQuery a bude kompatibilní IE8+
  • kontrola tokenu bude řešena již na úrovni RequestFactory
  • Presenter bude kontrolovat, že odkaz na chráněný cíl je odeslán jinou metodou než GET/HEAD, jinak 403

AJAX:

  • chráněný odkaz může být odeslán i AJAXem metodou PUT a DELETE a token se pak předá v X-CSRF-Token
  • to znamená, že RequestFactory bude kromě POST elementu __csrf kontrolovat i hlavičku X-CSRF-Token
@fprochazka

manuálně vykreslené formuláře bez použití Latte maker nebudou fungovat

Tam je jenom potřeba vypsat hiddeny, ne?

@dg
Owner
dg commented

Těžce základní nástřel low level ochrany dg@33fda30

@Vrtak-CZ

A co na to @spaze

@Vrtak-CZ

Bude možné tuhle ochranu vypnout pro jednotlivé "routy" nebo jenom globálně? (Přemýšlím nad potencionálníma problémama s "REST" api které nemusí umět sušenky)

@pavelkouril

manuálně vykreslené formuláře bez použití Latte maker nebudou fungovat

IMHO a huge deal breaker.

@dg
Owner
dg commented

Dobré je, že se to dá řešit postupně.

V první fázi by se mohla implementovat ta low-level ochrana a přizpůsobit jí formuláře. Kvůli kompatibilitě by se u požadavků s hlavičkou X-Requested-With nekontroloval X-CSRF-Token (obecně lze AJAXové požadavky považovat za bezpečné).

V druhé fázi by se připravila kontrola HTTP metody a přidávání atributů data-nette-secure v Presenteru.

A nakonec by se mohlo řešit chránění AJAXových požadavků.

@spaze

Nevýhoda ochrany CSRF "snadnější XSS" by šla trochu zmenšit pomocí označení té CSRF cookie (konfigurovatelným) příznakem HttpOnly.

@fabik

Pak by se ale musel změnit ten JavaScript pro posílání přes AJAX. Mohl by se mu třeba předávat ten token přes nějaký data atribut (tady by musel být doplněn ještě o ochranu proti BREACH útoku).

@dg
Owner
dg commented

Jaký je rozdíl, jestli token vytáhnu z cookie nebo DOM?

@fabik

Pokud bude mí ta cookie příznak HttpOnly, tak ji nevytáhnu. (Reagoval jsem na komentář nade mnou.)

@spaze

@dg, jestli jsem to pochopil správně, tak JS ten token nebude potřebovat na každé stránce, zatímco cookie je z JS dostupná na každé stránce a jak sám píšeš pak stačí udělat XSS i na stránce, kde není formulář.

@dg dg removed this from the 2.2 milestone
@hrach hrach closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 8, 2014
  1. @JanTvrdik @hrach

    Added secured signals

    JanTvrdik authored hrach committed
  2. @hrach
This page is out of date. Refresh to see the latest.
View
36 Nette/Application/UI/Presenter.php
@@ -43,6 +43,7 @@
const SIGNAL_KEY = 'do',
ACTION_KEY = 'action',
FLASH_KEY = '_fid',
+ CSRF_TOKEN_KEY = '_sec',
DEFAULT_ACTION = 'default';
/** @var int */
@@ -872,6 +873,20 @@ protected function createRequest($component, $destination, array $args, $mode)
if ($args) { // convert indexed parameters to named
self::argsToParams(get_class($component), $method, $args);
}
+
+ // secured signals
+ $signalReflection = $reflection->getMethod($method);
+ if ($signalReflection->hasAnnotation('secured')) {
+ $signalParams = array();
+ if ($args) {
+ foreach ($signalReflection->getParameters() as $param) {
+ if (isset($args[$param->name])) {
+ $signalParams[$param->name] = $args[$param->name];
+ }
+ }
+ }
+ $args[self::CSRF_TOKEN_KEY] = $this->getCsrfToken(get_class($component), $method, $signalParams);
+ }
}
// counterpart of IStatePersistent
@@ -989,6 +1004,27 @@ protected function createRequest($component, $destination, array $args, $mode)
/**
+ * Returns unique token for method and params
+ * @param string
+ * @param string
+ * @param array
+ * @return string
+ */
+ protected function getCsrfToken($control, $method, $params)
+ {
+ $session = $this->getSession('Nette.Application.UI.Presenter/CSRF');
+ if (!isset($session->token)) {
+ $session->token = Nette\Utils\Strings::random();
+ }
+
+ $params = Nette\Utils\Arrays::flatten($params);
+ $params = implode('|', array_keys($params)) . '|' . implode('|', array_values($params));
+ return substr(md5($control . $method . $params . $session->token), 0, 8);
+ }
+
+
+
+ /**
* Converts list of arguments to named parameters.
* @param string class name
* @param string method name
View
25 Nette/Application/UI/PresenterComponent.php
@@ -248,14 +248,35 @@ public static function getPersistentParams()
* Calls signal handler method.
* @param string
* @return void
- * @throws BadSignalException if there is not handler method
+ * @throws BadSignalException if there is not handler method or security token does not match
+ * @throws \RuntimeException if there is no redirect in a secured signal
*/
public function signalReceived($signal)
{
- if (!$this->tryCall($this->formatSignalMethod($signal), $this->params)) {
+ $method = $this->formatSignalMethod($signal);
+ $reflection = new Nette\Reflection\Method($this, $method);
+ if ($reflection->hasAnnotation('secured')) {
+ $params = array();
+ if ($this->params) {
+ foreach ($reflection->getParameters() as $param) {
+ if (isset($this->params[$param->name])) {
+ $params[$param->name] = $this->params[$param->name];
+ }
+ }
+ }
+ if (!isset($this->params[Presenter::CSRF_TOKEN_KEY]) || $this->params[Presenter::CSRF_TOKEN_KEY] !== $this->getPresenter()->getCsrfToken(get_class($this), $method, $params)) {
+ throw new BadSignalException("Invalid security token for signal '$signal' in class {$this->reflection->name}.");
+ }
+ }
+
+ if (!$this->tryCall($method, $this->params)) {
$class = get_class($this);
throw new BadSignalException("There is no handler for signal '$signal' in class $class.");
}
+
+ if (isset($this->params[Presenter::CSRF_TOKEN_KEY])) {
+ throw new \RuntimeException("Secured signal '$signal' did not redirect. Possible csrf-token reveal by http referer header.");
+ }
}
View
42 tests/Nette/Application.UI/Presenter.link().phpt
@@ -28,6 +28,16 @@ class TestControl extends Application\UI\Control
}
+
+ /**
+ * @secured
+ */
+ public function handlePay($amount = 0)
+ {
+ }
+
+
+
/**
* Loads params
* @param array
@@ -75,6 +85,14 @@ class TestPresenter extends Application\UI\Presenter
}
+ protected function getCsrfToken($control, $method, $params)
+ {
+ $params = Nette\Utils\Arrays::flatten($params);
+ $params = implode('|', array_keys($params)) . '|' . implode('|', array_values($params));
+ return substr(md5($control . $method . $params . 'abcd'), 0, 8);
+ }
+
+
protected function startup()
{
parent::startup();
@@ -112,6 +130,10 @@ class TestPresenter extends Application\UI\Presenter
Assert::same( '/index.php?action=default&presenter=Test', $this->link('this', array('var1' => $this->var1)) );
Assert::same( '/index.php?action=default&presenter=Test', $this->link('this!', array('var1' => $this->var1)) );
Assert::same( '/index.php?sort%5By%5D%5Basc%5D=1&action=default&presenter=Test', $this->link('this', array('sort' => array('y' => array('asc' => TRUE)))) );
+ Assert::same( '/index.php?_sec=6e8ac795&action=default&do=pay&presenter=Test', $this->link('pay!') );
+ Assert::same( '/index.php?amount=200&_sec=a0d9cd16&action=default&do=pay&presenter=Test', $this->link('pay!', 200) );
+ Assert::same( '/index.php?sections[0]=a&sections[1]=b&_sec=9d49cae9&action=default&do=list&presenter=Test', urldecode($this->link('list!', array(array('a', 'b')))) );
+ Assert::same( '/index.php?sections[0]=a&sections[1]=c&_sec=20e15718&action=default&do=list&presenter=Test', urldecode($this->link('list!', array(array('a', 'c')))) );
// Presenter & signal link type checking
Assert::same( "error: Invalid value for parameter 'x' in method TestPresenter::handlebuy(), expected integer.", $this->link('buy!', array(array())) );
@@ -130,6 +152,8 @@ class TestPresenter extends Application\UI\Presenter
Assert::same( '/index.php?mycontrol-x=1&mycontrol-round=1&action=default&presenter=Test', $this['mycontrol']->link('this', array('x' => 1, 'round' => 1)) );
Assert::same( '/index.php?mycontrol-x=1&mycontrol-round=1&action=default&presenter=Test', $this['mycontrol']->link('this?x=1&round=1') );
Assert::same( '/index.php?mycontrol-x=1&mycontrol-round=1&action=default&presenter=Test#frag', $this['mycontrol']->link('this?x=1&round=1#frag') );
+ Assert::same( '/index.php?mycontrol-_sec=97dc7d4e&action=default&do=mycontrol-pay&presenter=Test', $this['mycontrol']->link('pay') );
+ Assert::same( '/index.php?mycontrol-amount=200&mycontrol-_sec=32d45373&action=default&do=mycontrol-pay&presenter=Test', $this['mycontrol']->link('pay', 200) );
Assert::same( 'http://localhost/index.php?mycontrol-x=1&mycontrol-round=1&action=default&presenter=Test#frag', $this['mycontrol']->link('//this?x=1&round=1#frag') );
// Component link type checking
@@ -148,6 +172,24 @@ class TestPresenter extends Application\UI\Presenter
{
}
+
+
+ /**
+ * @secured
+ */
+ public function handlePay($amount = 0)
+ {
+ }
+
+
+
+ /**
+ * @secured
+ */
+ public function handleList(array $sections)
+ {
+ }
+
}
View
59 tests/Nette/Application.UI/PresenterComponent.signalReceived().phpt
@@ -0,0 +1,59 @@
+<?php
+
+/**
+ * Test: Nette\Application\UI\PresenterComponent and secured signals.
+ *
+ * @author Jan Skrasek
+ * @package Nette\Application\UI
+ * @subpackage UnitTests
+ */
+
+use Nette\Http;
+use Nette\Application;
+use Tester\Assert;
+
+require __DIR__ . '/../bootstrap.php';
+
+
+class TestPresenter extends Application\UI\Presenter
+{
+ /**
+ * @secured
+ */
+ function handleDelete()
+ {
+ }
+ function handleEdit()
+ {
+ $this->redirectUrl('http://example.com');
+ }
+ function getCsrfToken($control, $method, $params)
+ {
+ return 'hash';
+ }
+ function renderDefault()
+ {
+ }
+}
+
+
+$container = id(new Nette\Configurator)->setTempDirectory(TEMP_DIR)->createContainer();
+$presenter = new TestPresenter;
+$container->callMethod($presenter->injectPrimary);
+
+
+Assert::throws(function() use ($presenter) {
+ $request = new Application\Request('Test', Http\Request::GET, array('action' => NULL, 'do' => 'delete'));
+ $presenter->run($request);
+}, 'Nette\Application\UI\BadSignalException', "Invalid security token for signal 'delete' in class TestPresenter.");
+
+
+Assert::throws(function() use ($presenter) {
+ $request = new Application\Request('Test', Http\Request::GET, array('action' => NULL, 'do' => 'delete', '_sec' => 'hash'));
+ $presenter->run($request);
+}, 'RuntimeException', "Secured signal 'delete' did not redirect. Possible csrf-token reveal by http referer header.");
+
+
+$request = new Application\Request('Test', Http\Request::GET, array('action' => NULL, 'do' => 'edit', '_sec' => 'hash'));
+$response = $presenter->run($request);
+Assert::true($response instanceof Nette\Application\Responses\RedirectResponse);
Something went wrong with that request. Please try again.