LDAP password is logged in clear text about davis HOT 15 CLOSED

I think it's very legit. Will try to work on that soon!

from davis.

Interesting, I didn't encounter this edge case when testing on my side. I've made the code a bit smarter in db1ef33 🙏🏼

from davis.

Hi @mpflanzer

Thanks, that's a good catch. Will see on how to filter them out

from davis.

Hi @mpflanzer

I've added some circumventions in #116 — would you be so kind as to test your use cases and tell me if it's better?

Thanks a lot!

from davis.

Thanks, that's better for sure.

It still has the problem that should anything throw it would print the password. Do you know if there is a way to turn off printing the stack trace? I tried setting APP_ENV=prod and APP_DEBUG=0 but I still see the full trace. 🤔

from davis.

should anything throw it would print the password

That's true, but it's also true for ~any other software that prints production logs to be honest :)

The real need in your case is to deactivate logs globally in production (which is a very sane thing to do) to mitigate that risk altogether. Traces are just logs with an "error" level, but TBH, passwords could as well creep in other levels (warnings, etc), so removing debug traces would not be a good solution.

Would you be interested in having a way to disable logging altogether with an env var?

from davis.

Would you be interested in having a way to disable logging altogether with an env var?

That would make it a bit easier to deactivate them while still using the released docker image. If that's not a big change I think it would be nice. Otherwise rebuilding the docker image with a changed logger config is also not a disaster.

from davis.

Done here. If you confirm this is fine for you in your setup, I'll merge and provide a new release in the coming days

from davis.

👍 for the FilterProcessor approach. That looks really nice. But for some reason it doesn't have any effect for me. For example if I throw an (invalid) exception in ldapOpen I still get this error log:

[2023-11-02T10:19:51.289613+01:00] app.ERROR: [500]: Error - Can only throw objects [{"file":"/var/www/davis/src/Services/LDAPAuth.php","line":189,"function":"ldapOpen","class":"App\\Services\\LDAPAuth","type":"->","args":["moritz","asdasda "]}

The dav function could be another candidate to redact the arguments:

{"file":"/var/www/davis/vendor/symfony/http-kernel/HttpKernel.php","line":163,"function":"dav","class":"App\\Controller\\DAVController","type":"->","args":[{"Symfony\\Component\\HttpFoundation\\Request":"PUT /dav/calendars/moritz/default HTTP/1.1\r\nAccept:          text/xml\r\nAccept-Charset:  utf-8,*;q=0.1\r\nAccept-Encoding: gzip, deflate, br\r\nAccept-Language: en-GB,en;q=0.5\r\nAuthorization:   Basic bW9yaXR6OmFzZGFzZGEg\r\nCache-Control:   no-cache\r\nConnection:      keep-alive\r\nContent-Length:  0\r\nContent-Type:    \r\nHost:            calendar.chickadee-engineering.com\r\nIf-Match:        nothing\r\nOrigin:          https://calendar.chickadee-engineering.com\r\nPhp-Auth-Pw:     asdasda \r\nPhp-Auth-User:   moritz\r\nPragma:          no-cache\r\nSec-Fetch-Dest:  empty\r\nSec-Fetch-Mode:  no-cors\r\nSec-Fetch-Site:  same-origin\r\nUser-Agent:      Mozilla/5.0 (X11; Linux x86_64; rv:115.0) Gecko/20100101 Thunderbird/115.3.1\r\nX-Php-Ob-Level:  0\r\n\r\n"},"calendars/moritz/default"]}

And with the latest commit I don't get any log file anymore. In the docker container the environment contains LOG_FILE_PATH=%kernel.logs_dir%/%kernel.environment%.log but there is never a file in ./var/log/prod.log. If I take the commit before the log file appears as expected. 🤔

Maybe it's just my setup that is somehow broken but could you double check that it's behaving as expected for you?

from davis.

LOG_FILE_PATH=%kernel.logs_dir%/%kernel.environment%.log

You need to quote the path IMO, otherwise the variables are replaced sooner than expected

from davis.

I still get this error log:

Are you in production mode? The change only applies there

The dav function could be another candidate to redact the arguments

Will have a look!

from davis.

Yes, should be in production:

  davis:
    image: davis-dev
    #image: ghcr.io/tchapi/davis:v4.0.0
    environment:
      - APP_ENV=prod
      #- APP_DEBUG=0
      - LOG_FILE_PATH="%kernel.logs_dir%/%kernel.environment%.log"

Quoting the path (or not specifying anything) also doesn't help. No log file is generated with the latest commit

from davis.

Oh, gotcha, that's on me! Can you retry? I've pushed a fix (and added dav in the list of sensitive methods)

from davis.

Nice, the log file is now working. I still see the problem that the args are not redacted.

If instead of your loop I have the below it is working though. It's less generic than yours so I'm not super happy with it. I'll try to spend some more time with it to figure out the difference.

        foreach ($record["context"] as $idx => $item) {
           foreach ($item as $key => $value) {
            if (self::PASSWORD_KEY === strtolower($key)) {
                $record["context"][$idx][$key] = '****';
            } elseif ('function' === strtolower($key)) {
                // Remove potentially sensitive data from function arguments
                if (in_array($value, self::SENSITIVE_ARGS_FUNCTIONS)) {
                    $record["context"][$idx]['args'] = ['****'];
                }
            } 
          }
        }

edit: I figured out the problem. In the loop the function key is visited first and sets the args key. But then after the args key is visited and overwrites the redacted values with original value by passing down $item in the recursive call.

This for example would work for your loop: elseif (is_array($item) && $key != "args")

from davis.

LGTM, thanks for the effort.

from davis.