VNG-Realisatie/Haal-Centraal-WOZ-bevragen

Foutmelding wanneer niet-gespecificeerde parameter wordt gebruikt

Opened this issue · 16 comments

Wanneer een query parameter wordt gebruikt die niet is gespecificeerd zou ik een foutmelding verwachten.

Bijvoorbeeld "https://api.acceptatie.kadaster.nl/lvwoz/api/v1/wozobjecten?rsin=857567433&grondoppervlakte=106"

Nu wordt dit (niet-gespecificeerde parameter "grondoppervlakte") volledig genegeerd, ook in de self link:

{
    "_links": {
        "self": {
            "href": "http://api.acceptatie.kadaster.nl/haalcentraal-api/wozobjecten?rsin=857567433"
        }
    }
}

Zie ook https://github.com/VNG-Realisatie/Haal-Centraal-WOZ-bevragen/pull/49/files:

Rule: alleen gespecificeerde parameters mogen worden opgegeven
    Abstract Scenario: Niet gespecificeerd parameter
        Als '/wozobjecten?<parameter>=123456789' wordt aangeroepen
        Dan bevat de response de volgende kenmerken
        | naam   | waarde                                                     |
        | title  | Een of meerdere parameters zijn niet correct.              |
        | status | 400                                                        | 
        | detail | Zoeken met parameter '<parameter>' wordt niet ondersteund. |
        En bevat de response de volgende invalidParams
        | name        | reason                 |
        | <parameter> | wordt niet ondersteund |

        Voorbeelden:
        | description                                                                      | parameter     |
        | gebruik van een WOZ-object kenmerk die niet wordt ondersteund als zoek parameter | identificatie |
        | willekeurig string als zoek parameter                                            | bestaatniet   |

@fsamwel Dit is niet standaard voor een api, of wat voor een url dan ook. We kijken naar bestaande parameters, en of deze correct worden mee gegeven, maar niet naar alle andere mogelijke parameters die niet gebruikt worden door de API.

De self link is dan ook in feite correct. Aangezien de parameter grondoppervlakte niet bestaat op de API, heeft deze parameter ook geen effect op de uitkomst van de API, en is dus identiek aan de aanroep zonder de niet bestaande parameter.

@MelvLee jij hebt deze foutmelding gedefinieerd. Kan jij uitleggen waarom jij wel graag een foutmelding hierop wilt krijgen? (of het uit je pull request halen wanneer het eigenlijk toch niet zo nodig is)

Overigens is het wel goed de werking van de API te specificeren op dit punt, want de verschillende Haal Centraal API's reageren verschillend op dit punt. De BAG API geeft hierop een fout, de BRK API negeert de onbekende parameter, maar neemt deze wel op in de self-link, de BRP API (implementatie in Den Haag) geeft ook een foutmelding en de WOZ API negeert de parameter en neemt deze ook niet op in de self-link.

@MelvLee jij hebt deze foutmelding gedefinieerd. Kan jij uitleggen waarom jij wel graag een foutmelding hierop wilt krijgen? (of het uit je pull request halen wanneer het eigenlijk toch niet zo nodig is)

Dit is een copy-paste actie uit de foutafhandeling feature in HC Common.

Of het nodig is weet ik niet. Het hangt af van de gewenste functionaliteit. We kunnen ervoor kiezen om alleen de parameters te valideren die in de OAS specificaties staan en andere te negeren. Dit is wat .NET standaard doet en ik verwacht dat de Spring framework ook standaard doet. Dit is dus het makkelijkst voor de provider. Dit betekent ook wel dat je sommige situaties niet kan detecteren, zoals een letter invullen in huisnummer. In dat geval krijgt je in .NET code voor huisnummer de waarde 0.

Volgens mij kan het geen kwaad om het te negeren, maar 100% durf ik het niet te zeggen, want het zou heel goed kunnen dat het security wise een bad practice is.

Dit is wat .NET standaard doet en ik verwacht dat de Spring framework ook standaard doet.

