Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notificatie abonnement lijkt strijdig met AVG #1427

Closed
erikdelepper opened this issue Oct 30, 2019 · 17 comments
Closed

Notificatie abonnement lijkt strijdig met AVG #1427

erikdelepper opened this issue Oct 30, 2019 · 17 comments

Comments

@erikdelepper
Copy link

Bug

Bij een abonnement kun je aangeven op welke gegevens je je abonneert. Dat kan bijvoorbeeld op 'Indiener', wat feitelijk een BSN is. Omdat abonnering los van autorisatie gebeurt, is dit volgens ons strijdig met de AVG.

Notificaties zouden daarom arm moeten zijn; dat wil zeggen er zou alleen moeten worden genotificeerd dát een bepaald object gewijzigd. De abonnementnemer moet daarna zelf het object ophalen, waarbij de juiste autorisatie van toepassing is.

[assign aan Joeri en/of Sergei, voeg label 'bug' toe, kies bij 'projects' voor 'Scrum ZGW Realisatie Huidige Sprint']

Om gericht te ondersteunen graag ook antwoord op de onderstaande vragen:

Waar staan de componenten gehost? (ref.tst.vng.cloud of zelf binnen eigen organisatie)

Welke claims/payload bevat je JWT?

@joeribekker
Copy link
Collaborator

joeribekker commented Oct 30, 2019

Dat kan bijvoorbeeld op 'Indiener', wat feitelijk een BSN is

Waar lees je dat dit kan?

@erikdelepper
Copy link
Author

Dit staat (naar mening van verschillende ontwikkelaars die ik heb gesproken) in de documentatie onder https://notificaties-api.vng.cloud/api/v1/schema/#operation/notificaties_create

Daar worden kenmerken meegegeven, wat componenten de mogelijkheid geeft zelf te bepalen sleutel/waarden mee te geven. Aangezien er nergens anders in de API Docs een voorbeeld van een bericht is gaan we er dan even vanuit dat dat het bericht is dat naar de callback url wordt gestuurd.

Onder water wordt er gebruik gemaakt van RabitMQ. Daar staat volgens ons dat de aangeleverde body wordt doorgezet. Dat zou in combinatie met bovenstaande betekenen dat de criteria waarop gefilterd kan worden, worden doorgezet.

Uiteraard zou het kunnen dat de werkelijkheid anders is, maar dat halen wij niet uit de documentatie.

@sergei-maertens
Copy link
Collaborator

Ja, iedere publicerende API bepaalt welke kenmerken van toepassing zijn. Dit moet dus met aandacht gekozen worden, om te voorkomen dat er gevoelige gegevens opgenomen worden.

Voor de zaken API bijvoorbeeld staat gedefinieerd dat de kenmerken bronorganisatie, zaaktype en vertrouwelijkheidaanduiding meegestuurd wroden.

@sergei-maertens
Copy link
Collaborator

Bij een abonnement kun je aangeven op welke gegevens je je abonneert. Dat kan bijvoorbeeld op 'Indiener', wat feitelijk een BSN is.

Klopt niet - in elke API standaard wordt beschreven wat de relevante kenmerken zijn (voorbeeld https://zaken-api.vng.cloud/api/v1/schema/)

Notificaties zouden daarom arm moeten zijn; dat wil zeggen er zou alleen moeten worden genotificeerd dát een bepaald object gewijzigd. De abonnementnemer moet daarna zelf het object ophalen, waarbij de juiste autorisatie van toepassing is.

Dat is ook het geval 🙂

@michielverhoef volgens mij kan deze bug gesloten worden, de beschreven gewenste situatie is namelijk al de huidige situatie.

@joeribekker
Copy link
Collaborator

@erikdelepper Wat @sergei-maertens zegt :)

Tevens hebben wij er bewust voor gekozen om NIET het BSN als sleutel in URLs te gebruiken maar UUIDs. Het BSN lekt dus zelfs niet als je WEL een "Indiener" zou meegeven.

@CMasselink
Copy link
Collaborator

