https://redmine.openinfosecfoundation.org/https://redmine.openinfosecfoundation.org/favicon.ico?17011170022017-12-20T09:11:48ZOpen Information Security FoundationSuricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=92602017-12-20T09:11:48ZVictor Julienvictor@inliniac.net
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-2 priority-4 priority-default" href="/issues/2337">Bug #2337</a>: give warning if permissions won't allow log reopen after dropping privs</i> added</li></ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=93232017-12-30T15:12:57ZRichard Sailer
<ul></ul><p>Hi, I have two detail questions regarding the implementation of this:</p>
<a name="Question-1"></a>
<h2 >Question 1<a href="#Question-1" class="wiki-anchor">¶</a></h2>
Should the patch?
<ol>
<li>only check for the user we currently are or </li>
<li>also check for the user we will become when we drop privs.</li>
</ol>
<p>Currently I assume 2. because it would make more sense IMHO. Also, in the case of 2. I think it will suffice to only check for the<br />user we will become, because we can change users only if we are root, and checking privs for root is unnecesarry per definition. <br />Any objections to these?</p>
<a name="Question-2"></a>
<h2 >Question 2<a href="#Question-2" class="wiki-anchor">¶</a></h2>
<ul>
<li>If I implement 2. (as described above) I need to move the call to ConfigCheckLogDirectory() to a point later in the initialization.</li>
<li>Currently it gets called in ParseCommandLine(). At this point we can not know for sure if we will drop privs (--user could be a parameter behind -l, or priviledge dropping could be specified in the config file). </li>
<li>Therefore I would recommend moving the call to ConfigCheckLogDirectory() before extending. I would recommend after config reading in PostConfLoadedSetup(), Line 2736 (Commit 282dad) directly after we evaluated whether we will drop privs in InitSignalHandler().</li>
<li>Any objections? Thoughts?</li>
</ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=93242017-12-31T08:04:22ZJason Ishjason.ish@oisf.net
<ul></ul><p>Another observation I had. If the log directory does not exist, we exit. If its not writable, we continue and try to open up the interface. As both have the same affect of no logs being produced, the not writable case should probably also exit.</p>
<p>I think I would leave some form of ConfigCheckLogDirectory where it is. In the case where its set on the command line, the error message includes that context information. If set via the config, the error message provides that context. But perhaps ConfigCheckLogDirectory is better named something like ConfigCheckLogDirectoryExists(), and than a new function ...IsWritable() or something.</p>
<p>Its simple to test writability after switching users. But doing so, may result in nothing be logged to the console in daemon mode when it is not writable. As we daemonize, then drop privileges. So doing something like you suggest above at line 2736 is one option.</p> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=93252017-12-31T11:31:35ZRichard Sailer
<ul></ul><p>Jason Ish wrote:</p>
<blockquote>
<p>Another observation I had. If the log directory does not exist, we exit. If its not writable, we continue and try to open up the interface. As both have the same affect of no logs being produced, the not writable case should probably also exit.</p>
</blockquote>
<p>Actually I intendend to implement it this way anyway but forgot to mention it anywhere :p</p>
<blockquote>
<p>I think I would leave some form of ConfigCheckLogDirectory where it is. In the case where its set on the command line, the error message includes that context information. If set via the config, the error message provides that context. But perhaps ConfigCheckLogDirectory is better named something like ConfigCheckLogDirectoryExists(), and than a new function ...IsWritable() or something.</p>
</blockquote>
<p>Okay</p>
<blockquote>
<p>Its simple to test writability after switching users. But doing so, may result in nothing be logged to the console in daemon mode when it is not writable. As we daemonize, then drop privileges. So doing something like you suggest above at line 2736 is one option.</p>
</blockquote>
<p>Good. It's also not that complicated to test it before priv droping, it's a call to stat, and a few numeric comparisions of ids in the suri instance struct and the stat return struct.</p> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=94352018-02-05T14:20:27ZRichard Sailer
<ul><li><strong>% Done</strong> changed from <i>0</i> to <i>70</i></li></ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=110582019-02-18T22:15:02ZAndreas Herzoisf@herzandreas.de
<ul><li><strong>Assignee</strong> changed from <i>Richard Sailer</i> to <i>OISF Dev</i></li></ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=140352019-09-27T17:58:17ZVictor Julienvictor@inliniac.net
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Assigned</i></li><li><strong>Assignee</strong> changed from <i>OISF Dev</i> to <i>Shivani Bhardwaj</i></li><li><strong>Target version</strong> changed from <i>TBD</i> to <i>5.0.0</i></li></ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=140382019-09-27T18:01:48ZVictor Julienvictor@inliniac.net
<ul><li><strong>Related to</strong> <i><a class="issue tracker-4 status-1 priority-4 priority-default" href="/issues/1595">Optimization #1595</a>: Suricata starts in known conditions of no data</i> added</li></ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=140852019-10-01T10:01:40ZShivani Bhardwaj
<ul><li><strong>Priority</strong> changed from <i>Normal</i> to <i>Urgent</i></li></ul> Suricata - Bug #2386: check if default log dir is writable at start uphttps://redmine.openinfosecfoundation.org/issues/2386?journal_id=141702019-10-09T13:29:46ZVictor Julienvictor@inliniac.net
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li><li><strong>Priority</strong> changed from <i>Urgent</i> to <i>Normal</i></li></ul><p><a class="external" href="https://github.com/OISF/suricata/pull/4280">https://github.com/OISF/suricata/pull/4280</a></p>