| @@ -17,18 +17,8 @@ | ||
| 17 | 17 | // Performance optimization: Key cache |
| 18 | 18 | private static array $keyCache = []; |
| 19 | 19 | private static int $maxCacheSize = 10; // Limit cache size to prevent memory issues |
| 20 | 20 | |
| 21 | - /** | |
| 22 | - * Performance optimization: one salt per request. | |
| 23 | - * | |
| 24 | - * The salt is generated once per page load and reused for every encryption of that | |
| 25 | - * request, so the PBKDF2 key derivation runs once instead of once per address. | |
| 26 | - * This is safe: AES-GCM requires unique IVs, not unique salts - and the IV is still | |
| 27 | - * generated freshly for every single encrypt() call (see encrypt()). | |
| 28 | - */ | |
| 29 | - private static ?string $requestSalt = null; | |
| 30 | - | |
| 31 | 21 | // Performance optimization: Pre-check cipher availability |
| 32 | 22 | private static ?bool $cipherAvailable = null; |
| 33 | 23 | |
| 34 | 24 | /** |
| @@ -45,26 +35,8 @@ | ||
| 45 | 35 | return self::$cipherAvailable; |
| 46 | 36 | } |
| 47 | 37 | |
| 48 | 38 | /** |
| 49 | - * Returns the salt for the current request, generating it on first use. | |
| 50 | - * | |
| 51 | - * Reusing the salt within one request is what makes the key cache effective: | |
| 52 | - * all addresses of a page share the same password anyway, so they may share the | |
| 53 | - * derived key. Uniqueness of the ciphertext is provided by the per-encryption IV. | |
| 54 | - * | |
| 55 | - * @return string | |
| 56 | - * @throws \Exception | |
| 57 | - */ | |
| 58 | - private static function getRequestSalt(): string | |
| 59 | - { | |
| 60 | - if (self::$requestSalt === null) { | |
| 61 | - self::$requestSalt = random_bytes(self::SALT_LENGTH); | |
| 62 | - } | |
| 63 | - return self::$requestSalt; | |
| 64 | - } | |
| 65 | - | |
| 66 | - /** | |
| 67 | 39 | * Derives a key from password using PBKDF2 with caching - compatible with JavaScript |
| 68 | 40 | * |
| 69 | 41 | * @param string $password |
| 70 | 42 | * @param string $salt |
| @@ -76,13 +48,10 @@ | ||
| 76 | 48 | if (!function_exists('hash_pbkdf2')) { |
| 77 | 49 | throw new \Exception('PBKDF2 not available'); |
| 78 | 50 | } |
| 79 | 51 | |
| 80 | - // Performance optimization: Cache derived keys. | |
| 81 | - // The iteration count is part of the cache key: setIterations() may change it | |
| 82 | - // within a single request, and the same password+salt yields a different key | |
| 83 | - // for a different iteration count. | |
| 84 | - $cacheKey = hash('sha256', self::getIterations() . '|' . $password . $salt); | |
| 52 | + // Performance optimization: Cache derived keys | |
| 53 | + $cacheKey = hash('sha256', $password . $salt); | |
| 85 | 54 | |
| 86 | 55 | if (isset(self::$keyCache[$cacheKey])) { |
| 87 | 56 | return self::$keyCache[$cacheKey]; |
| 88 | 57 | } |
| @@ -117,17 +86,13 @@ | ||
| 117 | 86 | } |
| 118 | 87 | |
| 119 | 88 | self::setIterations(); |
| 120 | 89 | |
| 121 | - // Performance optimization: the salt is generated once per request, which lets | |
| 122 | - // the key cache do its job (one PBKDF2 run per page instead of one per address). | |
| 123 | - $salt = self::getRequestSalt(); | |
| 90 | + // Performance optimization: Generate salt and IV in one call | |
| 91 | + $randomBytes = random_bytes(self::SALT_LENGTH + self::IV_LENGTH); | |
| 92 | + $salt = substr($randomBytes, 0, self::SALT_LENGTH); | |
| 93 | + $iv = substr($randomBytes, self::SALT_LENGTH); | |
| 124 | 94 | |
| 125 | - // SECURITY: the IV must NEVER be cached or reused. Reusing an IV with the same | |
| 126 | - // key breaks AES-GCM completely (keystream reuse, forgeable auth tag). | |
| 127 | - // Therefore random_bytes() runs on every single encrypt() call. | |
| 128 | - $iv = random_bytes(self::IV_LENGTH); | |
| 129 | - | |
| 130 | 95 | // Derive key from password (now with caching) |
| 131 | 96 | $key = self::deriveKey($password, $salt); |
| 132 | 97 | |
| 133 | 98 | // Encrypt data |
| @@ -178,20 +143,30 @@ | ||
| 178 | 143 | |
| 179 | 144 | /** |
| 180 | 145 | * Clears the key cache - useful for memory management |
| 181 | 146 | * |
| 182 | - * Also drops the remembered request salt, so the next encrypt() starts from a | |
| 183 | - * freshly generated salt and a genuinely empty cache. | |
| 184 | - * | |
| 185 | 147 | * @return void |
| 186 | 148 | */ |
| 187 | 149 | public static function clearKeyCache(): void |
| 188 | 150 | { |
| 189 | 151 | self::$keyCache = []; |
| 190 | - self::$requestSalt = null; | |
| 191 | 152 | } |
| 192 | 153 | |
| 193 | 154 | /** |
| 155 | + * Gets current cache statistics | |
| 156 | + * | |
| 157 | + * @return array | |
| 158 | + */ | |
| 159 | + public static function getCacheStats(): array | |
| 160 | + { | |
| 161 | + return [ | |
| 162 | + 'cache_size' => count(self::$keyCache), | |
| 163 | + 'max_cache_size' => self::$maxCacheSize, | |
| 164 | + 'memory_usage_bytes' => memory_get_usage(), | |
| 165 | + ]; | |
| 166 | + } | |
| 167 | + | |
| 168 | + /** | |
| 194 | 169 | * Decrypts encrypted data using AES-256-GCM - JavaScript compatible |
| 195 | 170 | * |
| 196 | 171 | * @param string $encryptedData Base64 encoded encrypted data |
| 197 | 172 | * @param string $password |
| @@ -203,14 +178,8 @@ | ||
| 203 | 178 | if (!function_exists('openssl_decrypt')) { |
| 204 | 179 | throw new \Exception('OpenSSL extension not available'); |
| 205 | 180 | } |
| 206 | 181 | |
| 207 | - // Without this the iteration count is whatever a previous encrypt() | |
| 208 | - // happened to leave behind -- or the built-in default, if this request | |
| 209 | - // only ever decrypts. With a configured count other than 10000 the key | |
| 210 | - // derivation would then silently produce the wrong key. | |
| 211 | - self::setIterations(); | |
| 212 | - | |
| 213 | 182 | try { |
| 214 | 183 | $combined = base64_decode($encryptedData, true); |
| 215 | 184 | if ($combined === false) { |
| 216 | 185 | throw new \Exception('Invalid base64 encoding'); |
| @@ -254,24 +223,51 @@ | ||
| 254 | 223 | } |
| 255 | 224 | |
| 256 | 225 | return $decrypted; |
| 257 | 226 | } catch (\Throwable $e) { |
| 258 | - throw new \Exception('Decryption failed: ' . esc_html($e->getMessage())); | |
| 227 | + throw new \Exception('Decryption failed: ' . $e->getMessage()); | |
| 259 | 228 | } |
| 260 | 229 | } |
| 261 | 230 | |
| 262 | - // debugEncryption() and getCacheStats() used to sit here. The second was | |
| 263 | - // only ever called by the first, so removing one left the other behind -- | |
| 264 | - // which is how dead code usually spreads. | |
| 265 | - // | |
| 266 | - // debugEncryption(): it encrypted a string, decrypted it | |
| 267 | - // again and returned both, plus timings and the key-cache statistics. It | |
| 268 | - // had no caller anywhere in the plugin and was shipped to every site all | |
| 269 | - // the same. Nothing was reachable through it -- it is a static method, not | |
| 270 | - // an endpoint -- but a method that hands back a plaintext next to its | |
| 271 | - // ciphertext is a poor thing to leave lying around for the next person who | |
| 272 | - // needs somewhere to hook a quick diagnosis. | |
| 231 | + /** | |
| 232 | + * Test encryption/decryption with debug output | |
| 233 | + * | |
| 234 | + * @param string $plaintext | |
| 235 | + * @param string $password | |
| 236 | + * @return array Debug information | |
| 237 | + */ | |
| 238 | + public static function debugEncryption(string $plaintext, string $password): array | |
| 239 | + { | |
| 240 | + try { | |
| 241 | + $startTime = microtime(true); | |
| 242 | + $encrypted = self::encrypt($plaintext, $password); | |
| 243 | + $encryptTime = microtime(true) - $startTime; | |
| 273 | 244 | |
| 245 | + $startTime = microtime(true); | |
| 246 | + $decrypted = self::decrypt($encrypted, $password); | |
| 247 | + $decryptTime = microtime(true) - $startTime; | |
| 248 | + | |
| 249 | + return [ | |
| 250 | + 'success' => true, | |
| 251 | + 'plaintext' => $plaintext, | |
| 252 | + 'encrypted' => $encrypted, | |
| 253 | + 'decrypted' => $decrypted, | |
| 254 | + 'match' => ($plaintext === $decrypted), | |
| 255 | + 'encrypted_length' => strlen($encrypted), | |
| 256 | + 'binary_length' => strlen(base64_decode($encrypted)), | |
| 257 | + 'encrypt_time_ms' => round($encryptTime * 1000, 2), | |
| 258 | + 'decrypt_time_ms' => round($decryptTime * 1000, 2), | |
| 259 | + 'cache_stats' => self::getCacheStats() | |
| 260 | + ]; | |
| 261 | + } catch (\Exception $e) { | |
| 262 | + return [ | |
| 263 | + 'success' => false, | |
| 264 | + 'error' => $e->getMessage(), | |
| 265 | + 'plaintext' => $plaintext | |
| 266 | + ]; | |
| 267 | + } | |
| 268 | + } | |
| 269 | + | |
| 274 | 270 | /** |
| 275 | 271 | * Validates URL for security |
| 276 | 272 | * |
| 277 | 273 | * @param string $url |
| @@ -285,9 +281,9 @@ | ||
| 285 | 281 | if (strlen($url) > $maxLength) { |
| 286 | 282 | return false; |
| 287 | 283 | } |
| 288 | 284 | |
| 289 | - $parsedUrl = wp_parse_url($url); | |
| 285 | + $parsedUrl = parse_url($url); | |
| 290 | 286 | if (!$parsedUrl || !isset($parsedUrl['scheme'])) { |
| 291 | 287 | return false; |
| 292 | 288 | } |
| 293 | 289 | |
| @@ -315,28 +311,13 @@ | ||
| 315 | 311 | { |
| 316 | 312 | return self::$iterations; |
| 317 | 313 | } |
| 318 | 314 | |
| 319 | - /** Bounds for the PBKDF2 iteration count taken from the stored option. */ | |
| 320 | - private const MIN_ITERATIONS = 1000; | |
| 321 | - private const MAX_ITERATIONS = 1000000; | |
| 322 | - | |
| 323 | 315 | private static function setIterations(): void |
| 324 | 316 | { |
| 325 | 317 | $config = new Config(get_option('cryptX', [])); |
| 326 | - $configured = $config->get('iterations', self::$iterations); | |
| 318 | + self::$iterations = $config->get('iterations', self::getIterations()); | |
| 327 | 319 | |
| 328 | - // The option is not necessarily a sane integer: the settings page keeps | |
| 329 | - // it as a string, and a hand-edited row can hold anything. A zero makes | |
| 330 | - // hash_pbkdf2() throw a ValueError and a non-numeric string a TypeError | |
| 331 | - // -- neither of which is an \Exception, so the fallback in | |
| 332 | - // CryptX::encryptEmailAddressSecure() would not catch them and the | |
| 333 | - // front end would fatal on every page carrying an address. | |
| 334 | - if (!is_numeric($configured)) { | |
| 335 | - return; | |
| 336 | - } | |
| 337 | - | |
| 338 | - self::$iterations = max(self::MIN_ITERATIONS, min(self::MAX_ITERATIONS, (int) $configured)); | |
| 339 | 320 | } |
| 340 | 321 | |
| 341 | 322 | /** |
| 342 | 323 | * Get configuration for JavaScript |