@erikdelepper Je stipt volgens mij een heel terecht punt aan. Bij notificeren moet je rekening houden dat je niet (onbedoeld) gegevens lekt met notificaties.
Wij denken er in onze ZGW standaard al wel rekening mee gehouden te hebben doordat alleen informatie arme kenmerken mogelijke zijn.
Ben jij het, gezien bovesntaande, samen met de ontwikkelaars die je gesproken hebt het eens met ons dat we vanuit de standaard voldoende rekening hebben gehouden met de AVG?

We zullen kijken of we de documentatie op dit punt kunnen verbeteren, als jullie hier nog suggesties voor hebben, horen we dat uiteraard graag.

@erikdelepper
Copy link
Author

Dank voor de toelichting. Wij denken dat er toch nog een aantal gaten in zitten. Zo kan bij zaken op papier wel gedefinieerd zijn wat de kenmerken bij een notificatie zijn, het lijkt erop dat bij de notificatie zelf dat niet wordt afgedwongen.
Daarnaast rijst bij ons de vraag waar de controle ligt bij andere notificaties of daar alleen 'arme' kenmerken worden gebruikt. Ons punt is dus dat notificaties niet inherent veilig zijn en een fout snel gemaakt is.

@sergei-maertens
Copy link
Collaborator

sergei-maertens commented Nov 14, 2019

@erikdelepper in de referentie-implementatie wordt dat gevalideerd: https://github.com/VNG-Realisatie/gemma-notificatiecomponent/blob/develop/src/nrc/api/serializers.py#L169

Bij het registreren van een notificatiekanaal wordt vastgelegd wat de kenmerken zijn.

@rubenvdlinde
Copy link

rubenvdlinde commented Nov 20, 2019

Hey Mensen,

Ik heb samen met @ThijsBroersen gekeken naar de implementatie van het notificatie component binnen huwelijksplanner en we kwamen eigenlijk tot dezelfde conclusies als @erikdelepper . @CMasselink vroeg of we deze met ZGW wouden afstemmen (overigens enige tijd terug, maar het was druk en @erikdelepper had het al aangestiped). Maar aangezien we hier vanuit huwelijksplanner mee aan de slag willen toch een post.

Lopende lijn hierbij is dat het niet volgen van Websub en het implementeren van kenmerken een reeks problemen met zich mee lijkt te brengen. Ik heb die aan de hand van eerdere constateringen even uitgewerkt als challenges en hieronder neergezet. Als @Hugo-ter-Doest dat wil kan ik er ook losse issues van maken maar eigenlijk komen alle onderstaande challenges voort uit één keuze (namelijk kenmerken).

Het notificatie component lijkt strijdig met GEMMA architectuur structuur
Binnen het GEMMA landschap gaan we uit van data bij de bron, vanuit de documentatie van notificaties lijkt het er op dat er kenmerken naar ontvangers worden verstuurd. Daarmee ontstaat er buiten de logging om, een geheel nieuwe route waarover informatie wordt verstuurd. Niet alleen is dit geen onderdeel van de architectuur, het is strijdig met het uitgangspunt van data bij de bron.

Het notificatie component lijkt strijdig met best practices
Als we kijken naar voorbeelden van het updaten van “gevoelige” notificaties dan zien we dat er geen data wordt meegestuurd, zie bijvoorbeeld mollie. De gedachte is hier dat een webhook op de hoogte wordt gebracht van “een” wijziging, maar zich vervolgens zelf opnieuw moet valideren om die wijziging op te halen. Sterker nog zelfs het soort wijziging (update, delete etc) wordt niet vermeld.

Het zonder concrete vraag versturen van informatie lijkt tevens strijdig met privacy by design.