Exact dit. Ik heb hier nog geen mooie algemene specificatie van kunnen vinden, zoals een w3c rfc oid, maar heb wel wat rond gezocht op willekeurige urls, door achter de root een niet bestaande parameter te zetten, en ben nog nergens een foutmeldig tegen gekomen. Daar moet ik wel bij zeggen, dit is voor zover ik weet altijd het standaard gedrag geweest van Spring boot, dus het is ook iets waar ik helemaal geen rekening meer mee houd. Geeft maar weer het gevaar van aannames weer.

Dit betekent ook wel dat je sommige situaties niet kan detecteren, zoals een letter invullen in huisnummer. In dat geval krijgt je in .NET code voor huisnummer de waarde 0.

Deze volg ik niet helemaal. Als parameters gespecificeerd zijn op het endpoint kunnen we deze valideren op welke manier we ook willen. Dus wanneer zoeken op huisnummer is geïmplementeerd, kunnen we die ook valideren op numeriek.

Als parameters gespecificeerd zijn op het endpoint kunnen we deze valideren op welke manier we ook willen. Dus wanneer zoeken op huisnummer is geïmplementeerd, kunnen we die ook valideren op numeriek.

helemaal mee eens. Als je een niet-gespecificeerde parameter negeert zie ik daar ook niet veel schadelijks aan. Als je een gespecificeerde parameter krijgt moet die gevalideerd worden, bijvoorbeeld huisnummer=1b moet denk ik een foutmelding geven.

De Security gevolgen weet ik zo niet. Voor een website zou het ongevalideerd doorzetten van invoer een cross site scripting risico zijn. We geven de gebruikersinvoer immers ongewijzigd terug in de self link. Maar wat dat betreft is het maar de vraag of een foutmelding heel veel helpt als we gebruikersinvoer ook in de instance property in een foutmelding teruggeven.

We geven de gebruikersinvoer immers ongewijzigd terug in de self link. Maar wat dat betreft is het maar de vraag of een foutmelding heel veel helpt als we gebruikersinvoer ook in de instance property in een foutmelding teruggeven.

Dit is iets wat ook opgelost wordt door Spring's implementatie van REST. Voor zo ver Spring weet, zijn alleen de parameters die op het endpoint zijn gespecificeerd mee gegegeven. Dit heeft als tweede effect dat, tenzij ik er specifiek rekening mee houd, alleen gespecificeerde parameters kunnen verschijnen in de self link. Kort gezegd, wat de server betreft, bestaan niet gedefinieerde parameters helemaal niet.

(Ik zou ze vast wel ergens diep uit de servlet request kunnen vissen, maar dan moet je er echt naar gaan zoeken).

Als de parameters nergers worden geinterpreteerd, is er volgensmij ook geen XSS risico

voor nu wordt deze geparkeerd. Hoeft nu niet te worden opgelost.

Dit betekent ook wel dat je sommige situaties niet kan detecteren, zoals een letter invullen in huisnummer. In dat geval krijgt je in .NET code voor huisnummer de waarde 0.

Deze volg ik niet helemaal. Als parameters gespecificeerd zijn op het endpoint kunnen we deze valideren op welke manier we ook willen. Dus wanneer zoeken op huisnummer is geïmplementeerd, kunnen we die ook valideren op numeriek.

Wat ik bedoel is dat in .NET in het geval van huisnummer zonder custom code niet kan achterhalen of een consumer geen huisnummer, een ongeldig huisnummer (1A of alleen A) of 0 heeft opgegeven. In alle drie gevallen krijgt je standaard 0.

Ik concludeer uit de reacties van @kad-tromps en @MelvLee dat we niet gespecificeerde parameters kunnen negeren en die ook niet opnemen in instance en _links.self.

@MelvLee pas jij dat ook aan in de feature(s)?

Ik concludeer uit de reacties van @kad-tromps en @MelvLee dat we niet gespecificeerde parameters kunnen negeren en die ook niet opnemen in instance en _links.self.

