Marzec

qertoip: poziomy izolacji transakcji?

drogus: debugger?

Tomash: Deface?

Tradycyjny fejsik: https://www.facebook.com/events/247061535381920/

Mam nadzieję że tym razem nie będę zagranico :wink:

picie w bramie > picie zagranico. #pamiętamy

YES

Skoro mamy w tym miesiącu nasycenie i silną konkurencję w postaci warsztatów Vimcasts, moją prezentację Vimową możemy przełożyć na przyszłe, mniej obłożone terminy.

Do interesujących tematów bieżących dochodzi ostatnie zamieszanie z konsekwencjami braku użycia attr_accessible. Może ktoś miałby ochotę zrobić podsumowanie tematu?

Jako, że uczestniczyłem w dyskusji od początku to mógłbym powiedzieć kilka słów, chociaż jak czytam większość komentarzy to mi ręce opadają.

Jak dla mnie jedna wielka farsa ;].

Railsy pewnie od początku mają to rozwiązane w taki sposób. Nagle ktoś zwraca uwagę na to, że “hej, to może prowadzić do błędu”, bo programiści z githuba dają dupy i super, można się zastanowić nad tym jak to poprawić.

Tylko, że: Mamy poniedziałek, a w weekend i przed samym weekendem kolo, który to zauważył zachowuje się jak szczyl ze sraczką i troluje to tu, to tam. Brak natychmiastowej reakcji zostaje ogólnie podłapany przez różnych ludzi, którzy losowo wypowiadają się na temat tego jakie railsy są złe i jak należy to jak najszybciej poprawić.

Mało kto z takich ludzi zastanawia się nad tym, że to pomoże jedynie nowym aplikacjom. Istniejące, jeśli developerzy dali dupy (nie bójmy się tego powiedzieć) i tak będą musiały być przeorane, jeśli dopiero teraz zauważyli “o ja, to jest coś takiego jak mass assingment?”.

Jak to mówią, psy szczekają, karawana idzie dalej :wink:

Łukasz, ja też popieram rails-core w większości ich decyzji (nawet w szitstormie o asset pipeline), ale tutaj racja nie jest po ich stronie.

Gość zgłosił to do issues w Railsach w piątek, został olany a issue zamknięty z komentarzem że dobrzy developerzy są na to odporni. Kto ma w środowisku railsowym najlepszych developerów? Github. Jest odporny? Nie. Co śmieszniejsze*, w najważniejszym chyba od strony bezpieczeństwa ficzerze, czyli przypisywaniu klucza ssh do konta.

Igor w jeden dzień pokazał, że ostatnie kilka lat miziania się po pytkach podejściem “nah, przecież napisaliśmy w dokumentacji, każdy sensowny programista użyje attr_accessible” było błędem. Skoro robi to każdy dobry programista, to czemu nie jest defaultem (jak np. tokeny w formularzach)? To było złe i głupie, co było do udowodnienia.

Więc zamiast nazywać go trolem sprawdź lepiej czy Twoje aplikacje są odporne na podobnego exploita.

    • to nie jest śmieszne kiedy sobie wyobrazić że na takiego exploita wpada jakiś blackhat i sprzedaje to na czarnym rynku; procesy jakie płacący klienci Githuba wytoczyliby za wyciek prywatnych repozytoriów mogłyby zmusić Githuba do bankructwa.

Jak najbardziej zgadzam z tym, że to należy zmienić (napisałem “super, można się zastanowić nad tym jak to poprawić”).

Ja bardziej się czepiam sposobu eksponowania tego problemu. To nie jest kwestia buga, którego można poprawić w istniejących aplikacjach fixem, a core team się wypina i mówi nie. To nie jest kwestia życia i śmierci, żeby skierować całą uwagę na zmianę ustawienia, które będzie wykorzystywane w nowych aplikacjach i to dopiero od którejś z najbliższych wersji (co oznacza, że i tak wszyscy zaczynający na przykad od 3.1 też wejdą w to samo bagno).

I z tego co wiem, teorie spiskowe na temat zamykania ticketów, żeby problem zignorować też są mocno przesadzone.

Podsumowując - problem jest zdecydowanie warty uwagi, ale drama, która wokół niego powstała, zupełnie niesmaczna.

