Hi All, I think sanitizing the log it not the correct remedy for the "log forging" issue. We need to provide a utility to the developers to sanitize the user inputs before appending to the logs IMO. Developers should able to log with any formatted (even with CRLF) for visibility purposes.
Also IMHO, we should not use the servlet filter to "fix" the incorrect headers etc. We should fail the response sending throwing an exception to the application side. I think we can improve the performance of the code [1], there are unnecessary String search with regx, which is expensive. [1] https://github.com/wso2/carbon-kernel/pull/278/files#diff-5859ce33cfadc4c7933a6a08a605f8d1R72 Cheers, Ruwan On Thu, Nov 19, 2015 at 4:30 AM, Malithi Edirisinghe <[email protected]> wrote: > Hi Jagath, > > So as I said before this fix affects to CarbonConsoleAppender, > CarbonDailyRollingFileAppender and MemoryAppender. So if this to be > changed, either a new appender could be configured in log4j.properties or > the appender class of the CARBON_CONSOLE appender could be changed to a in > built log4j appender such as org.apache.log4j.ConsoleAppender keeping the > same layout. > > Thanks, > Malithi. > > On Thu, Nov 19, 2015 at 3:28 PM, Jagath Sisirakumara Ariyarathne < > [email protected]> wrote: > >> Hi Malithi, >> >> Is there any solution to fix this for ESB Log Mediator? Attached two >> screenshots for before and after updating kernel 4.4.2. As you can see, >> logs printed by log mediator are different and it is not the actual request >> received to the ESB with the new version. >> >> Thanks. >> >> On Thu, Nov 19, 2015 at 2:20 PM, Sajith Ariyarathna <[email protected]> >> wrote: >> >>> Hi Malithi, >>> >>> Seems that when you log.error(message, e); the stack trace does not get >>> sanitized, but if you do log.error(e); then the stack trace get >>> sanitized. >>> >>> Currently we are developing Jaggery app; when an exception occurred in >>> Jaggery code, Rhino engine logs the stack trace and that stack trace is >>> sanitized. Looks like our issue goes to Jaggery. >>> >>> I also notices that in Jaggery, logging exceptions like log.error(e); >>> outputs a sanitized stack trace. One can avoid that by logging exceptions >>> log.error(message, >>> e); like this. >>> >>> Hoping to discuss this with the Jaggery team. Thank you very much for >>> your assistance regarding this matter. >>> >>> >>> On Thu, Nov 19, 2015 at 12:26 PM, Malithi Edirisinghe <[email protected] >>> > wrote: >>> >>>> Hi Sajith, >>>> >>>> Actually the stack trace does not get sanitized. Could you please give >>>> some example. >>>> I have tried a simple test as below >>>> >>>> try { >>>> log.info("XACML policy schema loaded \n successfully."); >>>> throw new Exception("Test \n Exception"); >>>> } catch (Exception e) { >>>> log.error("Logging Test \n Exception", e); >>>> } >>>> >>>> But this prints the log properly and here the stack trace is not >>>> sanitized. Only the message that you log as an error or debug log gets >>>> sanitized. Even if you refer the code line that I have pointed above you >>>> will see it. >>>> >>>> So the output of the above code segment will be something like below. >>>> >>>> [2015-11-19 01:02:07,246] INFO >>>> {org.wso2.carbon.identity.entitlement.internal.SchemaBuilder} - XACML >>>> policy schema loaded _ successfully. (Sanitized) >>>> >>>> [2015-11-19 01:02:07,246] ERROR >>>> {org.wso2.carbon.identity.entitlement.internal.SchemaBuilder} - Logging >>>> Test _ Exception (Sanitized) >>>> >>>> java.lang.Exception: Test >>>> >>>> Exception >>>> >>>> at >>>> org.wso2.carbon.identity.entitlement.internal.SchemaBuilder.run(SchemaBuilder.java:46) >>>> >>>> at java.lang.Thread.run(Thread.java:722) >>>> >>>> Here you can clearly see that the stack trace is not sanitized. It >>>> prints the new line character in the exception as it is in the stack trace. >>>> >>>> Thanks, >>>> >>>> Malithi. >>>> >>>> On Thu, Nov 19, 2015 at 11:59 AM, Sajith Ariyarathna <[email protected] >>>> > wrote: >>>> >>>>> Hi Malithi, >>>>> >>>>> My concern is that current fix sanitizing stack traces even though >>>>> stack trace does not have CRLF injections in it. I believe sanitizing >>>>> log messages (e.g. log.error(message) , excaption.getMessage() ) >>>>> is sufficient to prevent CRLF injections in logs. >>>>> >>>>> I think we can overcome this problem with the my earlier proposed >>>>> approach (extending log4j PatternLayout class and overriding the >>>>> format method). >>>>> >>>>> Thanks. >>>>> >>>>> >>>>> On Thu, Nov 19, 2015 at 1:07 AM, Malithi Edirisinghe < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi Sajith, >>>>>> >>>>>> With the present fix there's no way that stack traces could be >>>>>> sanitized unless e.getMessage is explicitly used as the log message. >>>>>> >>>>>> As you can see at [1] only the logging message is sanitized here. >>>>>> Could you please elaborate more in case i'm misunderstanding your >>>>>> concern. >>>>>> >>>>>> [1] >>>>>> https://github.com/wso2/carbon-kernel/pull/278/files#diff-5859ce33cfadc4c7933a6a08a605f8d1R72 >>>>>> >>>>>> Thanks, >>>>>> Malithi. >>>>>> >>>>>> On Wed, Nov 18, 2015 at 6:42 PM, Sajith Ariyarathna < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi All, >>>>>>> >>>>>>> Me and RasikaP dig a little deep to find a solution to this problem. >>>>>>> >>>>>>> Instead of sanitizing final log message, you can sanitize when it is >>>>>>> formatted by extending the PatternLayout [1] class. Refer this code [2], >>>>>>> where public String format(LoggingEvent event) method is overridden >>>>>>> to achieve a custom log message formatting. You can configure log4j >>>>>>> (log4j.xml) to use your extended Pattern Layout class by adding <layout >>>>>>> class="org.apache.log4j.MyPatternLayout"> in your <appender> . >>>>>>> >>>>>>> WDYT? >>>>>>> >>>>>>> [1] >>>>>>> http://grepcode.com/file/repo1.maven.org/maven2/log4j/log4j/1.2.17/org/apache/log4j/PatternLayout.java?av=f >>>>>>> >>>>>>> [2] >>>>>>> http://apache-logging.6191.n7.nabble.com/how-to-search-and-replace-message-text-in-outgoing-log-messages-td35625.html#a35919 >>>>>>> >>>>>>> Thanks. >>>>>>> >>>>>>> On Wed, Nov 18, 2015 at 4:18 PM, Sajith Ariyarathna < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Hi Malithi, >>>>>>>> >>>>>>>> The problem with the given fix is that, even stack traces are >>>>>>>> sanitized. IMO, you don't need to sanitize stack traces. Sanitizing log >>>>>>>> messages (log.error("message"), exception.getMessage() ) is >>>>>>>> sufficient to prevent log forging. >>>>>>>> >>>>>>>> This problem affects to all products. I think we have to fix this >>>>>>>> ASAP. >>>>>>>> >>>>>>>> Thanks. >>>>>>>> >>>>>>>> On Wed, Nov 18, 2015 at 3:24 PM, Malithi Edirisinghe < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> Hi All, >>>>>>>>> >>>>>>>>> This was added for a security fix [1] and was discussed at >>>>>>>>> security-leads@ [2]. So the present fix affects to the existing >>>>>>>>> appenders (CarbonConsoleAppender, CarbonDailyRollingFileAppender, >>>>>>>>> MemoryAppender). >>>>>>>>> The other option that we could have done is to extend the existing >>>>>>>>> appenders and introduce a Secured set of appenders such that only >>>>>>>>> those >>>>>>>>> will sanitize the logging message. But, with the present fix I'm >>>>>>>>> afraid >>>>>>>>> that other than configuring the appender at log4j.properties to some >>>>>>>>> in >>>>>>>>> built log4j appender we won't be able to get rid of this sanitization >>>>>>>>> logic >>>>>>>>> at logging. >>>>>>>>> >>>>>>>>> [1] https://support.wso2.com/jira/browse/SECINTDEV-5 >>>>>>>>> [2] 'Preventing CRLF Injection when logging' >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Malithi. >>>>>>>>> >>>>>>>>> On Wed, Nov 18, 2015 at 3:05 PM, Viraj Senevirathne < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> Hi Krishantha, >>>>>>>>>> >>>>>>>>>> We have observed that* Log Mediator in ESB* is affected due to >>>>>>>>>> this change. If there are new lines in the message payload it very >>>>>>>>>> inconvenient and hard to read the logs. And user cannot see actual >>>>>>>>>> payload >>>>>>>>>> as it is, because this functionality change the message log. >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> >>>>>>>>>> On Wed, Nov 18, 2015 at 2:58 PM, Sajith Ariyarathna < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> Hi All, >>>>>>>>>>> >>>>>>>>>>> We are using carbon.kernel.version 4.4.2 in MDM 2.0.0 SNAPSHOT >>>>>>>>>>> and we face the same problem (new lines are replaced with >>>>>>>>>>> underscores in >>>>>>>>>>> logs). Because of this behavior, it is very hard to debug/find >>>>>>>>>>> problems by >>>>>>>>>>> reading error logs. Is there any way to by pass/stop this behavior >>>>>>>>>>> without >>>>>>>>>>> patching the carbon kernel? >>>>>>>>>>> >>>>>>>>>>> Thanks. >>>>>>>>>>> >>>>>>>>>>> On Fri, Oct 30, 2015 at 11:57 AM, Viraj Senevirathne < >>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Kasun, >>>>>>>>>>>> >>>>>>>>>>>> It seems that it has happened due to this commit >>>>>>>>>>>> https://github.com/wso2/carbon-kernel/commit/e0b6ae7d9f4cdee2f0bf3744b2a3ce02c3b808bf >>>>>>>>>>>> . We removed it and patched the kernel then issue was resolved. >>>>>>>>>>>> What can we >>>>>>>>>>>> do about this? >>>>>>>>>>>> >>>>>>>>>>>> Thank You, >>>>>>>>>>>> >>>>>>>>>>>> On Fri, Oct 30, 2015 at 9:15 AM, Kasun Gajasinghe < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Can you guys go through recent commits to >>>>>>>>>>>>> org.wso2.carbon.logging component and find out if any of those >>>>>>>>>>>>> caused this >>>>>>>>>>>>> issue? >>>>>>>>>>>>> >>>>>>>>>>>>> On Oct 29, 2015, at 9:23 PM, Jagath Sisirakumara Ariyarathne < >>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Hi Carbon Team, >>>>>>>>>>>>> >>>>>>>>>>>>> Any thought to figure out the issue is much appreciated. >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks. >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, Oct 28, 2015 at 3:42 PM, Viraj Senevirathne < >>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi All, >>>>>>>>>>>>>> >>>>>>>>>>>>>> We have upgraded the carbon version in ESB from 4.4.1 to >>>>>>>>>>>>>> 4.4.2. Then we have encountered following issue. >>>>>>>>>>>>>> >>>>>>>>>>>>>> *In carbon.kernel.version 4.4.1 and earlier carbon versions* >>>>>>>>>>>>>> >>>>>>>>>>>>>> TID: [-1234] [] [2015-10-23 16:43:26,614] INFO >>>>>>>>>>>>>> {org.apache.synapse.mediators.builtin.LogMediator} - To: >>>>>>>>>>>>>> /services/sendReciveProxy.sendReciveProxyHttpSoap11Endpoint, >>>>>>>>>>>>>> WSAction: >>>>>>>>>>>>>> urn:getQuote, SOAPAction: urn:getQuote, MessageID: >>>>>>>>>>>>>> urn:uuid:333b6811-04aa-4c6a-94fb-3edc4d56065d, Direction: >>>>>>>>>>>>>> request, >>>>>>>>>>>>>> Envelope: <?xml version='1.0' encoding='utf-8'?><soapenv:Envelope >>>>>>>>>>>>>> xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/"; >>>>>>>>>>>>>> xmlns:xsd="http://services.samples/xsd"; xmlns:ser=" >>>>>>>>>>>>>> http://services.samples";><soapenv:Body> >>>>>>>>>>>>>> <ser:getQuote> >>>>>>>>>>>>>> <!--Optional:--> >>>>>>>>>>>>>> <ser:request> >>>>>>>>>>>>>> <!--Optional:--> >>>>>>>>>>>>>> <xsd:symbol>IBM</xsd:symbol> >>>>>>>>>>>>>> </ser:request> >>>>>>>>>>>>>> </ser:getQuote> >>>>>>>>>>>>>> </soapenv:Body></soapenv:Envelope> >>>>>>>>>>>>>> {org.apache.synapse.mediators.builtin.LogMediator} >>>>>>>>>>>>>> >>>>>>>>>>>>>> *Same log in carbon.kernel.version 4.4.2 * >>>>>>>>>>>>>> >>>>>>>>>>>>>> [2015-10-28 15:38:36,027] INFO - LogMediator To: >>>>>>>>>>>>>> /services/callOutOnly.callOutOnlyHttpSoap11Endpoint, WSAction: >>>>>>>>>>>>>> urn:mediate, >>>>>>>>>>>>>> SOAPAction: urn:mediate, MessageID: >>>>>>>>>>>>>> urn:uuid:61f4b04c-0906-4228-975e-1b8f1be7450d, Direction: >>>>>>>>>>>>>> request, >>>>>>>>>>>>>> Envelope: <?xml version='1.0' encoding='utf-8'?><soapenv:Envelope >>>>>>>>>>>>>> xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/"; >>>>>>>>>>>>>> xmlns:wsa="http://www.w3.org/2005/08/addressing";><soapenv:Body>_ >>>>>>>>>>>>>> <m:placeOrder xmlns:m="http://services.samples";>_ >>>>>>>>>>>>>> <m:order>_ <m:price>3.141593E0</m:price>_ >>>>>>>>>>>>>> <m:quantity>4</m:quantity>_ <m:symbol>IBM</m:symbol>_ >>>>>>>>>>>>>> </m:order>_ </m:placeOrder>_ >>>>>>>>>>>>>> </soapenv:Body></soapenv:Envelope> (Sanitized) >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> As you can see all the new lines are replaced with _ . >>>>>>>>>>>>>> >>>>>>>>>>>>>> What could be the issue here? >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you, >>>>>>>>>>>>>> >>>>>>>>>>>>>> -- >>>>>>>>>>>>>> Viraj Senevirathne >>>>>>>>>>>>>> Software Engineer; WSO2, Inc. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Mobile : +94 71 958 0269 >>>>>>>>>>>>>> Email : [email protected] >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>>> Jagath Ariyarathne >>>>>>>>>>>>> Technical Lead >>>>>>>>>>>>> WSO2 Inc. http://wso2.com/ >>>>>>>>>>>>> Email: [email protected] >>>>>>>>>>>>> Mob : +94 77 386 7048 >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Viraj Senevirathne >>>>>>>>>>>> Software Engineer; WSO2, Inc. >>>>>>>>>>>> >>>>>>>>>>>> Mobile : +94 71 958 0269 >>>>>>>>>>>> Email : [email protected] >>>>>>>>>>>> >>>>>>>>>>>> _______________________________________________ >>>>>>>>>>>> Dev mailing list >>>>>>>>>>>> [email protected] >>>>>>>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Sajith Ariyarathna >>>>>>>>>>> Software Engineer; WSO2, Inc.; http://wso2.com/ >>>>>>>>>>> mobile: +94 77 6602284, +94 71 3951048 >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Viraj Senevirathne >>>>>>>>>> Software Engineer; WSO2, Inc. >>>>>>>>>> >>>>>>>>>> Mobile : +94 71 958 0269 >>>>>>>>>> Email : [email protected] >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> *Malithi Edirisinghe* >>>>>>>>> Senior Software Engineer >>>>>>>>> WSO2 Inc. >>>>>>>>> >>>>>>>>> Mobile : +94 (0) 718176807 >>>>>>>>> [email protected] >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Sajith Ariyarathna >>>>>>>> Software Engineer; WSO2, Inc.; http://wso2.com/ >>>>>>>> mobile: +94 77 6602284, +94 71 3951048 >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Sajith Ariyarathna >>>>>>> Software Engineer; WSO2, Inc.; http://wso2.com/ >>>>>>> mobile: +94 77 6602284, +94 71 3951048 >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> >>>>>> *Malithi Edirisinghe* >>>>>> Senior Software Engineer >>>>>> WSO2 Inc. >>>>>> >>>>>> Mobile : +94 (0) 718176807 >>>>>> [email protected] >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Sajith Ariyarathna >>>>> Software Engineer; WSO2, Inc.; http://wso2.com/ >>>>> mobile: +94 77 6602284, +94 71 3951048 >>>>> >>>> >>>> >>>> >>>> -- >>>> >>>> *Malithi Edirisinghe* >>>> Senior Software Engineer >>>> WSO2 Inc. >>>> >>>> Mobile : +94 (0) 718176807 >>>> [email protected] >>>> >>> >>> >>> >>> -- >>> Sajith Ariyarathna >>> Software Engineer; WSO2, Inc.; http://wso2.com/ >>> mobile: +94 77 6602284, +94 71 3951048 >>> >>> _______________________________________________ >>> Dev mailing list >>> [email protected] >>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>> >>> >> >> >> -- >> Jagath Ariyarathne >> Technical Lead >> WSO2 Inc. http://wso2.com/ >> Email: [email protected] >> Mob : +94 77 386 7048 >> >> > > > -- > > *Malithi Edirisinghe* > Senior Software Engineer > WSO2 Inc. > > Mobile : +94 (0) 718176807 > [email protected] > > _______________________________________________ > Dev mailing list > [email protected] > http://wso2.org/cgi-bin/mailman/listinfo/dev > > -- *Ruwan Abeykoon* *Architect,* *WSO2, Inc. http://wso2.com <http://wso2.com/> * *lean.enterprise.middleware.* email: [email protected]
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