@MelvLee pas jij dat ook aan in de feature(s)?

Feature is al aangepast

Ik begrijp dat het issue met de onbekende parameter is geparkeerd. Vooralsnog worden onbekende parameters dus genegeerd.
Overigens wordt deze nu wel mee teruggegeven in de self link.

De aanroep GET https://api.acceptatie.kadaster.nl/lvwoz/api/v1/wozobjecten?rsin=857567433&grondoppervlakte=106 geeft echter alleen het volgende terug

{
    "_embedded": {},
    "_links": {
        "self": {
            "href": "https://api.acceptatie.kadaster.nl/haalcentraal-api/wozobjecten?rsin=857567433&grondoppervlakte=106"
        }
    }
}

Wat volgens mij toch niet correct is. Blijkbaar is de resource er wel maar bevat deze geen content.

Ik realiseer me nu dat het gaat om een lege collection die teruggegeven wordt.

Blijft alleen de opmerking staan m.b.t. de onbekende parameter die in de self link wordt teruggegeven. Die zou ik daar niet verwachten. Hij wordt immers genegeerd.

Is begrijp inmiddels dat het juist een goed gebruik is dat de self link identiek is aan de url die is ingestuurd. Ook onbekende parameters worden daarom opgenomen in de self link.

Volgens mij kan het geen kwaad om het te negeren, maar 100% durf ik het niet te zeggen, want het zou heel goed kunnen dat het security wise een bad practice is.

@MelvLee Heb jij hier inmiddels al wat meer ervaring mee. Levert het negeren van een invalid parameter risico's op? Een consumer kan er dan immers alles inzetten wat hij wil, toch? Of is die vraag met de opmerking:

Als de parameters nergers worden geinterpreteerd, is er volgensmij ook geen XSS risico

van @kad-tromps al beantwoord?

Volgens mij kan het geen kwaad om het te negeren, maar 100% durf ik het niet te zeggen, want het zou heel goed kunnen dat het security wise een bad practice is.

@MelvLee Heb jij hier inmiddels al wat meer ervaring mee. Levert het negeren van een invalid parameter risico's op? Een consumer kan er dan immers alles inzetten wat hij wil, toch? Of is die vraag met de opmerking:

Als de parameters nergers worden geinterpreteerd, is er volgensmij ook geen XSS risico

van @kad-tromps al beantwoord?

Wat is eigenlijk de context van je vraag? Het is namelijk geen probleem als niet-gespecificeerde parameters wordt genegeerd. Maar het kan wel een issue zijn als de request wordt gelogd en één of meerdere niet-gespecificeerde parameters bevat malafide code en een kwetsbare versie van log4j wordt nog gebruikt. Het wel of niet geven van een foutmelding bij niet-gespecificeerde parameters zorgt er namelijk niet voor dat er geen probleem ontstaat als er een kwetsbare versie van log4j nog wordt gebruikt (als dit het geval is)

Wat ik eigenlijk wil weten is of we het gebruik van niet gespecificeerde parameters nu wel of niet moeten gaan toestaan. Daar gaat dit issue uiteindelijk om. Als ik het goed begrijp kunnen we niet gespecificeerde parameters gebruiken als:

  • de kwetsbare versie van log4jrequest niet worden gebruikt;
  • of als requests niet worden gelogd.

Lijkt me een vraag die het Kadaster moet beantwoorden. Als een van deze vragen met 'ja' wordt beantwoord dan kunnen we dat soort parameters toestaan en kan dit issue worden gesloten. Daarbij moet je n.m.m. ook in ogenschouw nemen in hoeverre je kunt garanderen dat de beantwoording van deze vraag ook in de toekomst voor beide 'ja' zal zijn. Hoe groot is het gevaar dat er in de toekomst niet weer een kwetsbare log4j versie zal komen?

Staan we niet gespecificeerde parameters toe moeten we dat dan nog ergens documenteren?