Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:69915 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 32346 invoked from network); 28 Oct 2013 13:24:13 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 28 Oct 2013 13:24:13 -0000 Authentication-Results: pb1.pair.com header.from=julienpauli@gmail.com; sender-id=pass Authentication-Results: pb1.pair.com smtp.mail=julienpauli@gmail.com; spf=pass; sender-id=pass Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.212.41 as permitted sender) X-PHP-List-Original-Sender: julienpauli@gmail.com X-Host-Fingerprint: 209.85.212.41 mail-vb0-f41.google.com Received: from [209.85.212.41] ([209.85.212.41:41803] helo=mail-vb0-f41.google.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 59/C5-22054-C756E625 for ; Mon, 28 Oct 2013 08:24:13 -0500 Received: by mail-vb0-f41.google.com with SMTP id w8so4654221vbj.0 for ; Mon, 28 Oct 2013 06:24:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; bh=VW/C0v4m/VupnpVfkkZG4WsmQlA/nFOkl6oXKltuTbM=; b=aSrea8VmBPkCidtHjD/Umj6cTvFvw4lF2VzQ2zcA88qiOFIX5PiSJxMyefzSwENSbc 08TNfrkQd/lSpEP4LyKhdptz2h6hNgLHAlvIh+ad5P87XZi3EXOONSbb2eJZLwz/1Zzb LKe5V9TujNbqZcdCY8bfauwexw5B5FNA4mvQF8tw2/ffPqgtPgE/5oR0BSmJ7hsuvlUw K63dURIpXA9P3aKiwIhzY2KL8RRmomU/gwxpWVXvTEOWQjUuwVJd781H4kNVrCm0ce1T 2wGhg6r9W9aDEWqG+kQUwzu2VnT7YdKdmyCKl96iNWKd/boae4F5ZewugP53Z3VF9L/h 0oVA== X-Received: by 10.58.218.161 with SMTP id ph1mr63094vec.40.1382966649981; Mon, 28 Oct 2013 06:24:09 -0700 (PDT) MIME-Version: 1.0 Sender: julienpauli@gmail.com Received: by 10.220.73.197 with HTTP; Mon, 28 Oct 2013 06:23:29 -0700 (PDT) In-Reply-To: References: <3E.D7.40084.12BBA625@pb1.pair.com> <526B554F.1020606@pthreads.org> <526CAF56.70908@pthreads.org> <526E464B.5030208@php.net> <526E4E53.5010107@php.net> Date: Mon, 28 Oct 2013 14:23:29 +0100 X-Google-Sender-Auth: 8eRFBKOd5Sh-DJTCA3e86QohP3Y Message-ID: To: Joe Watkins Cc: PHP Internals Content-Type: multipart/alternative; boundary=047d7bdc90ece891ac04e9cd033b Subject: Re: [PHP-DEV] error_log binary unsafe From: jpauli@php.net (Julien Pauli) --047d7bdc90ece891ac04e9cd033b Content-Type: text/plain; charset=ISO-8859-1 On Mon, Oct 28, 2013 at 1:03 PM, Joe Watkins wrote: > On 10/28/2013 11:45 AM, Joe Watkins wrote: > >> On 10/28/2013 11:42 AM, Julien Pauli wrote: >> >>> On Mon, Oct 28, 2013 at 12:11 PM, Joe Watkins wrote: >>> >>> On 10/28/2013 10:50 AM, Julien Pauli wrote: >>>> >>>> On Sun, Oct 27, 2013 at 7:14 AM, Joe Watkins >>>>> wrote: >>>>> >>>>> On 10/26/2013 11:54 PM, Yasuo Ohgaki wrote: >>>>> >>>>>> >>>>>> Hi Joe, >>>>>> >>>>>>> >>>>>>> On Sun, Oct 27, 2013 at 7:48 AM, Yasuo Ohgaki >>>>>> >>>>>> yohgaki@ohgaki.net>> wrote: >>>>>>> >>>>>>> On Sat, Oct 26, 2013 at 2:38 PM, Joe Watkins >>>>>>> >>>>>>> ******> >>>>>>> wrote: >>>>>>> >>>>>>> Mail is not yet handled, TCP/IP is not supported any more, >>>>>>> streams are binary safe. >>>>>>> The SAPI and default error logging mechanism are all that >>>>>>> require attention. >>>>>>> >>>>>>> The patch is not final and doesn't include a fix for every >>>>>>> implementation of SAPI. >>>>>>> >>>>>>> I don't see the need for confusion ?? >>>>>>> >>>>>>> >>>>>>> Generally speaking, I'm not against making functions/features >>>>>>> binary safe. >>>>>>> >>>>>>> There are many implementations of syslog/SAPI and not sure >>>>>>> if it >>>>>>> is good for all. >>>>>>> It could cause BC issue also. For example, application like >>>>>>> OSSEC >>>>>>> HIDS detects >>>>>>> possible intrusion by analyzing logs. Patching SAPI may break >>>>>>> these applications. >>>>>>> >>>>>>> I'm not against applying your patch to master, but it's not for >>>>>>> released versions. >>>>>>> We needs UPGRADE note if the patch is applied. >>>>>>> >>>>>>> >>>>>>> I think it's good for master. >>>>>>> Do you have commit karma? >>>>>>> If not, I'm willing to merge your patch unless there are not >>>>>>> objections. >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> -- >>>>>>> Yasuo Ohgaki >>>>>>> yohgaki@ohgaki.net >>>>>>> >>>>>>> >>>>>>> I don't have karma ... >>>>>> >>>>>> There are lots of SAPI's, but this patch wasn't meant to implement >>>>>> them >>>>>> all, only to provide a route whereby they can implement binary safe >>>>>> log_message in the shape of log_message_ex. >>>>>> >>>>>> The patch implements binsafe log for cli and cgi, do we need to >>>>>> implement >>>>>> any more ?? >>>>>> >>>>>> >>>>>> Joe: Why do you use strlen() ? This leads to the same not binary safe >>>>> string, am I wrong ?? >>>>> https://github.com/krakjoe/****php-src/commit/** >>>>> be5f38ddd449c20230c042aef9757e****fb2ee08188#diff-** >>>>> 1a9cfc6173e3a434387996e46086da****56R610>>>> krakjoe/php-src/commit/**be5f38ddd449c20230c042aef9757e** >>>>> fb2ee08188#diff-**1a9cfc6173e3a434387996e46086da**56R610 >>>>> > >>>>> >>>>> >>>>> Julien Pauli >>>>> >>>>> >>>>> (sorry if you got this twice, my interweb is playing up) >>>> >>>> Hi Julien, >>>> >>>> The binary safe logging interface for SAPI is log_message_ex >>>> and >>>> the binary safe logging function for php is php_log_err_ex >>>> The old function must remain, and use string length just as >>>> it did >>>> before. >>>> >>>> >>> I see, that means that the actual patch does not turn logging to binary >>> safe logs, but gives functions for that. >>> Turning logs to binary safe would then mean tracking every unsafe log >>> function (php_log_err()) and turn it to php_log_err_ex() with an explicit >>> string length. >>> >>> >>> Julien Pauli >>> >>> >> http://lxr.php.net/search?q=&**defs=&refs=php_log_err&path=&** >> hist=&project=PHP_5_5 >> >> >> Not the enormous task you imagine it to be :) >> >> Cheers >> Joe >> > > Actually, looking at the one occurrence of php_err_log in mysqlnd, it > doesn't need updating because it's not being used, the call to > mysqlnd_get_backtrace is wrong ... > > When it is in use, the backtrace provides a length and so we can switch to > php_err_log_ex easily. > > So, there isn't anything else to change, or not neceesarily ... > > What there is to consider is Apache and FPM: > > Apaches logging functions don't accept a length because they are > formatters. > I'm reluctant to touch apache logging at all, it could be a > massive BC break. > APR has functions for logging data that do accept a length and > would be binary safe, however, I've never used them, we want an apache geek > really to look at that ... > Knowing little bit of APR, this is "just" a library abstracting the OS. If there are apr function for binary safe logging, I guess they won't require a mass effort to implement, there shouldn't need an Apache guru IMO. The only thing to check, is the APR version they appeared, and if we are compatible with that. > > FPM has a custom logging interface (zlog) that doesn't accept a > length either, I've not looked too deeply into FPM, I'm not sure, do we > have an "fpm" expert to turn to, it never really comes up ?? > > Other SAPI's: > Theese should be updated to use binary safe logging > (log_message_ex), I've looked into the most popular, updated the cli and > cgi versions for reference. > > Oh an the mail function, php_mail, also doesn't accept a length ... > > Not sure where to go from here ... Not sure as well, if we can't turn the logging system to a full binary safe system, with no BC , then we shouldn't bother patching the 5.x branch and think about a deeper solution against master, which would then require an RFC. Julien.Pauli --047d7bdc90ece891ac04e9cd033b--