Het notificatie component lijkt strijdig met internationale standaarden
Er is een internationale standaard voor het versturen van notificatie berichten naar webhooks, namelijk websub . Hoewel deze standaard wel voorziet in het versturen van kenmerken (zou ik niet aanraden), voorziet die ook in een legio aan andere beveiligingsmechanismen. Maar veel belangrijker de hele setup is anders, websub kent geen kanalen maar je abonneert simpelweg op een endpoint. De manier waarop wordt je ook meegegeven als header op dat endpoint waarmee je over een strategie beschikt voor auto exploring, linked data etc.

Het notificatie component lijkt strijdig met AVG
Als we status updates met data gaan versturen over secundaire kanalen, die niet terugkomen in onze AVG loging, dan is het component niet AVG proof. Sterker nog de ontvanger krijgt informatie zonder dat daar een concreet/direct verzoek aan ten grondslag ligt. En deze context is informatie over zaken trouwens ook aan te duiden als persoonlijke informatie.

Het notificatie component lijkt een datalek te bevatten
Ongetwijfeld is dit wel geregeld, maar ik heb niet in de documentatie kunnen terugvinden dat vertrouwelijkheids aanduiding effect heeft op het versturen van notificaties. Dus volgens mij kan ik een abonnement nemen op een zaak, wordt deze later aangemerkt als vertrouwelijk, maar blijf ik als cliënt dan toch updates ontvangen.

Als bovenstaande klopt heeft het te maken met de keuze om niet per notificatie voor versturen te controleren of er nog authorisatie is. Zie ook Het component lijkt intrinsiek onveilig

Het notificatie component lijkt onnodig complex
Er is nu gekozen voor een systematiek met kanalen waarop je als client dan een soort van gefilterd abonnement neemt met als handleiding daarvoor het goed lezen van documentatie. Dat is een stuk bewerkelijker dan websub, waarbij je simpelweg een abonnement neemt op een endpoint, waarbij het endpoint je onder water vertelt ‘ik heb een abonnement, dat kan je hier aanvragen’ en de “kanalen” worden gevormd door de query parameters die het endpoint van zichzelf heeft.

Het notificatie component lijkt onnodig simpel
Er nu gekozen voor een systematiek met kanalen waarop je dan een soort van gefilterd abonnement neemt, waarbij er beperkte kenmerken worden doorgezet,was er gekozen voor het opnieuw ophalen van gegevens, dan hadden er rijke objecten mét vertrouwelijke informatie kunnen worden verstrekt (er is dan immers sprake van een nieuwe vraag).

Het notificatie component lijkt een onnodige beheers last te creëren
Doordat er nu wordt gewerkt met kanalen en veilig gekeurde kenmerken, moeten er dus kenmerken “veilig” verklaard worden. Dit vraagt om een stramien waarop de VNG op alle API’s gaat meekijken en beoordeelt. En dat er een club komt met ‘stempel macht’ om een kenmerk onderdeel te maken van de kenmerken standaard. Gezien de breedte van het landschap, het aantal componenten en doorlooptijd van componenten, lijkt me dat een inrichtings fout. Gezien het feit dat er ook voor gekozen had kunnen worden om géén kenmerken mee te sturen, in welk geval je zou kunnen leunen op huidige standaarden en al deze beheersmaatregelen dus niet nodig waren.

Het is nu bijvoorbeeld al zo dat in het authenticatie component (het meest veiligheids gevoelige document) de kenmerken niet staan gedefinieerd in de documentatie, terwijl het component als voorwaarde het notificatie component noemt. Als we dat binnen één team op het méést essentiële component ogenschijnlijk niet geregeld krijgen dan gaat dat op het gehele VNG landschap waarschijnlijk ook niet lukken.

Het notificatie component creëert een eeuwigdurende drain op resources
Een harde filtering toevoegen aan het component is dan de enige route om het nog enigszins veilig te maken. Dat is ook een beetje de lijn in de discussie hierboven Dat betekent dat we:
eigenlijk maar één notificatie component kunnen hebben,beheert door de VNG en
dit component continu moet worden bijgewerkt aan de hand van wijzigingen in het landschap. Dat is een eeuwig lopende bouw opdracht van groot formaat.

