From ca7b37ce5a5c68ea4a105377754005a772c5deaa Mon Sep 17 00:00:00 2001 From: J0WI Date: Mon, 19 Apr 2021 15:50:30 +0200 Subject: [PATCH] Make Security module strict Signed-off-by: J0WI --- .../Security/Bruteforce/Capabilities.php | 5 +++- lib/private/Security/Certificate.php | 23 +++++++++------- lib/private/Security/CertificateManager.php | 22 ++++++++------- lib/private/Security/CredentialsManager.php | 17 +++++++----- lib/private/Security/TrustedDomainHelper.php | 7 +++-- lib/public/ICertificate.php | 27 ++++++++++--------- lib/public/ICertificateManager.php | 14 ++++++---- .../IContentSecurityPolicyManager.php | 3 +++ lib/public/Security/ICredentialsManager.php | 11 +++++--- .../Security/Bruteforce/CapabilitiesTest.php | 3 +++ .../lib/Security/Bruteforce/ThrottlerTest.php | 3 +++ .../CSP/AddContentSecurityPolicyEventTest.php | 1 + .../CSP/ContentSecurityPolicyManagerTest.php | 3 +++ .../ContentSecurityPolicyNonceManagerTest.php | 3 +++ .../Security/CSRF/CsrfTokenGeneratorTest.php | 3 +++ .../Security/CSRF/CsrfTokenManagerTest.php | 3 +++ tests/lib/Security/CSRF/CsrfTokenTest.php | 3 +++ .../CSRF/TokenStorage/SessionStorageTest.php | 3 +++ tests/lib/Security/CertificateManagerTest.php | 3 +++ tests/lib/Security/CertificateTest.php | 3 +++ tests/lib/Security/CredentialsManagerTest.php | 3 +++ tests/lib/Security/CryptoTest.php | 3 +++ tests/lib/Security/HasherTest.php | 3 +++ tests/lib/Security/IdentityProof/KeyTest.php | 3 +++ .../Security/IdentityProof/ManagerTest.php | 3 +++ .../lib/Security/IdentityProof/SignerTest.php | 3 +++ .../lib/Security/Normalizer/IpAddressTest.php | 3 +++ .../RateLimiting/Backend/MemoryCacheTest.php | 3 +++ .../lib/Security/RateLimiting/LimiterTest.php | 3 +++ tests/lib/Security/SecureRandomTest.php | 3 +++ .../lib/Security/TrustedDomainHelperTest.php | 3 +++ 31 files changed, 143 insertions(+), 50 deletions(-) diff --git a/lib/private/Security/Bruteforce/Capabilities.php b/lib/private/Security/Bruteforce/Capabilities.php index 7547348ce3..7c4c2a1367 100644 --- a/lib/private/Security/Bruteforce/Capabilities.php +++ b/lib/private/Security/Bruteforce/Capabilities.php @@ -1,4 +1,7 @@ * @@ -46,7 +49,7 @@ class Capabilities implements IPublicCapability { $this->throttler = $throttler; } - public function getCapabilities() { + public function getCapabilities(): array { if (version_compare(\OC::$server->getConfig()->getSystemValue('version', '0.0.0.0'), '12.0.0.0', '<')) { return []; } diff --git a/lib/private/Security/Certificate.php b/lib/private/Security/Certificate.php index c89122f9a4..e299f9d2b8 100644 --- a/lib/private/Security/Certificate.php +++ b/lib/private/Security/Certificate.php @@ -1,4 +1,7 @@ name = $name; $gmt = new \DateTimeZone('GMT'); @@ -75,42 +78,42 @@ class Certificate implements ICertificate { /** * @return string */ - public function getName() { + public function getName(): string { return $this->name; } /** * @return string|null */ - public function getCommonName() { + public function getCommonName(): ?string { return $this->commonName; } /** - * @return string + * @return string|null */ - public function getOrganization() { + public function getOrganization(): ?string { return $this->organization; } /** * @return \DateTime */ - public function getIssueDate() { + public function getIssueDate(): \DateTime { return $this->issueDate; } /** * @return \DateTime */ - public function getExpireDate() { + public function getExpireDate(): \DateTime { return $this->expireDate; } /** * @return bool */ - public function isExpired() { + public function isExpired(): bool { $now = new \DateTime(); return $this->issueDate > $now or $now > $this->expireDate; } @@ -118,14 +121,14 @@ class Certificate implements ICertificate { /** * @return string|null */ - public function getIssuerName() { + public function getIssuerName(): ?string { return $this->issuerName; } /** * @return string|null */ - public function getIssuerOrganization() { + public function getIssuerOrganization(): ?string { return $this->issuerOrganization; } } diff --git a/lib/private/Security/CertificateManager.php b/lib/private/Security/CertificateManager.php index ef0c656332..9734f9b644 100644 --- a/lib/private/Security/CertificateManager.php +++ b/lib/private/Security/CertificateManager.php @@ -1,4 +1,7 @@ config->getSystemValue('installed', false)) { return []; } @@ -130,7 +134,7 @@ class CertificateManager implements ICertificateManager { /** * create the certificate bundle of all trusted certificated */ - public function createCertificateBundle() { + public function createCertificateBundle(): void { $path = $this->getPathToCertificates(); $certs = $this->listCertificates(); @@ -182,7 +186,7 @@ class CertificateManager implements ICertificateManager { * @return \OCP\ICertificate * @throws \Exception If the certificate could not get added */ - public function addCertificate($certificate, $name) { + public function addCertificate(string $certificate, string $name): ICertificate { if (!Filesystem::isValidPath($name) or Filesystem::isFileBlacklisted($name)) { throw new \Exception('Filename is not valid'); } @@ -209,7 +213,7 @@ class CertificateManager implements ICertificateManager { * @param string $name * @return bool */ - public function removeCertificate($name) { + public function removeCertificate(string $name): bool { if (!Filesystem::isValidPath($name)) { return false; } @@ -226,7 +230,7 @@ class CertificateManager implements ICertificateManager { * * @return string */ - public function getCertificateBundle() { + public function getCertificateBundle(): string { return $this->getPathToCertificates() . 'rootcerts.crt'; } @@ -235,7 +239,7 @@ class CertificateManager implements ICertificateManager { * * @return string */ - public function getAbsoluteBundlePath() { + public function getAbsoluteBundlePath(): string { if (!$this->hasCertificates()) { return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt'; } @@ -250,7 +254,7 @@ class CertificateManager implements ICertificateManager { /** * @return string */ - private function getPathToCertificates() { + private function getPathToCertificates(): string { return '/files_external/'; } @@ -259,7 +263,7 @@ class CertificateManager implements ICertificateManager { * * @return bool */ - private function needsRebundling() { + private function needsRebundling(): bool { $targetBundle = $this->getCertificateBundle(); if (!$this->view->file_exists($targetBundle)) { return true; @@ -274,7 +278,7 @@ class CertificateManager implements ICertificateManager { * * @return int */ - protected function getFilemtimeOfCaBundle() { + protected function getFilemtimeOfCaBundle(): int { return filemtime(\OC::$SERVERROOT . '/resources/config/ca-bundle.crt'); } } diff --git a/lib/private/Security/CredentialsManager.php b/lib/private/Security/CredentialsManager.php index 7ba8a0020f..4688bea8dc 100644 --- a/lib/private/Security/CredentialsManager.php +++ b/lib/private/Security/CredentialsManager.php @@ -1,4 +1,7 @@ crypto->encrypt(json_encode($credentials)); $this->dbConnection->setValues(self::DB_TABLE, [ - 'user' => (string)$userId, + 'user' => $userId, 'identifier' => $identifier, ], [ 'credentials' => $value, @@ -77,7 +80,7 @@ class CredentialsManager implements ICredentialsManager { * @param string $identifier * @return mixed */ - public function retrieve($userId, $identifier) { + public function retrieve(string $userId, string $identifier) { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('credentials') ->from(self::DB_TABLE) @@ -86,7 +89,7 @@ class CredentialsManager implements ICredentialsManager { if ($userId === '') { $qb->andWhere($qb->expr()->emptyString('user')); } else { - $qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter((string)$userId))); + $qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($userId))); } $qResult = $qb->execute(); @@ -108,7 +111,7 @@ class CredentialsManager implements ICredentialsManager { * @param string $identifier * @return int rows removed */ - public function delete($userId, $identifier) { + public function delete(string $userId, string $identifier): int { $qb = $this->dbConnection->getQueryBuilder(); $qb->delete(self::DB_TABLE) ->where($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier))); @@ -116,7 +119,7 @@ class CredentialsManager implements ICredentialsManager { if ($userId === '') { $qb->andWhere($qb->expr()->emptyString('user')); } else { - $qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter((string)$userId))); + $qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($userId))); } return $qb->execute(); @@ -128,7 +131,7 @@ class CredentialsManager implements ICredentialsManager { * @param string $userId * @return int rows removed */ - public function erase($userId) { + public function erase(string $userId): int { $qb = $this->dbConnection->getQueryBuilder(); $qb->delete(self::DB_TABLE) ->where($qb->expr()->eq('user', $qb->createNamedParameter($userId))) diff --git a/lib/private/Security/TrustedDomainHelper.php b/lib/private/Security/TrustedDomainHelper.php index 8004bf7dc6..f99b505157 100644 --- a/lib/private/Security/TrustedDomainHelper.php +++ b/lib/private/Security/TrustedDomainHelper.php @@ -1,4 +1,7 @@ config->getSystemValue('overwritehost') !== '') { return true; diff --git a/lib/public/ICertificate.php b/lib/public/ICertificate.php index dbedd27430..c9e56e0e87 100644 --- a/lib/public/ICertificate.php +++ b/lib/public/ICertificate.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php index b4a7016bc2..fb28a7c9c7 100644 --- a/tests/lib/Security/Bruteforce/ThrottlerTest.php +++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CSP/AddContentSecurityPolicyEventTest.php b/tests/lib/Security/CSP/AddContentSecurityPolicyEventTest.php index ef894bb56f..7de4665ec5 100644 --- a/tests/lib/Security/CSP/AddContentSecurityPolicyEventTest.php +++ b/tests/lib/Security/CSP/AddContentSecurityPolicyEventTest.php @@ -1,6 +1,7 @@ * diff --git a/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php b/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php index 2fc7a53e78..b91482ab2c 100644 --- a/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php +++ b/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php b/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php index 783eb35eef..0e21f13e6f 100644 --- a/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php +++ b/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CSRF/CsrfTokenGeneratorTest.php b/tests/lib/Security/CSRF/CsrfTokenGeneratorTest.php index 10ab0f00c9..256199eff1 100644 --- a/tests/lib/Security/CSRF/CsrfTokenGeneratorTest.php +++ b/tests/lib/Security/CSRF/CsrfTokenGeneratorTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php index 29fbbfe3b2..44a9a7a929 100644 --- a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php +++ b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CSRF/CsrfTokenTest.php b/tests/lib/Security/CSRF/CsrfTokenTest.php index fbb92cd315..b694e79723 100644 --- a/tests/lib/Security/CSRF/CsrfTokenTest.php +++ b/tests/lib/Security/CSRF/CsrfTokenTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CSRF/TokenStorage/SessionStorageTest.php b/tests/lib/Security/CSRF/TokenStorage/SessionStorageTest.php index 8d4a966efe..f55cf49f14 100644 --- a/tests/lib/Security/CSRF/TokenStorage/SessionStorageTest.php +++ b/tests/lib/Security/CSRF/TokenStorage/SessionStorageTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CertificateManagerTest.php b/tests/lib/Security/CertificateManagerTest.php index 3af4b56461..c50296c2a3 100644 --- a/tests/lib/Security/CertificateManagerTest.php +++ b/tests/lib/Security/CertificateManagerTest.php @@ -1,4 +1,7 @@ * This file is licensed under the Affero General Public License version 3 or diff --git a/tests/lib/Security/CertificateTest.php b/tests/lib/Security/CertificateTest.php index 223fb22690..5d3fd602fb 100644 --- a/tests/lib/Security/CertificateTest.php +++ b/tests/lib/Security/CertificateTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CredentialsManagerTest.php b/tests/lib/Security/CredentialsManagerTest.php index 3ce80227f4..fb8c3df8a9 100644 --- a/tests/lib/Security/CredentialsManagerTest.php +++ b/tests/lib/Security/CredentialsManagerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/CryptoTest.php b/tests/lib/Security/CryptoTest.php index 0c7c1aa1ac..bdbad8b261 100644 --- a/tests/lib/Security/CryptoTest.php +++ b/tests/lib/Security/CryptoTest.php @@ -1,4 +1,7 @@ * This file is licensed under the Affero General Public License version 3 or diff --git a/tests/lib/Security/HasherTest.php b/tests/lib/Security/HasherTest.php index 3bd9862552..5836703504 100644 --- a/tests/lib/Security/HasherTest.php +++ b/tests/lib/Security/HasherTest.php @@ -1,4 +1,7 @@ * This file is licensed under the Affero General Public License version 3 or diff --git a/tests/lib/Security/IdentityProof/KeyTest.php b/tests/lib/Security/IdentityProof/KeyTest.php index ae5f77f160..4c852ff702 100644 --- a/tests/lib/Security/IdentityProof/KeyTest.php +++ b/tests/lib/Security/IdentityProof/KeyTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/IdentityProof/ManagerTest.php b/tests/lib/Security/IdentityProof/ManagerTest.php index 16ceec248a..0a6f97f3fc 100644 --- a/tests/lib/Security/IdentityProof/ManagerTest.php +++ b/tests/lib/Security/IdentityProof/ManagerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/IdentityProof/SignerTest.php b/tests/lib/Security/IdentityProof/SignerTest.php index 3ce211d51e..f9abf84071 100644 --- a/tests/lib/Security/IdentityProof/SignerTest.php +++ b/tests/lib/Security/IdentityProof/SignerTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/Normalizer/IpAddressTest.php b/tests/lib/Security/Normalizer/IpAddressTest.php index 044fc52b4b..b202ecd723 100644 --- a/tests/lib/Security/Normalizer/IpAddressTest.php +++ b/tests/lib/Security/Normalizer/IpAddressTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php index 902c586dc1..ff58bd5c09 100644 --- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php +++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/RateLimiting/LimiterTest.php b/tests/lib/Security/RateLimiting/LimiterTest.php index 76121a49bc..8b3509a479 100644 --- a/tests/lib/Security/RateLimiting/LimiterTest.php +++ b/tests/lib/Security/RateLimiting/LimiterTest.php @@ -1,4 +1,7 @@ * diff --git a/tests/lib/Security/SecureRandomTest.php b/tests/lib/Security/SecureRandomTest.php index 0ffd7ae7c1..7257d52e8f 100644 --- a/tests/lib/Security/SecureRandomTest.php +++ b/tests/lib/Security/SecureRandomTest.php @@ -1,4 +1,7 @@ * This file is licensed under the Affero General Public License version 3 or diff --git a/tests/lib/Security/TrustedDomainHelperTest.php b/tests/lib/Security/TrustedDomainHelperTest.php index 2796dead0e..aedc4cb6a1 100644 --- a/tests/lib/Security/TrustedDomainHelperTest.php +++ b/tests/lib/Security/TrustedDomainHelperTest.php @@ -1,4 +1,7 @@ * This file is licensed under the Affero General Public License version 3 or