Sytuacja wyglądała mniej więcej tak:

  • homakov zgłosił issue i zaproponował tam domyślne whitelistowanie, przez chwilę toczyła się dyskusja (oprócz tego proponował też inne rzeczy, ale były lekko mówiąc kiepskimi pomysłami)
  • homakov pokazał, że github jest podatny na atak i w tym momencie powiadomiliśmy githuba (on z tego co mówił też to zrobił)
  • fxn zamknął to issue (co imho było błędem), ale dyskusja toczyła się dalej
  • zaproponowałem jako “minimum” generowanie dokumentacji (ale prywatnie “lobbowałem” za tym, żeby zrobić coś więcej, później wytłumaczę problemy z tym wyjściem, które zaproponował homakov)
  • homakov odpisał, że ok, to jest minimum, które go satysfakconuje: https://github.com/rails/rails/issues/5228#issuecomment-4304367

I teraz najlepsze. Pierwsze issue, które homakov znalazł zostało zgłoszone githubowi w piątek. Zamknąłem tamtego ticketa, który był “shackowany”, bo a) sam w sobie nie wnosił nic do dyskusji i b) stwierdziłem, że dopóki to nie będzie naprawione przez githuba lepiej, żeby wszyscy o tym nie wiedzieli. I teraz nasz wspaniały bohater zamiast dać chociaż trochę czasu na review całej aplikacji i poprawienie błędów, nie mógł wytrzymać i koniecznie musiał zrobić totalny shitstorm. Przypominam, że github, to dość duża aplikacja, więc jej review i poprawienie błędów może trochę zająć, a był weekend. I za to homakov został chwilowo zbanowany. Na miejscu githuba zrobilibyście inaczej? Ktoś wam zgłasza problem i zamiast poczekać jak człowiek chociaż na koniec weekendu zaczyna dalej buszować po aplikacji - niby można założyć, że skoro robi to publicznie, to nie chce tego wykorzystać, ale jaką mogą mieć pewność tak naprawdę? Lepiej ryzykować, że sprawdzał to też na kontach prywatnych?

No ale dobra, shitstorm już się zaczął. I teraz 90% ludzi, którzy to czytają, nie mają zupełnie pojęcia o co chodzi. Połowa osób z tego co widzę uważa, że to jest coś co railsy mogły załatwić jakimś quick fixem, który automatycznie naprawi githuba i inne podatne na atak aplikacje. Część ludzi nawet po zobaczeniu “fixa” scommitowanego przez koza myśli, że to nagle naprawi ich aplikacje i się pytają czy powinni od razu zupdate’ować (podpowiadam, że jest to kod dodany do generatorów).

Przy okazji, na takie ataki jest podatny każdy framework, który ma mass asignment. Merb miał to samo, sinatra będzie miała to samo jak używa się paramsów, klony railsów w nodzie i php też dokładnie to samo.