Het notificatie component lijkt als gevolg hiervan architecturaal inherent onveilig
De ingewikkelde beheers constructie vraagt om een menselijke inschattingsfout met betrekking tot veilige kenmerken of de implementatie daarvan. Hiermee organiseren we een onnodige kwetsbaarheid in het systeem in.

Het component lijkt intrinsiek onveilig
Het is mij niet duidelijk vanuit de documentatie hoe en waar er wordt gecontroleerd bij het verzenden van notificaties, of de ontvangende partij nog steeds recht op die informatie heeft. Het lijkt erop dat bij het verliezen van leesrechten op een object een webhook daar nog steeds updates over ontvangt.

De gedachten over veiligheid zijn strijdig met daarvoor geldende richtlijnen
Het “we gaan het opvangen met een standaardisatie”, ofwel vastleggen van wat wél mag is het oplossen van een technisch en architecturale keuze met een beheersmaatregel. In plaats daarvan dient de onderliggende techniek en architectuur zo in elkaar te zitten dat deze inherent veilig is.

Samenvattend
De keuze voor het werken met kanalen en kenmerken lijkt strijdig met het GEMMA landschap, algemene en internationale standaarden, AVG, privacy by design en lijkt veiligheidsproblemen op te leveren of op zijn minst in de hand te werken.

Mijn persoonlijke voorstel zou dan ook zijn dit component opnieuw op te trekken waarbij het wordt gebaseerd op websub aangevuld met de best practices van mollie en nauw geïntegreerd wordt met het authorisatie component (denk aan het opnieuw checken van rechten voor versturen). In ieder geval zou ik het component in haar huidige vorm, liever niet als referentie willen aanmerken.

@thijs
Copy link

thijs commented Nov 20, 2019 via email

@michielverhoef
Copy link
Collaborator

Excuses, dat had @ThijsBroersen moeten zijn vermoed ik.

@rubenvdlinde
Copy link

rubenvdlinde commented Nov 20, 2019

Excuses dat was ik die iets te snel op enter drukte :( hij is inmiddels aan aangepast zie ik (ik vermoed door @michielverhoef ) waarvoor dank!

@Hugo-ter-Doest
Copy link
Contributor

Ik zou graag zien dat je de claims die nu beginnen met "lijkt" verifieert alvorens ze op te schrijven. Het kan nl. ook zijn dat de API wel voldoet op zo'n punt, en dat het niet goed gedocumenteerd is hoe het werkt. En inderdaad graag separate user stories opvoeren voor zaken die anders moeten.

@erikdelepper
Copy link
Author

@Hugo-ter-Doest, ik snap dat je dit zegt, maar realiseer je hoeveel werk dat is en daarmee hoeveel geld dat kost. We kunnen niet van een commerciële partij verwachten dat zij dit gratis gaan uitzoeken en vanuit de gemeente hebben wij er we moeite mee leveranciers te gaan inhuren om mogelijk fouten op te sporen in de referentie-API's van VNG.

@Hugo-ter-Doest
Copy link
Contributor

Het probleem dat ik hier als product owner mee heb is dat het nu net lijkt of het niet voldoet, terwijl dat niet geverifieerd is.

@erikdelepper
Copy link
Author

Het is zeker dat het niet voldoet. Want volgens de Definition of Done is de documentatie onderdeel van oplevering. Geconstateerd wordt dat er minimaal een probleem zit in de documentatie en mogelijk ook in de referentie-API.

@Hugo-ter-Doest
Copy link
Contributor

Ik sluit hem af omdat de inhoud van notificaties niet door de notificatie-component wordt bepaald maar door de notificaties publicerende API. Dat is niet per se onveilig en zeker geen datalek. Je kunt werken met zodanig arme notificaties dat je alleen te weet dat er iets is gebeurd met een resource.

Ten tweede vind ik de discussie tendentieus in die zin dat er claims worden gemaakt die beginnen met "lijkt strijdig". Daarmee krijgt de lezer de indruk dat er iets niet klopt terwijl dat niet het geval hoeft te zijn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants