CouchDB
  1. CouchDB
  2. COUCHDB-464

Allow POST to _log for external processes

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Component/s: None
    • Labels:
      None
    • Skill Level:
      Regular Contributors Level (Easy to Medium)

      Description

      Add POST support to _log so that external processes can also log to couch.log. This would allow couchdb-lucene (to pick a random example) to log consistently.

      1. 0001-Add-POST-support-to-_log.patch
        1 kB
        Robert Newson
      2. 0001-Add-POST-support-to-_log.patch
        1 kB
        Robert Newson
      3. 0001-Add-POST-support-to-_log.patch
        1 kB
        Robert Newson

        Activity

        Hide
        Paul Joseph Davis added a comment -

        As a matter of fact, all control characters should probably be stripped from user supplied log messages. There's a CVE out there somewhere that discusses how to inject control sequences so that if someone tails the log as root it'll execute arbitrary commands. Technically more of a bug in the terminal, but still something to be aware of.

        Show
        Paul Joseph Davis added a comment - As a matter of fact, all control characters should probably be stripped from user supplied log messages. There's a CVE out there somewhere that discusses how to inject control sequences so that if someone tails the log as root it'll execute arbitrary commands. Technically more of a bug in the terminal, but still something to be aware of.
        Hide
        Jan Lehnardt added a comment -

        Love the attention to detail Noah. All +1 from me.

        Show
        Jan Lehnardt added a comment - Love the attention to detail Noah. All +1 from me.
        Hide
        Sam Bisbee added a comment -

        Yeah, new lines should just be stripped out. Maybe even all escape characters: I suppose text editors with wrapping turned on would make enough tabs look like a newline, etc.

        Show
        Sam Bisbee added a comment - Yeah, new lines should just be stripped out. Maybe even all escape characters: I suppose text editors with wrapping turned on would make enough tabs look like a newline, etc.
        Hide
        Noah Slater added a comment -

        Cool.

        Just a note to say that even with a privilege/role system in place, we need to think about newlines. At the moment, even with these in place, I could create a log record that uses a newline to spoof a new log message without the "source" field to indicate where this log message came from. The only sane way forward that I can think of is to collapse whitespace.

        Show
        Noah Slater added a comment - Cool. Just a note to say that even with a privilege/role system in place, we need to think about newlines. At the moment, even with these in place, I could create a log record that uses a newline to spoof a new log message without the "source" field to indicate where this log message came from. The only sane way forward that I can think of is to collapse whitespace.
        Hide
        Jason Smith added a comment -

        The patch could check for either the _admin or _externals role right now. (IIRC, roles prefixed with underscore in user documents are a validation error.)

        A future step is Jan's idea to apply the _externals role to some requests that arrive. For example:

        1. When spawning an external, CouchDB generates random UUIDs for a username and password, and passes those along as environment. Those are associated with that child process.
        2. An authentication handler checks whether query credentials match those assigned to any externals.
        3. If they match, the "_external" role is assigned. (And incidentally, the "source" field is already known.)

        (Iris Couch uses an authentication handler similar to this already. I will donate the code if it is wanted.)

        P.S. "Logging" is a privilege, whereas "an external program" is more properly a role, in a role-based access control system. So I like "_external" as a role name.

        Show
        Jason Smith added a comment - The patch could check for either the _admin or _externals role right now. (IIRC, roles prefixed with underscore in user documents are a validation error.) A future step is Jan's idea to apply the _externals role to some requests that arrive. For example: 1. When spawning an external, CouchDB generates random UUIDs for a username and password, and passes those along as environment. Those are associated with that child process. 2. An authentication handler checks whether query credentials match those assigned to any externals. 3. If they match, the "_external" role is assigned. (And incidentally, the "source" field is already known.) (Iris Couch uses an authentication handler similar to this already. I will donate the code if it is wanted.) P.S. "Logging" is a privilege, whereas "an external program" is more properly a role, in a role-based access control system. So I like "_external" as a role name.
        Hide
        Jan Lehnardt added a comment -

        Good call, I'd suggest two changes:

        1. add a mandatory "source" field that is the prefix in the log.
        2. require admin privileges to make the request.
        2.1. I'd be happy to introduce in the future a _log or _externals role, so external scripts don't need to have full admin credentials.

        Show
        Jan Lehnardt added a comment - Good call, I'd suggest two changes: 1. add a mandatory "source" field that is the prefix in the log. 2. require admin privileges to make the request. 2.1. I'd be happy to introduce in the future a _log or _externals role, so external scripts don't need to have full admin credentials.
        Hide
        Noah Slater added a comment -

        Doesn't this allow malicious user agents to craft spoofed log entries for CouchDB? You could make it look like something very serious was happening, causing the CouchDB admin to take measures that harm the server or the data it contains. If we're going to do this at all (and I am not sure I see a valid use case here) then the message should be prefixed with a big fat notice that it's user generated.

        Show
        Noah Slater added a comment - Doesn't this allow malicious user agents to craft spoofed log entries for CouchDB? You could make it look like something very serious was happening, causing the CouchDB admin to take measures that harm the server or the data it contains. If we're going to do this at all (and I am not sure I see a valid use case here) then the message should be prefixed with a big fat notice that it's user generated.
        Hide
        Jan Lehnardt added a comment -

        Close with proper fix version

        Show
        Jan Lehnardt added a comment - Close with proper fix version
        Hide
        Robert Newson added a comment -

        Because people using couchdb-lucene with couchdb would prefer to read couch.log for the log messages, and I suspect that applies to other externals/addons, etc.

        It's clumsy if all addons are forced to log differently, it reduces the degree that addons can be integrated. Since couchdb has support for grafting external processes (which actually can log, I think), it's a shame that db_update_notification processes cannot.

        That said, for couchdb-lucene, I intend to get off db_update_notification entirely and move indexing into the the external searching process instead (typicall mapped to _fti). I think I didn't do that before because the external was only started on demand? I forget.

        Show
        Robert Newson added a comment - Because people using couchdb-lucene with couchdb would prefer to read couch.log for the log messages, and I suspect that applies to other externals/addons, etc. It's clumsy if all addons are forced to log differently, it reduces the degree that addons can be integrated. Since couchdb has support for grafting external processes (which actually can log, I think), it's a shame that db_update_notification processes cannot. That said, for couchdb-lucene, I intend to get off db_update_notification entirely and move indexing into the the external searching process instead (typicall mapped to _fti). I think I didn't do that before because the external was only started on demand? I forget.
        Hide
        Damien Katz added a comment -

        Why not just write the log messages to a db?

        Show
        Damien Katz added a comment - Why not just write the log messages to a db?
        Hide
        Robert Newson added a comment -

        To log to couch.log you post a JSON object like this;

        {"level":"info|debug|error", "message":"your message here"}
        Show
        Robert Newson added a comment - To log to couch.log you post a JSON object like this; {"level":"info|debug|error", "message":"your message here"}
        Hide
        Robert Newson added a comment -

        back to 200 for success. 202 implies an asynchronous handling of the request, and it isn't quite. While the log message is not fsynced to disk, which means it does happen after the client gets its 200, it's not that big a deal.

        Show
        Robert Newson added a comment - back to 200 for success. 202 implies an asynchronous handling of the request, and it isn't quite. While the log message is not fsynced to disk, which means it does happen after the client gets its 200, it's not that big a deal.
        Hide
        Robert Newson added a comment -


        Uses 202 (Accepted) instead of 200 and uses 400 with a readable error message if the log level isn't recognized.

        Show
        Robert Newson added a comment - Uses 202 (Accepted) instead of 200 and uses 400 with a readable error message if the log level isn't recognized.
        Hide
        Robert Newson added a comment -

        the case clause is remarkably ugly and duplicative. Advice welcome.

        Show
        Robert Newson added a comment - the case clause is remarkably ugly and duplicative. Advice welcome.

          People

          • Assignee:
            Unassigned
            Reporter:
            Robert Newson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development