Hi Jacques, That is actually a nice idea. debugOfbiz reads like a verb. What does debugOfbiz do? It debugs ofbiz :)
I am not sure however, if task shortcuts apply to rule-tasks. Are you sure they work in the short form? I ask because rule tasks are constructed from regex so it would be weird if they do work. Taher Alkhateeb On Aug 14, 2016 11:29 AM, "Jacques Le Roux" <[email protected]> wrote: > Hi Taher, > > Inline > > Le 14/08/2016 à 09:31, Taher Alkhateeb a écrit : > >> Hi Jacques, >> >> I removed ofbizSecure and ofbizBackgroundSecure in OFBIZ-7951 at r1756305. >> You can start working on renaming. >> >> When renaming the tasks, please make sure to update >> StartupCommandUtil.java >> and README.md to reflect the changed names. I think we also need to update >> the demos to use ofbiz instead of ofbizSecure right? >> > > Yep, all that's planned > > >> There is one slight disadvantage of renaming ofbizBackground to background >> and ofbizDebug to debug in that it does not become obvious to users that >> this is an ofbiz server command (they all start with the word ofbiz). >> There >> is also a small chance of name clashing with plugins that we might >> introduce in the future. I'm stating this here in case we face such an >> issue in the future to be aware of it and fix it. >> > > Mmm... this is something I did not think about. I'll rather rename them > debugOfbiz and backgroundOfbiz in order to keep the the differentiation. > Still the shortcuts can be used :) > > Jacques > > >> Taher Alkhateeb >> >> On Sat, Aug 13, 2016 at 1:23 AM, Jacques Le Roux < >> [email protected]> wrote: >> >> Hi Taher, >>> >>> I agree, to confirm: >>> >>> I'll revert my local changes (but removing the debug task), there are a >>> breeze to "redo" anyway (not alike of course), it's just plain global S/R >>> >>> Then we will use "background" on trunk demo, when you will have done >>> OFBIZ-7951 and I have renamed the ofbizBackground pattern to background >>> >>> BuildBot is another thing, it notably builds and test so does not need a >>> secure task, it loads demo data and tests. As a committer you can see the >>> script here https://svn.apache.org/repos/infra/infrastructure/buildbot/a >>> egis/buildmaster/master1/projects/ofbiz.conf >>> >>> I need to update and complete the wiki documentation regarding Gradle and >>> I trust you about explaining things in the README.MD >>> >>> I'll make obvious that by default we only trace and not reject >>> deserialisation >>> >>> Jacques >>> >>> >>> >>> Le 13/08/2016 à 00:01, Taher Alkhateeb a écrit : >>> >>> Hi jacques, >>>> >>>> Okay great thank you for the thorough explanation. So based on your >>>> feedback I think that it does not make any difference whether we keep or >>>> delete the ofbizSecure and ofbizBackgroundSecure because the work has to >>>> be >>>> done either way to ensure that the OOTB whitelist is satisfied and >>>> whether >>>> notsoserial is default or not does not affect anyone during development >>>> or >>>> production. So I think we can safely remove these tasks and default them >>>> into the other tasks, the completion of the whitelist is another task >>>> for >>>> another day. >>>> >>>> Can I go ahead with the task or are you currently working on >>>> intersecting >>>> work? Would you mind helping out in switching buildbot to ofbiz instead >>>> of >>>> ofbizSecure? >>>> >>>> Taher Alkhateeb >>>> >>>> On Fri, Aug 12, 2016 at 11:22 PM, Jacques Le Roux < >>>> [email protected]> wrote: >>>> >>>> Le 12/08/2016 à 20:51, Taher Alkhateeb a écrit : >>>> >>>>> Hi Jacques, >>>>> >>>>>> Ok so I'm a little confused now. If the whitelist is incomplete and we >>>>>> did >>>>>> not exhaust the investigation, then why are we running the demo on >>>>>> ofbizSecure? >>>>>> >>>>>> The main reason is we have no known deserialization issues OOTB. >>>>>> >>>>> Initially we had issues and I planned to complete the list. But then >>>>> thought that anyway, since we run it only during a day, it will never >>>>> be >>>>> complete and I gave up. >>>>> >>>>> Or actually, let me ask you in another why ... When should we prefer >>>>> _not_ >>>>> >>>>> to have ofbizSecure and why? >>>>>> >>>>>> When you are sure to not have any deserialization issues. For >>>>>> instance, >>>>>> >>>>> as >>>>> soon as you introduce RMI you are at risk. Notsoserial has a list of >>>>> know >>>>> issues https://github.com/kantega/notsoserial#which-classes-are-rej >>>>> ected >>>>> Note that we don't have these issues in OFBiz: see OFBIZ-6726, >>>>> OFBIZ-6568 >>>>> and OFBIZ-6942 as explained at https://cwiki.apache.org/confl >>>>> uence/display/OFBIZ/The+infamous+Java+serialization+vulnerability >>>>> >>>>> Jacques >>>>> >>>>> >>>>> >>>>> >>>>> Taher Alkhateeb >>>>> >>>>>> On Fri, Aug 12, 2016 at 9:42 PM, Jacques Le Roux < >>>>>> [email protected]> wrote: >>>>>> >>>>>> I conceived the Ant secure target with demo in mind, thinking users >>>>>> would >>>>>> >>>>>> adapt it to their needs. >>>>>>> On trunk demo we use an empty whitelist, and trace deserialization. >>>>>>> And >>>>>>> because it must run w/o manual intervention we don't reject >>>>>>> deserialization, we trace them. >>>>>>> At https://cwiki.apache.org/confluence/display/OFBIZ/The+infamo >>>>>>> us+Java+serialization+vulnerability I recommend to reject >>>>>>> deserialization >>>>>>> entirely (ie "just use an empty whitelist") >>>>>>> https://github.com/kantega/not >>>>>>> soserial#rejecting-deserialization-entirely >>>>>>> This is why I picked notsoserial over all other solutions. If you >>>>>>> use >>>>>>> this option you are safe! The idea is to test your code before >>>>>>> production >>>>>>> and to add libs in the whitelist if needed. >>>>>>> >>>>>>> It's difficult to set a whitelist OOTB because it depends on so many >>>>>>> things, so far today we hit those on trunk demo (most of the time I >>>>>>> saw >>>>>>> only that) >>>>>>> java.util.HashMap >>>>>>> java.lang.Boolean >>>>>>> java.lang.Integer >>>>>>> java.lang.Number >>>>>>> org.apache.derby.iapi.services.io.FormatIdInputStream.readObject >>>>>>> >>>>>>> So if we remove ofbizSecure and ofbizBackgroundSecure by including >>>>>>> them >>>>>>> by >>>>>>> default we indeed need to create an as far as possible complete OOTB >>>>>>> whitelist >>>>>>> This needs some work I can't do, even if the trunk demo could be used >>>>>>> appropriately by reporting each day the deserializations done. >>>>>>> >>>>>>> If we want to complete the list using the trunk demo we would still >>>>>>> need >>>>>>> a >>>>>>> mean to trace the deserializations there. I see 2 options >>>>>>> 1) a specific Gradle task using the notsoserial tracing and report >>>>>>> about >>>>>>> it. It's easier but defeats the purpose of removing ofbizSecure and >>>>>>> ofbizBackgroundSecure >>>>>>> 2) simply using the daily logs and look for "|Deserialization not >>>>>>> allowed >>>>>>> for class"|. It's not that more work, in both cases we need to fetch >>>>>>> the >>>>>>> info and parse. Most of the time, this should have a limited impact >>>>>>> on >>>>>>> the >>>>>>> demo usability. >>>>>>> ||| >>>>>>> |We could also decide to neglect this aspect, create an OOTB reputed >>>>>>> to >>>>>>> be >>>>>>> incomplete whitelist and benefit from adopters researches to complete >>>>>>> it. >>>>>>> Then we need to carefully document what we are doing for adopters to >>>>>>> easily >>>>>>> be safe. >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> Le 07/08/2016 à 11:44, Taher Alkhateeb a écrit : >>>>>>> >>>>>>> Hi Scott, >>>>>>> >>>>>>> Yeah agreed. I think logging for failed serialization might be a bit >>>>>>>> heavy because you might need to use reflections or custom class >>>>>>>> loaders >>>>>>>> to >>>>>>>> dig around and provide useful messages. If a runtime exceptions >>>>>>>> bubbles >>>>>>>> up >>>>>>>> to main then it will show up in the console and I think this could >>>>>>>> be enough for developers to investigate. >>>>>>>> >>>>>>>> On Sunday, 7 August 2016, Scott Gray<[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>> I would suggest enabling the whitelist by default, adding whatever >>>>>>>> classes >>>>>>>> >>>>>>>> OFBiz needs OOTB and then having a clear failure message in the logs >>>>>>>> >>>>>>>>> when a >>>>>>>>> custom class fails serialization. Would that work? >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> Scott >>>>>>>>> >>>>>>>>> On 6/08/2016 23:13, "Jacques Le Roux" < >>>>>>>>> [email protected] >>>>>>>>> <javascript:;>> wrote: >>>>>>>>> >>>>>>>>> I'd not be against but we need to be clear while documenting that >>>>>>>>> it's >>>>>>>>> not >>>>>>>>> >>>>>>>>> enough for security (when needed, users need to refer to the wiki >>>>>>>>> >>>>>>>>> page), >>>>>>>>>> >>>>>>>>>> a >>>>>>>>>> >>>>>>>>>> white list is necessary (again only when needed, not OOTB) >>>>>>>>> >>>>>>>>> I guess (at least I hope for them) most sysadmin, devops are aware >>>>>>>>>> of >>>>>>>>>> the >>>>>>>>>> possible issue, but simpler users need to be warned... >>>>>>>>>> >>>>>>>>>> Jacques >>>>>>>>>> >>>>>>>>>> Le 06/08/2016 à 12:49, Taher Alkhateeb a écrit : >>>>>>>>>> >>>>>>>>>> Hi Jacques, >>>>>>>>>> >>>>>>>>>> As I referred to earlier I suggest the following: >>>>>>>>>> >>>>>>>>>>> - remove ofbizSecure and ofbizBackgroundSecure >>>>>>>>>>> - make all other server tasks secure by default (i.e. loading >>>>>>>>>>> >>>>>>>>>>> notsoserial >>>>>>>>>>> >>>>>>>>>>> and all other jvm args which are currently used in ofbizSecure). >>>>>>>>>> This >>>>>>>>>> >>>>>>>>>> means >>>>>>>>>> >>>>>>>>>>> ofbiz, ofbizBackground and ofbizDebug >>>>>>>>>>> - update the documentation so that users need not worry about >>>>>>>>>>> calling >>>>>>>>>>> >>>>>>>>>>> any >>>>>>>>>>> >>>>>>>>>>> secure tasks. So they only need to do custom work such as the >>>>>>>>>> whitelist, >>>>>>>>>> >>>>>>>>>> etc ... >>>>>>>>>> >>>>>>>>>>> I am not sure but I think there is no performance penalty right? >>>>>>>>>>> That >>>>>>>>>>> is >>>>>>>>>>> why I suggest lumping them together. >>>>>>>>>>> >>>>>>>>>>> Taher Alkhateeb >>>>>>>>>>> >>>>>>>>>>> On Aug 6, 2016 11:40 AM, "Jacques Le Roux" < >>>>>>>>>>> >>>>>>>>>>> [email protected] <javascript:;>> >>>>>>>>>>> >>>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>> Le 06/08/2016 à 11:43, Taher Alkhateeb a écrit : >>>>>>>>> >>>>>>>>>> Hi Jacques, >>>>>>>>>>> >>>>>>>>>>> I think that filling the white list ,etc ... might be something >>>>>>>>>>>> to >>>>>>>>>>>> >>>>>>>>>>>> keep >>>>>>>>>>>>> >>>>>>>>>>>>> in >>>>>>>>>>>> >>>>>>>>>>> the page on securing OFBiz (documentation). >>>>>>>>>>> >>>>>>>>>>> I prefer to have a direct link to notsoserial documentation to be >>>>>>>>>>>> >>>>>>>>>>>>> sure >>>>>>>>>>>>> >>>>>>>>>>>>> it's up to date. That's what I did on the related wiki page >>>>>>>>>>>>> >>>>>>>>>>>>> I understand your point about >>>>>>>>>>>> >>>>>>>>>>>> making it more "explicit" which makes sense, it has, however, >>>>>>>>>>>> the >>>>>>>>>>>> >>>>>>>>>>>> downside >>>>>>>>>>>> >>>>>>>>>>>>> of making the users aware that there are different tasks to >>>>>>>>>>>>> run, >>>>>>>>>>>>> and >>>>>>>>>>>>> also >>>>>>>>>>>>> the rc scripts need to be modified to production and might be >>>>>>>>>>>>> >>>>>>>>>>>>> confusing >>>>>>>>>>>>> >>>>>>>>>>>>> (ofbiz, ofbizBackground, ofbizBackgroundSecure, ofbizSecure) >>>>>>>>>>>> >>>>>>>>>>> might be >>>>>>>>>>> too >>>>>>>>>>> >>>>>>>>>>> many options to choose from in a production environment. >>>>>>>>>>>> >>>>>>>>>>>>> No strong opinion, but I am suggesting to make it a little >>>>>>>>>>>>> easier >>>>>>>>>>>>> for >>>>>>>>>>>>> people with a less-is-more kind of approach. >>>>>>>>>>>>> >>>>>>>>>>>>> What would you suggest? It seems to me that removing these >>>>>>>>>>>>> options >>>>>>>>>>>>> >>>>>>>>>>>>> would >>>>>>>>>>>>> >>>>>>>>>>>>> degrade the information about rare but possible vulnerabilities >>>>>>>>>>>> >>>>>>>>>>> Jacques >>>>>>>>>>> >>>>>>>>>>> Taher Alkhateeb >>>>>>>>>>>> >>>>>>>>>>>> On Sat, Aug 6, 2016 at 11:44 AM, Jacques Le Roux < >>>>>>>>>>>> >>>>>>>>>>>> [email protected] <javascript:;>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> The idea is that by default the task does not do much. You have >>>>>>>>>>>>> to >>>>>>>>>>>>> follow >>>>>>>>>>>>> >>>>>>>>>>>>> the advices they give to make it really effective (filling a >>>>>>>>>>>>> white >>>>>>>>>>>>> list >>>>>>>>>>>>> >>>>>>>>>>>>> is >>>>>>>>>>>>> >>>>>>>>>>>> the better way) >>>>>>>>>>> >>>>>>>>>>> That's why I separated it from the rest to make it more obvious >>>>>>>>>>>> >>>>>>>>>>>>> for >>>>>>>>>>>>>> users. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Currently "gradlew tasks" gives you this information >>>>>>>>>>>>>> >>>>>>>>>>>>>> Pattern: ofbizSecure <Commands>: Execute OFBiz startup >>>>>>>>>>>>>> commands >>>>>>>>>>>>>> pre-loading the notsoserial Java agent >>>>>>>>>>>>>> Pattern: ofbizBackgroundSecure <Commands>: Execute OFBiz >>>>>>>>>>>>>> startup >>>>>>>>>>>>>> commands >>>>>>>>>>>>>> in background (secure mode) and output to console.log >>>>>>>>>>>>>> >>>>>>>>>>>>>> Jacques >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Le 06/08/2016 à 03:33, Scott Gray a écrit : >>>>>>>>>>>>>> >>>>>>>>>>>>>> Why isn't whatever functionality 'ofbizSecure' provides, just >>>>>>>>>>>>>> >>>>>>>>>>>>>> included >>>>>>>>>>>>>> >>>>>>>>>>>>>> as >>>>>>>>>>>>> >>>>>>>>>>>> part of the regular 'ofbiz' task? >>>>>>>>>>> >>>>>>>>>>> On 5 August 2016 at 21:35, Jacques Le Roux < >>>>>>>>>>>> >>>>>>>>>>>>> [email protected] <javascript:;>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Le 05/08/2016 à 11:21, Taher Alkhateeb a écrit : >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> +1 makes sense >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Should we also remove the tasks ofbizSecure and >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> ofbizBackgroundSecure >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>> >>>>>>>>>>>>>> replace them with some scripts in /tools if people are not >>>>>>>>>>>>> using >>>>>>>>>>>>> >>>>>>>>>>>> them? >>>>>>>>>>>> >>>>>>>>>>>>> (I >>>>>>>>>>>>>>>>> assume we only use them with demos?) >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Aug 5, 2016 10:07 AM, "Jacques Le >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Roux"<jacques.le.roux@les7arts >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> .com >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>> Nope, those are intended to be used in production if ever you >>>>>>>>>>>> >>>>>>>>>>>>> need >>>>>>>>>>>>>>>>> it. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> See the warning therehttps://cwiki.apache.org/confl >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> uence/display/OFBIZ/Keeping+OFBiz+secure for details >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Jacques >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >
