search

backdrop-contrib / project

Projects associate a code-based project with releases and power the update server of BackdropCMS.org
2 stars 10 forks source link

Hash IP addresses when tracking project usage #39

Closed ghost closed 3 years ago

ghost commented 4 years ago

From @jenlampton in https://github.com/backdrop/backdrop-issues/issues/3688:

This is a follow-up to #3168 which I think will replace the need for that issue entirely.

From my comment:

I think the only real issue here is that we are sending an IP address so that we can tell all the sites apart. Why don't we just hash the IP address so that no recognizable data is ever stored?

I'm not sure if this is an issue that needs to go against core, project module, or b.org, but we should be able to move this issue when we decide where it should go :)

quicksketch commented 4 years ago

I think this is a good idea. Interesting, I went to check why this was in the first place and project_usage.install includes this description on the DB column:

      'hostname' => array(
        'description' => 'The IP address of the incoming request (to detect possible abuse).',

I'm guessing the thinking there is that any abusive IP addresses would then be blocked. That said, I think if we wanted to block abuse, we could do it in software in front of Backdrop (such as a CDN or abuse blocking service with a proxy). So I don't think project module is the right place to track this information and including it is only a liability and privacy concern in our code base.

quicksketch commented 4 years ago

I filed a PR for this: https://github.com/backdrop-contrib/project/pull/40

Rather than make a new hashing algorithm, I reused the hashing we use on passwords, since that's probably already the most secure hashing we have available.

jenlampton commented 4 years ago

If/when we do get autoban in core, we can also check the IP before it's hashed, and not save the hashed data if it matches an abusive IP or is in a range of blocked IPs. By the time were saving usage data for that site it should have passed all the checks anyway.

On Thu, Feb 20, 2020, 10:22 PM quicksketch notifications@github.com wrote:

I filed a PR for this: #40 https://github.com/backdrop-contrib/project/pull/40

Rather than make a new hashing algorithm, I reused the hashing we use on passwords, since that's probably already the most secure hashing we have available.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop-contrib/project/issues/39?email_source=notifications&email_token=AADBER4PH3WC54JCZKNNR2DRD5XINA5CNFSM4JKFFVH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMRU4WY#issuecomment-589516379, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER6R6VP2PKTKF5IT5DLRD5XINANCNFSM4JKFFVHQ .

quicksketch commented 3 years ago

Fixed in https://github.com/backdrop-contrib/project/pull/40.