Add IP Addresses to Synapse log files

[SPiRiT369]

Logging clients IP Addresses is essential for security.

I use fail2ban to automatically block IP Addresses when I detect clients trying to bruteforce my server (e.g. guess mail server passwords).

With Synapse I can't do that because the log file records do not include source IP Address. For example:
2022-02-16 19:33:23,003 - synapse.handlers.auth - 1387 - WARNING - POST-4233 - Failed password login for user <user>

I think that it will be very useful (and also essential) to include IP addresses in the log file, at least in security-related events such as registrations and authentication failures.

Hiding IP Addresses from Synapse logs does not improve privacy, as the address is visible in the webserver's logs and also stored in the database. So I don't see any benefit of hiding it.

I am aware that ratelimiting is an option, but it's a different solution.

[DMRobertson]

Synapse includes IP addresses and other data in the Processing lines as it finishes handling a given request. Look for lines matching the regex POST-4233.*Processed request.

[SPiRiT369]

This is not good enough because:

• I only log WARNING+ events. "Processed request" lines are only in INFO logs, which I don't want to enable.
• The IP address should be in the same log line as "Failed password login for user " because that's how I can catch such events automatically.

[dklimpel]

You can additionally configure your fail2ban to block IPs with error 403 at /_matrix/client/r0/login on your reverse proxy.

[SPiRiT369]

Yes @dklimpel - if 403 is raised on auth failure only then this should work. But it is still something that should go thru synapse log. I want to have separate rule for synapse instead of modifying the webserver rule.

[dklimpel]

My thought would be security and request blocking as early as possible.

It would probably make more sense to document all relevant log entries.
E.g.

• Invalid username
• Wrong password
• Rate limiting

This is related to:

• fail2ban regexps for synapse #1154

[erikjohnston]

We're uncomfortable making it so that we change this particular line or make it log IP addresses for all lines by default. We would accept a patch for passing in the IP address as a optional field so that admins can configure their logging to include the IP address.

[richvdh]

another option might be to make use of a structured log format (see #8683)

[erikjohnston]

We're uncomfortable making it so that we change this particular line or make it log IP addresses for all lines by default. We would accept a patch for passing in the IP address as a optional field so that admins can configure their logging to include the IP address.

In fact, do we already support this? https://github.com/matrix-org/synapse/blob/develop/synapse/logging/context.py#L605

In which case adding ip_address the logger formatter might Just Work?

[SPiRiT369]

In fact, do we already support this? https://github.com/matrix-org/synapse/blob/develop/synapse/logging/context.py#L605
In which case adding ip_address the logger formatter might Just Work?

How can I do it? I've tried with %(ip_address)s, and it didn't work (caused python error).

[erikjohnston]

Oh, hmm, probably because not every log line has an IP address associated with it. If you're using py3.10 then I think you can use the new defaults param on the in built log formatter:

formatters:
    precise:
        (): logging.Formatter
        fmt: '%(asctime)s - %(name)s - %(lineno)d - %(levelname)s - %(request)s - %(ip_address)s - %(message)s'
        defaults:
            ip_address: "n/a"

Otherwise you could create a custom formatter class that handles ip_address not always being defined.

On the Synapse side: not sure if we should give defaults to the extra params?

[SPiRiT369]

Oh wow, @erikjohnston, you solved the problem! 👍
Thank you very much for this solution and for your help!
Is that ok if I close this issue or should you do that as the one who provided solution?

[clokep]

I think we can go ahead and close it since we came to a solution. 👍

[SPiRiT369]

I've just noticed that although it DOES solve the problem, other events cause python errors. So I need to look further on this :(

[SPiRiT369]

@clokep is it possible to re-open this case, as the previous solution is faulty? :/ This "feature" is a must.

[DMRobertson]

I've just noticed that although it DOES solve the problem, other events cause python errors. So I need to look further on this :(

@clokep is it possible to re-open this case, as the previous solution is faulty? :/ This "feature" is a must.

@SPiRiT369 I think it'd be best if you can open a new issue, containing

• the logging config which causes the problem,
• the python errors you're seeing, and
• instructions on how to reproduce them.