Starring / follow repo geeft CSP error #26
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Prio - Hoog
Prio - Laag
Prio - Middel
styling
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
MinBZK/Codeplatform#26
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Over CSP, momenteel staat die zelfs iets 'te' streng voor Forgejo. BIj het starren of volgen van repo's krijg je deze melding:
Originally posted by @wouter.kobes in #11 (comment)
Hoewel de 'star' zelf wel wordt opgeteld in zowel backend als frontend.
codeberg.org/forgejo/forgejo@!12247 (commit
d72f5952a8)JavaScript error: call to Function() blocked by CSP (https://code.overheid.nl/assets/js/index.js?v=16.0.0-dev-235-d72f5952a8~gitea-1.22.0 @ 12:66518). Open browser console to see more details.
(Un)follow klaagt nog. Star/Unstar werkt correct voor zover ik kan zien. Ik ga dat oppakken.
Het ging vandaag nog even over de CSP fix, en of we die moeten blijven hacken, of juist uitleggen dat we momenteel niet voldoen aan de CSP eisen van Internet.nl omdat de upstream dat nog niet geaccepteerd heeft, in een soort "complience debt overzicht" (ik hoop dat ik het zo goed zeg)
codeberg.org/underdarknl/forgejo@2dbe4d5f10Beide routes zijn mogelijk, maar gezien de complexiteit van de software, en de in mijn optiek vrij grote kans op XSS (er gebeurt veel, en er is veel 'user-supplied' input en content) is het denk ik verstandig om CSP wel strikt te houden.
Laten we focussen op het upstreamen van deze fixes, dat brengt ons ook in het juiste vaarwater voor wat betreft de aanhaking bij het project zelf, en voor eventuele toekomstige issues die we willen fixen of gefixt zien.
Het risico op XSS lijkt mij beperkt en acceptabel omdat we met een kleine groep pilotgebruikers werken. Daarnaast werkt een grote instance als Codeberg nu ook zonder CSP zijn daar voor zover ik weet geen XSS-aanvallen gemeld. Daarom lijkt het mij beter om CSP in report-only mode te zetten om zo eerst alle functionaliteit in Forgejo CSP-ready te kunnen maken, zodat we daarna CSP strikt aan kunnen zetten zonder dat er functionaliteit stuk gaat.
Daarnaast is het waarschijnlijk sneller om de CSP fixes als kleine PR per gefixte feature/pagina upstream aan te bieden ipv een grote PR die heel Forgejo aanpakt.
Ik heb op verschillende plekken in de code potentiele XSS hoekjes gevonden, ik zou daar toch wat voorzichtig mee zijn. CSP report-uri's toevoegen is een goed idee, al moet iemand dat wel optuigen en gaan monitoren.
Ik zie de potentiele schade vooral buiten deze hostname ontstaan. We zijn immers onderdeel van overheid.nl, en daarmee is een inbraak op deze site met wat pech een deur naar het browser security domain van andere subdomeinen (als zij bijvoorbeeld cookies niet netjes onbereikbaar voor Js en .overheid.nl maken). Ik durf niet te zeggen dat er nergens op het overheid.nl domein te breed leesbare cookies beschikbaar zouden zijn die langs een XSS hier gelezen kunnen worden. Het zelfde geldt natuurlijk voor XSRF achtige zaken op sites die niet netjes hun cors afdwingen, daarbij maakt het niet uit of de cookies wel of niet leesbaar zouden zijn op het code. subdomain.
De CSP fixes die ik heb geupstreamed zijn opgenomen in de Forgejo roadmap voor 1.16, en zijn onderdeel van hun reactie op een ietwat irresponsible disclusure over de security posture. Ik verwacht dat er wellicht technisch nog opmerkingen zijn die verwerkt moeten worden, maar dat de patches welkom en gewaardeerd zijn.