I teraz najważniejsza rzecz. Duża część osób twierdzi, że filtrowanie atrybutów powinno się odbywać w kontrolerach i to z dobrych powodów. Oczywiście homakov wie lepiej ( https://github.com/rails/rails/issues/5228#issuecomment-4327289 ). Ale na dłuższą metę trzymanie się attr_accessible w modelu jest złym pomysłem. Dla nowych aplikacji w 3.2.x, dopóki nie będzie lepszej odpowiedzi, to będzie ok, bo developerzy będą od początku na to uważali, ale to jest krótkowzroczne. Bardzo fajnie, że zwrócono na to uwagę, bo do tej pory nie było żadnej dyskusji, ale robiąc automatyczne whitelistowanie nie ma już dalej możliwości wprowadzenia automatyki. Jeżeli zmieni się koncepcję, żeby robić to na poziomie kontrolerów, to np. można automatycznie sprawdzać jakie pola zostały wygenerowane na formach i zrobić coś w stylu CSRF tokena, który później wyczyści wszystko czego nie było na formie. Wtedy w przypadku aplikacji HTMLowych nie trzeba będzie prawie nic robić. Jeżeli chodzi o API, to można paramsy oznaczać jako “tainted” i dopóki nie zostaną oznaczone jako safe albo przefiltrowane, to nie mogą zostać użyte przy mass assignment. Ale lepiej oczywiście tupnąć nóżką, zrobić shitstorm, wciągnąć w to setki ludzi, którzy nie mają pojęcia o czym piszą i ogólnie zrobić z tego wielką szopkę.

Zgadzam się z tym, że dobrze, że ta dyskusja się rozpoczęła, ale sposób w jaki zachowuje się homakov jest dla mnie kompletnie bez sensu.

UPDATE:

Tak jeszcze doprecyzuję, bo może zbyt mocno tego nie napisałem - zgadzam się z tym, że to powinno być defaultem już od dawna i w 3.2.x to się przyda, ale proponuję też poczytać to: http://news.ycombinator.com/item?id=3664334. Nie wiem czy się dobrze wyrażam, ale chodzi mi o to, że nawet takie rzeczy wymagają jakiejś dyskusji i nawet jeżeli to ma potrwać kilka dni, to trzeba chociaż trochę cierpliwości. Skoro i tak nie dało się naprawić w automatyczny sposób obecnie działających aplikacji, to nie wiem skąd takie ciśnienie, żeby to zrobić JUŻ TERAZ!!!111111oneoneone

Właśnie o takich idiotach pisałem: https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57#commitcomment-1046261 Ci goście w większości nie odróżniają railsów od githuba i myślą, że railsy zignorowały dziurę w railsach. Kiepskie domyślne ustawienia, to nie dziura, ale co z tego, wystarczy przeczytać nagłówek i skomentować.

Ano, czyli rails-core zademonstrował że nie uważa tego za issue.
Czyli należało im pokazać że się dramatycznie mylą.

Jeśli znajduje się takiego babola (rozmawiamy wciąż o podmianie klucza ssh do repozytorium!), to nie ma że weekend, wieczór, noc albo cokolwiek innego. Sorry. Github dał dupy w całej tej historii, do czego się zresztą przyznali. Zachowali się tak, że teraz każdy haker od zabezpieczeń zastanowi się dwa razy zanim im coś przekaże.

W jakiej warstwie frameworka nie było to rozwiązywane, railsowe podejście “enabled by default” jest po prostu złe. Teraz to się czepiasz podrzędnych detali.

Drogomir, ja rozumiem że się lubisz (ja też!) z ludźmi z rails-core, a może nawet identyfikujesz. Ale tu nie ma czego bronić, ta sprawa została technicznie i wizerunkowo przegrana przez Railsy i Githuba. Issue został zbagatelizowany przez rails-core, a potem demonstracja dlaczego nie powinien być bagatelizowany spotkała się z negatywną reakcją serwisu, któremu Igor zrobił dużą przysługę whitehatując*. Akurat z zamieszanych w tę aferę chłopak (18 lat!) ma najmniej sobie do zarzucenia.

    • wciąż mówimy o uratowaniu od procesów o grube miliony gdyby tę dziurę znalazł blackhat i sprzedał

Ale oni przecież jeszcze w weekend to poprawili… tylko, że Igor jest w gorącej wodzie kompany. Naprawdę nie mógł usiedzieć na dupie i dać im troszkę więcej czasu na przeoranie całego serwisu? ;]

Nie wiem czemu tak usilnie stara się to przedstawić tylko i wyłącznie w czarno-białych barwach. Że cały rails core-team się wypiął nawet nie dyskutując o tym i gdyby nie heroiczna walka Igora, to nikt nie kiwnąłby palcem. Teraz się o tym nie przekonamy, ale to są mimo wszystko sensowni ludzie a to jest kwestia domyślnych ustawień. Seriously - po tylu latach nagle to jest takie mega ważne, żeby to przepchnąć w ciągu jednego weekendu, bo github dał dupy?

Jak dla mnie cały czas się rozchodzi o to, w jaki sposób, na szybko chciał przepchnąć swoją wizję. Ja się zgadzam, że to trzeba zmienić, ale też przypominam, że to nie zmienia absolutnie nic w już istniejących dziurawych aplikacjach railsowych. W ogóle część osób ciągle nie rozumie, że to nie jest bug, który da się poprawić w jakikolwiek sposób, nie ruszając aplikacji (padały niedorzeczne pytania - jak github, to poprawił, czy spatchował railsy?).

I tutaj się mylisz. On tego drugiego babola (z kluczem) nie zgłosił githubowi tylko od razu wykorzystał. Zgłosił tylko pierwszego, dosyć niegroźnego w issues. Dlatego według mnie zachował się jak gówniarz.

