Re: What do you think CSPRNG in PHP

2022年7月17日(日) 6:33 Tim Düsterhus <tim@bastelstu.be>: > Hi > > On 7/15/22 17:54, Go Kudo wrote: > > However, there are several challenges to this. > > > > - Increased maintenance costs > > - Requires optimization for CPU architecture > > - Requires familiarity with CSPRNG > > > > PHP already bundles xxHash and appears ready to make this happen. > > > > Also, an appropriate CSPRNG implementation may be able to resolve the > > current complex macro branching. > > > > What do you think about this? > > This would be a strong no from my side. There's all types of failure > modes that decrease the security of the CSPRNG (i.e. making it insecure) > and we really don't want to be the ones to blame if something goes > wrong. And historically many non-kernel CSPRNGs later proved to be > insecure in specific situations. > > I also would assume that for a typical PHP application both of the > following is true: > - The majority of the requests don't need any randomness. > - The majority of the requests that *need* randomness don't need any > significant amount of randomness. > - The majority of the requests that need significant amounts of > randomness are fine with a regular PRNG (e.g. Xoshiro or Pcg). > - The cost of a few getrandom() syscalls is not really measurable > compared to the time spent waiting for the database, file IO or template > rendering. > > Attempting to optimize the speed of the CSPRNG is premature > optimization. That also the reason why I suggested to use the 'Secure' > engine by default in the Randomizer: It's a safe default choice for the > vast majority of users. > > Personally I likely wouldn't have merged the PR in question for the same > reasons. But at least in that case glibc is at fault :-) > > Best regards > Tim Düsterhus > Hi Tim. You are right. Implementing a CSPRNG on your own obviously increases maintenance costs and security risks. However, I still think the overhead of the getrandom syscall in a Linux environment is significant and should be considered. I would suggest deprecating mt_srand()/srand() and using php_random_bytes() in sessions etc. for PHP 8.3 for better security. However, as Nikita mentioned before, the overhead of the getrandom syscall in a Linux environment is significant and this proposal may result in performance degradation. https://github.com/php/php-src/commit/53ee3f7f897f7ee33a4c45210014648043386e13 Therefore, I have created a PoC to buffer php_random_bytes. This has resulted in a significant performance improvement. https://github.com/zeriyoshi/php-src/tree/random_buf ``` # perf result of current implementation: Samples: 120 of event 'cpu-clock:pppH', Event count (approx.): 30000000 Overhead Command Shared Object Symbol 32.50% php [kernel.kallsyms] [k] preempt_count_sub 30.00% php libc.so.6 [.] syscall 18.33% php [kernel.kallsyms] [k] do_el0_svc 2.50% php php [.] execute_ex 1.67% php [kernel.kallsyms] [k] __this_cpu_preempt_check 1.67% php [kernel.kallsyms] [k] __uaccess_mask_ptr 1.67% php [kernel.kallsyms] [k] _raw_spin_unlock_irqrestore 0.83% php [kernel.kallsyms] [k] __arm64_sys_getrandom 0.83% php [kernel.kallsyms] [k] __kern_my_cpu_offset 0.83% php [kernel.kallsyms] [k] __mod_lruvec_state 0.83% php [kernel.kallsyms] [k] __mod_memcg_state 0.83% php [kernel.kallsyms] [k] __next_zones_zonelist 0.83% php [kernel.kallsyms] [k] arch_local_irq_restore 0.83% php [kernel.kallsyms] [k] arch_local_irq_save 0.83% php [kernel.kallsyms] [k] check_stack_object 0.83% php ld-linux-aarch64.so.1 [.] 0x000000000000a260 0.83% php php [.] php_random_bytes 0.83% php php [.] syscall@plt 0.83% php php [.] zend_hash_add 0.83% php php [.] zend_hash_graceful_reverse_destroy 0.83% php php [.] zend_string_init_interned_permanent $ time ./sapi/cli/php -r 'for ($i = 0; $i < 65535; $i++){ random_bytes(4); }' real 0m0.069s user 0m0.025s sys 0m0.044s # PoC result: Samples: 19 of event 'cpu-clock:pppH', Event count (approx.): 4750000 Overhead Command Shared Object Symbol 31.58% php [kernel.kallsyms] [k] __flush_cache_user_range 15.79% php php [.] _efree 10.53% php php [.] zend_register_functions 5.26% php [kernel.kallsyms] [k] __rcu_read_unlock 5.26% php [kernel.kallsyms] [k] page_add_file_rmap 5.26% php [kernel.kallsyms] [k] pfn_valid 5.26% php [kernel.kallsyms] [k] preempt_count_sub 5.26% php libc.so.6 [.] cfree 5.26% php libc.so.6 [.] 0x0000000000097b40 5.26% php php [.] execute_ex 5.26% php php [.] zend_register_constant $ time ./sapi/cli/php -r 'for ($i = 0; $i < 65535; $i++){ random_bytes(4); }' real 0m0.016s user 0m0.009s sys 0m0.007s ``` I think this is a safe implementation due to the nature of CSPRNG, what do you think? Best Regards, Go Kudo