De PHPhulp kroeg
verificatie, veiligheid
Ik heb laatst hier al een vraag over gesteld, maar ik wil graag voor mezelf nog even een en ander op een rijtje krijgen. Aangezien deze vraag eigenlijk op heel veel classes van toepassing is, hoop ik op heldere antwoorden zodat ik voor mezelf de juiste keuze kan maken.
Ik ga nu als voorbeeld uit van een User, maar het kan eigenlijk om van alles gaan (een product, een nieuwsbericht, een adres enz.).
Ik zal de vragen nummeren zodat jullie makkelijker kunnen antwoorden.
1) Moet je informatie uit je eigen database beschouwen als veilig?
Stel iemand schrijft zich in op jouw site. De benodigde controles (geldige naam, geldige leeftijd e.d.) zijn uitgevoerd en de user belandt vervolgens in de database. Op een later moment haal je die gegevens weer op uit de database (bijv. omdat de betreffende user inlogt). Beschouw je deze gegevens nu als veilig? Of ga je alle gegevens weer opnieuw controleren?
2) Wie bepaalt of het een geldige User is? Doet de User class dat zelf? Of vindt de verificatie plaats via een aparte verificatie class (Verifier) en is de User class slechts een container die de geverifieerde data vasthoudt?
Een voorbeeld van wat ik hiermee bedoel:
Verificatie vindt plaats in de User class zelf:
Code (php)
1
2
3
4
2
3
4
<?php
$user = new User();
$user->setName('Piet'); // controleert of de naam een string is en meer dan x tekens bevat
?>
$user = new User();
$user->setName('Piet'); // controleert of de naam een string is en meer dan x tekens bevat
?>
Of... we maken gebruik van een Verifier class:
Code (php)
1
2
3
4
5
2
3
4
5
<?php
$verifier = new Verifier();
$verifier->verifyName('Piet'); // controleert of de naam een string is en meer dan x tekens bevat
$user = $verifier->getUser(); // geeft een user-object terug op basis van de geverifieerde gegevens
?>
$verifier = new Verifier();
$verifier->verifyName('Piet'); // controleert of de naam een string is en meer dan x tekens bevat
$user = $verifier->getUser(); // geeft een user-object terug op basis van de geverifieerde gegevens
?>
Hoe zouden jullie het aanpakken?
Quote:
Moet je informatie uit je eigen database beschouwen als veilig?
Wat vind jij dan onveilig?
Dat er bijvoorbeeld een ' in een stuk tekst kan staan?
Dat zou nog steeds kunnen. Dus als jij van de user Jeanne d'Arc de naam opnieuw moet opslaan in een tabel, dan zul je nog steeds moeten escapen.
Lengte username:
Als je eenmaal hebt bepaald dat "Piet" geldige naam is, omdat hij meer dan x tekens lang is, dan moet je dat niet later afkeuren.
Stel dat Jo een goede naam was volgens jouw verificatie, omdat het minimaal 2 letters is.
Dan kom je op het idee dat een naam toch zeker wel uit 3 letters zou bestaan.
Jo doet een actie (zeg een post op een forum) en dan loopt het proces spaak op een te korte naam?
Sowieso vind ik een minimale lengte van een naam al dubieus. Ik heb vaak zat dat op 3 zien staan. Ik kom dan zelf nog goed weg, maar genoemde Jo niet.
En volgens mij zijn er zelfs (buitenlandse) namen van 1 letter.
Er is zelfs een dorp in Nederland met 2 straten die beide een straatnaam van 1 letter hebben.
Die mensen hebben vaak problemen om iets te bestellen: op basis van postcode wordt automagisch de straatnaam ingevuld in een form en dat kunnen ze niet aanpassen.
Vervolgens komt er zo'n leuk verificatie scriptje in actie om de invoer dan doodleuk af te keuren.
Toevoeging op 03/04/2014 20:57:45:
http://nl.wikipedia.org/wiki/Straatnaam#Langste_straatnamen
Opgeslagen gegevens beschouw ik als veilig.
>> 2) Wie bepaalt of het een geldige User is? Doet de User class dat zelf? Of vindt de verificatie plaats via een aparte verificatie class (Verifier) en is de User class slechts een container die de geverifieerde data vasthoudt?
Ligt aan het type validatie (sorry, maar ook dit is situatie afhankelijk). Stel we hebben een Money klasse, dan bepaald die klasse gewoon lekker zelf of de gebruikte currency wel valid is.
Maar bij bijv. een user waarvan de naam X tekens lang moet zijn zou ik gebruik maken van een buitenstaande Validator. De User object geeft dan zelf wel aan, doormiddel van metadata, aan welke eisen de waarde moet voldoen, maar het werkelijke valideren gebeurd door een validator.
Een heel simpel voorbeeldje met het gebruik van de Symfony validator (sorry, ik heb nog niet met een andere gewerkt):
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
<?php
use Symfony\Validator\Mapping\ClassMetadata;
use Symfony\Validator\Mapping\Constraints as Assert;
class User
{
private $name;
public static function getValidationMetadata(ClassMetadata $metadata)
{
// met een "Constraint" leg je een bepaalde beperking op
// in dit geval leggen we deze op een property (namelijke $name)
$metadata->addPropertyConstraint('name', array(
// het veld mag niet leeg zijn, een user zonder naam bestaat immers niet
new Assert\NotBlank(),
// het veld moet minimaal 5 tekens lang zijn, beetje onzin maar het gaat om het idee
new Assert\Length(array('min' => 5)),
));
}
}
?>
use Symfony\Validator\Mapping\ClassMetadata;
use Symfony\Validator\Mapping\Constraints as Assert;
class User
{
private $name;
public static function getValidationMetadata(ClassMetadata $metadata)
{
// met een "Constraint" leg je een bepaalde beperking op
// in dit geval leggen we deze op een property (namelijke $name)
$metadata->addPropertyConstraint('name', array(
// het veld mag niet leeg zijn, een user zonder naam bestaat immers niet
new Assert\NotBlank(),
// het veld moet minimaal 5 tekens lang zijn, beetje onzin maar het gaat om het idee
new Assert\Length(array('min' => 5)),
));
}
}
?>
Nee, maar het mag wel. Het wordt ook heel vaak gedaan.
Wel is het slim om het dat je gebruikt in je applicatie een zo beperkt mogelijke set rechten te geven.
Je zou ook één account kunnen hebben voor selects en één voor updates/inserts/deletes. Mocht er een sql-injectie lek in je search functionaliteit zitten dan kunnen ze alleen data bemachtigen. Ze kunnen zichzelf niet admin maken omdat het select account geen updates uit kan voeren.
Maar dan moet je in je code base wel het de juiste verbinding gebruiken voor de juiste query. Dan kun je je afvragen of het niet efficiënter is om een keer al je queries na te laten lopen door een tweede set ogen.
Een andere opties is om directe toegang tot tabellen te verbieden en stored procedures schrijft (op een account dat wel directe toegang heeft natuurlijk, aangezien stored procedures tenzij anders aangegeven uitgevoerd met de rechten van de owner) die een API vormen. Dan gebruik je in je applicatie een account dat alleen maar stored procedures aan mag roepen.
Als programeur moet je afwegingen maken. Paranoia kost werk, het is makkelijker om de data uit een database voor een groot deel te vertrouwen.
Tekst die door users ingevoerd wordt vertrouw je niet zomaar.
Tekst die niet door de applicatie ingevoerd kan worden zou je kunnen besluiten om blindelings te vertrouwen. Zeker als het account dat de applicatie gebruikt alleen select rechten heeft op die tabel. Het is makkelijker en wij programmeurs zijn lui!
Ik zou bijvoorbeeld een tabel timezone(id, identifier) kunnen vullen op basis van https://php.net/manual/en/datetimezone.listidentifiers.php
In theorie is het dan niet nodig om htmlspecialchars() over de waarde heen te gooien als ik het op een pagina wil printen omdat mijn applicatie geen bugs hoort te bevatten.
"In theory, there is no difference between theory and practice. But, in practice, there is." - source unknown.
De vraag is dus, wat mij betreft, hoe paranoïde ben jij?
2) Wie bepaalt of het een geldige User is? Doet de User class dat zelf? Of vindt de verificatie plaats via een aparte verificatie class (Verifier) en is de User class slechts een container die de geverifieerde data vasthoudt?
Ik heb niet heel veel verstand van SOLID, maar volgens mij zou volgens het "Single responsibility principle" user validatie zijn eigen class moeten krijgen...
>> Dat er bijvoorbeeld een ' in een stuk tekst kan staan?
Nou, in een van mijn vorige topics zei iemand hier (ik weet niet meer wie) dat hij z'n data dubbel controleert, omdat mogelijk de database gehackt wordt en er andere gegevens in komen te staan dan die jij erin hebt gezet. Dus stel je slaat keurig de naam 'Piet' op in de database, en iemand zou dan de database hacken en in plaats van 'Piet' staat er nu in de database '<script>evil</script>'. En om zoiets te voorkomen, ging hij dus de gegevens uit de database telkens controleren.
>> Er is zelfs een dorp in Nederland met 2 straten die beide een straatnaam van 1 letter hebben.
Serieus? Welk dorp en welke straten??
>> Opgeslagen gegevens beschouw ik als veilig.
Ja, dat zou ik ook denken. En daar heeft mijn vraag dan ook echt betrekking op. Als je er vanuit gaat dat de informatie in de database veilig is, dan zou ik dus denken dat je de informatie vanuit de database rechtstreeks in een User object kunt injecteren en dat je die dus niet nogmaals hoeft te controleren. En in dat geval betekent het dus dat je inderdaad een aparte Verifier of Validator class zou moeten gebruiken. Correct?
>> Een heel simpel voorbeeldje met het gebruik van de Symfony validator (sorry, ik heb nog niet met een andere gewerkt):
Geeft niet, ik vind het al fijn dat je een voorbeeldje plaatst :)
En wat nu als ik, indien nodig, per class (in dit geval User) een validator maak, waarmee je dus zoiets kan doen:
Code (php)
1
2
3
4
5
2
3
4
5
<?php
$validator = new UserValidator();
$validator->validateName('Piet'); // controleert of de naam een string is en meer dan x tekens bevat
$user = $validator->createUser(); // geeft een user-object terug op basis van de geverifieerde gegevens
?>
$validator = new UserValidator();
$validator->validateName('Piet'); // controleert of de naam een string is en meer dan x tekens bevat
$user = $validator->createUser(); // geeft een user-object terug op basis van de geverifieerde gegevens
?>
Zou dat een werkbare oplossing zijn?
>> Maar dan moet je in je code base wel het de juiste verbinding gebruiken voor de juiste query.
Oké. Als ik er services van maak zou dit vrij simpel te realiseren moeten zijn.
>> Ik heb niet heel veel verstand van SOLID, maar volgens mij zou volgens het "Single responsibility principle" user validatie zijn eigen class moeten krijgen...
Ik snap wat je bedoelt. Je zou dan een User en een UserValidator class verwachten. In de praktijk zie ik dit echter nergens, en wordt alles in de class zelf gecontroleerd. Vandaar ook mijn verwarring en onzekerheid of het wel de bedoeling is om met een losse Validator te werken.
Gewijzigd op 03/04/2014 21:46:23 door Ozzie PHP
Ik vind zelf van niet. Het valideren zelf is namelijk niet afhankelijk van welk object je valideert, alleen het geen je moet valideren is afhankelijk van het object. Je zou dan zeggen: Inheritance!! Maar dan komen we weer op een punt dat ik vaak in mijn verhaal probeer duidelijk te maken: Inheritance is zelden het goede antwoord naar mijn mening. Je zou in dit geval af kunnen met 1 algemene validator en dan metadata. Je kan ook met 1 validator werken die bijv. het Traverser/Visitor pattern gebruikt en dan je eigen Visitors maken per object.
Ik geloof niet dat ik zei dat ik dat doe, ik zei dat je data het liefst zo dicht mogelijk bij de opslag laag valideert.
Kun je iets toelichten wat je hiermee bedoelt. Ik heb de zin al een paar keer gelezen maar het kwartje valt niet.
Waarom is het volgen jou niet goed om in een UserValidator class te valideren of de gegevens voor de User correct zijn?
>> Ik geloof niet dat ik zei dat ik dat doe, ik zei dat je data het liefst zo dicht mogelijk bij de opslag laag valideert.
Was jij dat? Ik wist het niet meer. Maar ik meende dat je zoiets zei dat data uit de database niet per definitie veilig is.
Ozzie PHP op 03/04/2014 21:44:57:
>> Er is zelfs een dorp in Nederland met 2 straten die beide een straatnaam van 1 letter hebben.
Serieus? Welk dorp en welke straten??
Serieus? Welk dorp en welke straten??
Dat zijn de straten A en B in Ottoland.
Ward van der Put op 04/04/2014 07:48:33:
Dat zijn de straten A en B in Ottoland.
En vergeet E in Zuidlaren niet. ;-)
Ik heb eens een artikeltje geschreven over invoer validatie, met daarin zo ongeveer de opmerking: sluit niet uit dat jouw validatie niet deugt en dat de user het wel bij het rechte eind heeft.
Zo kan het dus zijn dat de straatnaam echt maar 1 letter heeft.
zo kan een telefoonnummer best langer zijn dan 10 cijfers. Niet alleen voor buitenlandse nummers, maar ook datanummers zijn tegenwoordig 12 cijferig
Hou het dus bij "mogelijk klopt uw invoer niet, weet u het zeker?" En niet botweg afkeuren.
Dat is inderdaad wel een hele goede!
Meestal controleer ik of er minstens 2/3 tekens zijn ingevuld om te voorkomen dat ze maar even snel een karakter invullen en klaar. Daarom heb ik dus nooit mensen gehad uit Ottoland of Zuidlaren...
Gewijzigd op 04/04/2014 11:26:35 door Michael -
Toen ik hem schreef dacht ik al, oei dit wordt te cryptisch :) Wat ik bedoel is dat de manier hoe je valideert altijd hetzelfde is. Namelijk: Je kijkt of de waarde voldoet aan de voorwaarde. Het enige wat verschilt tussen verschillende klassen is de voorwaarde (dus wat je valideert). Als je dus per klasse een nieuwe validator zou schrijven gaan je met 100% tegen het DRY principe in.
Op het eerste gezicht zou je dan direct denken aan inheritance: Gewoon een algemene validator maken voor het valideren en dan gewoon per subvalidator aangeven wat de voorwaarden zijn.
Als je dan echter wat gaat nadenken kom je tot de conclusie dat inheritance hier niet goed is. Zo ga je namelijk alsnog tegen het DRY principe in.
Da's inderdaad een bijzonder mooie tip!
@Wouter
Thanks voor de uitleg. Nu snap ik je al beter :)
Wat nu als je de validatie wél aan een aparte UserValidator class overlaat, zonder gebruik te maken van inheritance? Dus dat je zoiets krijgt:
Mag je nu ook geen statische methods meer gebruiken? :-(
En zo dan?
Code (php)
1
2
3
4
5
6
7
8
2
3
4
5
6
7
8
<?php
public function validateName($name) {
$this->input_validator->isString($name);
$this->input_validator->stringMinChar($name, 2); // minimaal 2 karakters
}
?>
public function validateName($name) {
$this->input_validator->isString($name);
$this->input_validator->stringMinChar($name, 2); // minimaal 2 karakters
}
?>