-
Notifications
You must be signed in to change notification settings - Fork 40
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Security tools : Strict Cookies, Headers, and PHPStan custom rule (…
…#101) * Add SameSite Cookie attribute * Add security headers * Add PHP Stan rule for Route Security checking
- Loading branch information
Showing
14 changed files
with
1,062 additions
and
421 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
docker-compose.override.yml | ||
/.env | ||
.ssh/ | ||
.ssh/ | ||
.idea/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
# config/packages/nelmio_security.yaml | ||
nelmio_security: | ||
# signs/verifies all cookies | ||
# FIXME : | ||
# - PHPSESSID cookie is not correctly passed into the signing process, so we cannot use '*'. But additional cookies could be set a signed here | ||
# - see https://github.com/nelmio/NelmioSecurityBundle/issues/154 | ||
signed_cookie: | ||
names: [] | ||
# prevents framing of the entire site | ||
clickjacking: | ||
paths: | ||
'^/.*': DENY | ||
hosts: | ||
# - '^foo\.com$' | ||
# - '\.example\.org$' | ||
|
||
# prevents redirections outside the website's domain | ||
external_redirects: | ||
abort: true | ||
log: true | ||
|
||
# prevents inline scripts, unsafe eval, external scripts/images/styles/frames, etc | ||
csp: | ||
hosts: [] | ||
content_types: [] | ||
enforce: | ||
level1_fallback: false | ||
browser_adaptive: | ||
enabled: false | ||
# report-uri: '%router.request_context.base_url%/nelmio/csp/report' | ||
default-src: | ||
- 'none' | ||
script-src: | ||
- 'self' | ||
block-all-mixed-content: true # defaults to false, blocks HTTP content over HTTPS transport | ||
# upgrade-insecure-requests: true # defaults to false, upgrades HTTP requests to HTTPS transport | ||
|
||
# disables content type sniffing for script resources | ||
content_type: | ||
nosniff: true | ||
|
||
# forces Microsoft's XSS-Protection with | ||
# its block mode | ||
xss_protection: | ||
enabled: true | ||
mode_block: true | ||
# report_uri: '%router.request_context.base_url%/nelmio/xss/report' | ||
|
||
# Send a full URL in the ``Referer`` header when performing a same-origin request, | ||
# only send the origin of the document to secure destination (HTTPS->HTTPS), | ||
# and send no header to a less secure destination (HTTPS->HTTP). | ||
# If ``strict-origin-when-cross-origin`` is not supported, use ``no-referrer`` policy, | ||
# no referrer information is sent along with requests. | ||
referrer_policy: | ||
enabled: true | ||
policies: | ||
- 'no-referrer' | ||
- 'strict-origin-when-cross-origin' | ||
|
||
# forces HTTPS handling, don't combine with flexible mode | ||
# and make sure you have SSL working on your site before enabling this | ||
# forced_ssl: | ||
# hsts_max_age: 2592000 # 30 days | ||
# hsts_subdomains: true | ||
# redirect_status_code: 302 # default, switch to 301 for permanent redirects | ||
|
||
# flexible HTTPS handling, read the detailed config info | ||
# and make sure you have SSL working on your site before enabling this | ||
|
||
|
||
when@prod: | ||
nelmio_security: | ||
# depends if you have a reverse proxy that will handle HTTPS, uncomment if you deploy with ansible for instance | ||
# forced_ssl: | ||
# hsts_max_age: 2592000 # 30 days | ||
# hsts_subdomains: true | ||
# redirect_status_code: 302 # default, switch to 301 for permanent redirects | ||
|
||
# Seems unnecessary because we are'nt using any insecure page in prod | ||
# flexible_ssl: | ||
# cookie_name: auth | ||
# unsecured_logout: false |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
apps/back/src/DevTools/PHPStan/IKnowWhatImDoingThisIsAPublicRoute.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace App\DevTools\PHPStan; | ||
|
||
use Attribute; | ||
|
||
/** | ||
* Add this attribute to Routes that can be accessed by ANY ONE | ||
* This is generally the case for | ||
* - public website / content | ||
* - Login / Password recovery | ||
* - If you are securing your route elsewhere (firewall, security.yml, custom function, etc.) | ||
*/ | ||
#[Attribute(Attribute::TARGET_METHOD)] | ||
class IKnowWhatImDoingThisIsAPublicRoute | ||
{ | ||
} |
178 changes: 178 additions & 0 deletions
178
apps/back/src/DevTools/PHPStan/RouteSecurityChecker.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,178 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace App\DevTools\PHPStan; | ||
|
||
use PhpParser\Node; | ||
use PhpParser\Node\Expr\MethodCall; | ||
use PhpParser\NodeTraverser; | ||
use PhpParser\NodeVisitorAbstract; | ||
use PHPStan\Analyser\Scope; | ||
use PHPStan\Node\InClassMethodNode; | ||
use PHPStan\Rules\Rule; | ||
use PHPStan\Rules\RuleError; | ||
use PHPStan\Rules\RuleErrorBuilder; | ||
use ReflectionAttribute; | ||
use ReflectionException; | ||
use ReflectionMethod; | ||
use Symfony\Component\Routing\Annotation\Route; | ||
use Symfony\Component\Security\Http\Attribute\IsGranted; | ||
|
||
use function array_filter; | ||
use function array_values; | ||
|
||
/** @implements Rule<InClassMethodNode> */ | ||
class RouteSecurityChecker implements Rule | ||
{ | ||
public function getNodeType(): string | ||
{ | ||
return InClassMethodNode::class; | ||
} | ||
|
||
/** @inheritdoc */ | ||
public function processNode(Node $node, Scope $scope): array | ||
{ | ||
\assert($node instanceof InClassMethodNode); | ||
$className = $scope->getClassReflection()?->getName(); | ||
$functionName = $scope->getFunctionName(); | ||
if (! $className || ! $functionName) { | ||
return []; | ||
} | ||
|
||
try { | ||
$reflection = new ReflectionMethod($className, $functionName); | ||
} catch (ReflectionException) { | ||
return []; | ||
} | ||
|
||
$attributes = $reflection->getAttributes(); | ||
|
||
// Parse only Routes | ||
$routeAttribute = $this->getAttribute(Route::class, $attributes); | ||
if (! $routeAttribute) { | ||
return []; | ||
} | ||
|
||
if ($this->functionAttributesContain(IKnowWhatImDoingThisIsAPublicRoute::class, $attributes)) { | ||
return []; | ||
} | ||
|
||
$isGrantedAttribute = $this->getAttribute(IsGranted::class, $attributes); | ||
|
||
// LVL 1 NO PROTECTION AT ALL :: Missing vertical controls : no IsGranted and no denyAccessUnlessGranted (even without subject) | ||
if ($isGrantedAttribute === null && ! $this->isDenyAccessUnlessGrantedCalledInRouteFunction($node, false)) { | ||
return $this->buildError( | ||
sprintf('🛑🔓 SECURITY: Route %s::%s is public !', $className, $functionName), | ||
"Add an #[IsGranted] attribute or use the `denyAccessUnlessGranted` function. | ||
If you are sure that this route should remain public, add the " . IKnowWhatImDoingThisIsAPublicRoute::class . ' attribute', | ||
); | ||
} | ||
|
||
if ($this->functionAttributesContain(ThisRouteDoesntNeedAVoter::class, $attributes)){ | ||
return []; | ||
} | ||
|
||
// LVL 2 VERTICAL ACCESS ONLY :: IsGranted is present BUT no voter is called | ||
if ($isGrantedAttribute === null && ! $this->isDenyAccessUnlessGrantedCalledInRouteFunction($node, true)) { | ||
return $this->buildError( | ||
sprintf('🛑🔓 SECURITY: Route %s::%s is insufficiently protected !', $className, $functionName), | ||
"Pass the 'subject' argument to the \$this->denyAccessUnlessGranted() call. | ||
If you are sure that this route's protection should only on user's permissions, add a ".ThisRouteDoesntNeedAVoter::class." attribute.", | ||
); | ||
} | ||
if ($isGrantedAttribute !== null){ | ||
$isGrantedAttributeInstance = $isGrantedAttribute->newInstance(); | ||
\assert($isGrantedAttributeInstance instanceof IsGranted); | ||
if ($isGrantedAttributeInstance->subject === null) { | ||
return $this->buildError( | ||
sprintf('🛑🔓 SECURITY: Route %s::%s is insufficiently protected !', $className, $functionName), | ||
"Pass the 'subject' argument to the 'IsGranted' attribute. | ||
If you are sure that this route's protection should only on user's permissions, add a ".ThisRouteDoesntNeedAVoter::class.' attribute.', | ||
); | ||
} | ||
} | ||
return []; | ||
} | ||
|
||
/** @param array<ReflectionAttribute<object>> $attributes */ | ||
private function functionAttributesContain(string $attributeClass, array $attributes): bool | ||
{ | ||
return \count($this->getAttributes($attributeClass, $attributes)) > 0; | ||
} | ||
|
||
/** | ||
* @param array<ReflectionAttribute<object>> $attributes | ||
* | ||
* @return ReflectionAttribute<object>|null | ||
*/ | ||
private function getAttribute(string $attributeClass, array $attributes): ReflectionAttribute|null | ||
{ | ||
$attributes = $this->getAttributes($attributeClass, $attributes); | ||
|
||
return \count($attributes) > 0 ? $attributes[0] : null; | ||
} | ||
|
||
/** | ||
* @param array<ReflectionAttribute<object>> $attributes | ||
* | ||
* @return array<ReflectionAttribute<object>> | ||
*/ | ||
private function getAttributes(string $attributeClass, array $attributes): array | ||
{ | ||
return array_values(array_filter( | ||
$attributes, | ||
static fn (ReflectionAttribute $attr) => $attr->getName() === $attributeClass | ||
)); | ||
} | ||
|
||
/** | ||
* @return array<RuleError> | ||
*/ | ||
private function buildError(string $message, string $tip): array | ||
{ | ||
return [ | ||
RuleErrorBuilder::message($message . "\n") | ||
->tip($tip) | ||
->build(), | ||
]; | ||
} | ||
|
||
private function isDenyAccessUnlessGrantedCalledInRouteFunction(InClassMethodNode $node, bool $requireSubject): bool | ||
{ | ||
$visitor = new class ($requireSubject) extends NodeVisitorAbstract { | ||
private bool $isSecurityCheckFunctionCalled = false; | ||
|
||
public function __construct(private readonly bool $requireSubject) | ||
{ | ||
} | ||
|
||
public function enterNode(Node $node): int|null | ||
{ | ||
if ( | ||
$node instanceof MethodCall && $node->name instanceof Node\Identifier | ||
&& $node->name->toString() === 'denyAccessUnlessGranted' | ||
&& (!$this->requireSubject || isset($node->args[1])) | ||
) { | ||
$this->isSecurityCheckFunctionCalled = true; | ||
|
||
return NodeTraverser::STOP_TRAVERSAL; | ||
} | ||
|
||
return null; | ||
} | ||
|
||
public function isIsSecurityCheckFunctionCalled(): bool | ||
{ | ||
return $this->isSecurityCheckFunctionCalled; | ||
} | ||
}; | ||
|
||
$traverser = new NodeTraverser(); | ||
$traverser->addVisitor($visitor); | ||
|
||
$traverser->traverse($node->getOriginalNode()->stmts ?? []); | ||
|
||
return $visitor->isIsSecurityCheckFunctionCalled(); | ||
} | ||
} |
17 changes: 17 additions & 0 deletions
17
apps/back/src/DevTools/PHPStan/ThisRouteDoesntNeedAVoter.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace App\DevTools\PHPStan; | ||
|
||
use Attribute; | ||
|
||
/** | ||
* Add this attribute if the Route is limited to some users with specific permissions, but no horizontal check is needed | ||
* - no "ownership" | ||
* - no user context should allow / deny user from accessing the underlying resources | ||
*/ | ||
#[Attribute(Attribute::TARGET_METHOD)] | ||
class ThisRouteDoesntNeedAVoter | ||
{ | ||
} |
Oops, something went wrong.