Tak, bo w jeden dzień na przykładzie Githuba zinwalidowano argument “dobrym programistom to się nie zdarza”. Otóż wygląda na to, że zdarza się najlepszym. Ja wiem że przyznanie się do kiepskiego podejścia do sprawy przez kilka lat boli i budzi opór, ale sorry – trzeba się uczyć. Na błędach też. A nie karać osobę, która nam życzliwie pokazuje, że robiliśmy źle.

Seriously, wszyscy płaczą “omg, czemu nam tego po cichu i w sekrecie nie powiedział, żebyśmy mogli to naprawić”. Przecież powiedział. Dał trzy dni. Przez rails-core został olany, więc pokazał jak bardzo się mylą.

Blackhat jeśli znajdzie exploita w Twoim serwisie nie daje Ci nawet jednego dnia. Znaj proporcją.

Ja wiem że mógł zrobić to po cichu, github by sobie spatchował aplikację, ale wtedy wszystkie pozostałe aplikacje railsowe pozostałyby na to podatne w błogiej nieświadomości. Ja tam jestem Igorowi wdzięczny że zaalarmował całe środowisko.

Ależ oczywiście że się da, jedną linijką w initializerze.

Tylko że wtedy prawdopodobnie posypie się aplikacja (działająca tylko dlatego, że miałeś mass-assignment wszystkiego włączony).

Tak, bo w jeden dzień na przykładzie Githuba zinwalidowano argument “dobrym programistom to się nie zdarza”. Otóż wygląda na to, że zdarza się najlepszym. Ja wiem że przyznanie się do kiepskiego podejścia do sprawy przez kilka lat boli i budzi opór, ale sorry – trzeba się uczyć. Na błędach też. A nie karać osobę, która nam życzliwie pokazuje, że robiliśmy źle.[/quote]
Życzyliwie? Ja podtrzymuję to, że według mnie zachował się jak gówniarz, bo mógł to spokojnie zrobić jak zrównoważony psychicznie człowiek, np. tak jak koleś, który znalazł mega dziurę w heroku, przez którą można było dostać dostęp do kodu aplikacji. Jeżeli chciał chwilę fame’u, to mógłby już po naprawieniu tej dziury przez githuba zrobić artykuł o tym jak tego dokonał i że to bylo możliwe, tak czy siak wpadłoby na hacker news i wszystkie inne reddity, developerzy by się dowiedzieli. Ale lepiej zrobić shitstorm, przez który teraz połowa ludzi w ogóle nie wie o co chodzi i dissuje wszystko dookoła.

Problem jest w tym, że imho on tego nie zrobił dlatego, że go zignorował core team (bo dalej była dyskusja i napisał, że chociaż wygenerowane komentarze są dla niego ok), tylko dlatego, że znalazł poważną dziurę i tak się tym podniecił, że nie mógł wytrzymać ani minuty dłużej tylko ją wykorzystać bez powiadomienia o niej githuba wcześniej.

Ależ ja ciągle się zgadzam, że to podejście jest niebezpieczne i trzeba je zmienić. I nie neguje tego ;). Tylko też nie rozumiem pośpiechu w chęci wprowadzenia samej zmiany.

Rzeczywiście, to będzie naprawiona ;). Nie łap za słówka! :> Mi chodzi o pełne załatanie, żeby test suite był ciągle zieloniutki :slight_smile:

UPDATE:

Zwróć uwagę na post Piotrka, kilka wyżej. http://rubyonrails.pl/forum/p33770-Dzisiaj-05%3A08%3A34#p33770
To jest moim zdaniem mocno pomijana sprawa…

Fajnie, że to znalazł, ale zachował się jak gówniarz niestety. Nie wiem czy właściwe jest kontrastowanie tego z faktem, że nie zachował się jak skurwiel i info nie sprzedał.

No więc nie wiem jak Ty, ale ja uważam że znajdując takiego exploita trzeba walić w dzwony i robić alarm, a nie wynajdować wszystkie aplikacje railsowe na świecie i wysyłać grzecznego maila “czy moglibyście to sprawdzić i załatać u siebie” do maintainera każdego z nich. Jestem wdzięczny Igorowi.

Że mógł to rozegrać lepiej – być może mógł. Ale on ma 18 lat, for fuck’s sake!

Nie chodzi o to, żeby to wyciszyć i wysyłać maile do developerów, i tak to by się na maksa rozeszło. Chodzi o to, żeby dać serwisowi czas na odpowiedź.

No właśnie - gówniarz jeszcze z niego :wink: (nie obrażając innych 18 latków :stuck_out_tongue: )