From 68870119f4514d6e218fbc06136f9f8cdc093b09 Mon Sep 17 00:00:00 2001 From: Deimos Date: Sun, 6 Sep 2020 18:32:10 -0600 Subject: [PATCH] Remove remnants of Redis breached-passwords check We've been using pts_lbsearch on the text file for a few weeks now, and it's working fine. Checks generally seem to take about 10 ms, and that's totally fine for the relatively uncommon events of registrations and password changes. This removes everything related to the previous Redis-based method, which means we no longer need the second Redis server or the ReBloom module. --- .../prometheus_redis_exporter.service | 2 +- salt/salt/redis/breached-passwords.sls | 29 --- salt/salt/redis/modules/rebloom.sls | 18 -- salt/salt/redis/redis_breached_passwords.conf | 24 --- .../redis/redis_breached_passwords.service | 14 -- salt/salt/top.sls | 2 - tildes/scripts/breached_passwords.py | 170 ------------------ tildes/tildes/lib/password.py | 7 - .../includes/password_restrictions.jinja2 | 1 - 9 files changed, 1 insertion(+), 266 deletions(-) delete mode 100644 salt/salt/redis/breached-passwords.sls delete mode 100644 salt/salt/redis/modules/rebloom.sls delete mode 100644 salt/salt/redis/redis_breached_passwords.conf delete mode 100644 salt/salt/redis/redis_breached_passwords.service delete mode 100644 tildes/scripts/breached_passwords.py diff --git a/salt/salt/prometheus/exporters/prometheus_redis_exporter.service b/salt/salt/prometheus/exporters/prometheus_redis_exporter.service index 1b5c03a..511ad1e 100644 --- a/salt/salt/prometheus/exporters/prometheus_redis_exporter.service +++ b/salt/salt/prometheus/exporters/prometheus_redis_exporter.service @@ -8,7 +8,7 @@ RemainAfterExit=no WorkingDirectory=/opt/prometheus_redis_exporter User=prometheus Group=prometheus -Environment="REDIS_ADDR=unix:///run/redis/socket,unix:///run/redis_breached_passwords/socket" +Environment="REDIS_ADDR=unix:///run/redis/socket" ExecStart=/opt/prometheus_redis_exporter/redis_exporter [Install] diff --git a/salt/salt/redis/breached-passwords.sls b/salt/salt/redis/breached-passwords.sls deleted file mode 100644 index 39f4807..0000000 --- a/salt/salt/redis/breached-passwords.sls +++ /dev/null @@ -1,29 +0,0 @@ -/run/redis_breached_passwords: - file.directory: - - user: redis - - group: redis - - mode: 755 - - require: - - user: redis-user - -/etc/redis_breached_passwords.conf: - file.managed: - - source: salt://redis/redis_breached_passwords.conf - - user: redis - - group: redis - - mode: 600 - -/etc/systemd/system/redis_breached_passwords.service: - file.managed: - - source: salt://redis/redis_breached_passwords.service - - user: root - - group: root - - mode: 644 - - require_in: - - service: redis_breached_passwords.service - -redis_breached_passwords.service: - service.running: - - enable: True - - watch: - - file: /etc/redis_breached_passwords.conf diff --git a/salt/salt/redis/modules/rebloom.sls b/salt/salt/redis/modules/rebloom.sls deleted file mode 100644 index ad84ca0..0000000 --- a/salt/salt/redis/modules/rebloom.sls +++ /dev/null @@ -1,18 +0,0 @@ -# Take care if updating this module - Redis Labs changed the license on July 16, 2018 -# to Apache 2 with their "Commons Clause": https://commonsclause.com/ -# The legality and specific implications of that clause are currently unclear, so we -# probably shouldn't update to a version under that license without more research. -rebloom-clone: - git.latest: - - name: https://github.com/RedisLabsModules/rebloom - - rev: 4947c9a75838688df56fc818729b93bf36588400 - - target: /opt/rebloom - -rebloom-make: - cmd.run: - - name: make - - cwd: /opt/rebloom - - onchanges: - - git: rebloom-clone - - require_in: - - service: redis_breached_passwords.service diff --git a/salt/salt/redis/redis_breached_passwords.conf b/salt/salt/redis/redis_breached_passwords.conf deleted file mode 100644 index bf9f781..0000000 --- a/salt/salt/redis/redis_breached_passwords.conf +++ /dev/null @@ -1,24 +0,0 @@ -loadmodule /opt/rebloom/rebloom.so -bind 127.0.0.1 - -# only listen on unix socket -port 0 - -unixsocket /run/redis_breached_passwords/socket -unixsocketperm 777 - -timeout 0 - -supervised systemd -pidfile /run/redis_breached_passwords/pid - -loglevel notice -logfile "" - -databases 1 -rdbchecksum yes - -dir /var/lib/redis -dbfilename breached_passwords_dump.rdb - -appendonly no diff --git a/salt/salt/redis/redis_breached_passwords.service b/salt/salt/redis/redis_breached_passwords.service deleted file mode 100644 index 9cd57e9..0000000 --- a/salt/salt/redis/redis_breached_passwords.service +++ /dev/null @@ -1,14 +0,0 @@ -[Unit] -Description=redis breached passwords daemon -After=network.target - -[Service] -PIDFile=/run/redis_breached_passwords/pid -User=redis -Group=redis -RuntimeDirectory=redis_breached_passwords -ExecStart=/usr/local/bin/redis-server /etc/redis_breached_passwords.conf -Restart=always - -[Install] -WantedBy=multi-user.target diff --git a/salt/salt/top.sls b/salt/salt/top.sls index 75cc6ac..9ef97a7 100644 --- a/salt/salt/top.sls +++ b/salt/salt/top.sls @@ -9,8 +9,6 @@ base: - postgresql.pgbouncer - python - redis - - redis.breached-passwords - - redis.modules.rebloom - redis.modules.redis-cell - postgresql-redis-bridge - scripts diff --git a/tildes/scripts/breached_passwords.py b/tildes/scripts/breached_passwords.py deleted file mode 100644 index 7cfdbab..0000000 --- a/tildes/scripts/breached_passwords.py +++ /dev/null @@ -1,170 +0,0 @@ -# Copyright (c) 2018 Tildes contributors -# SPDX-License-Identifier: AGPL-3.0-or-later - -"""Command-line tools for managing a breached-passwords bloom filter. - -This tool will help with creating and updating a bloom filter in Redis (using ReBloom: -https://github.com/RedisLabsModules/rebloom) to hold hashes for passwords that have been -revealed through data breaches (to prevent users from using these passwords here). The -dumps are likely primarily sourced from Troy Hunt's "Pwned Passwords" files: -https://haveibeenpwned.com/Passwords - -Specifically, the commands in this tool allow building the bloom filter somewhere else, -then the RDB file can be transferred to the production server. Note that it is expected -that a separate redis server instance is running solely for holding this bloom filter. -Replacing the RDB file will result in all other keys being lost. - -Expected usage of this tool should look something like: - -On the machine building the bloom filter: - python breached_passwords.py init --estimate 350000000 - python breached_passwords.py addhashes pwned-passwords-1.0.txt - python breached_passwords.py addhashes pwned-passwords-update-1.txt - -Then the RDB file can simply be transferred to the production server, overwriting any -previous RDB file. - -""" - -import subprocess -from typing import Any - -import click -from redis import Redis, ResponseError - -from tildes.lib.password import ( - BREACHED_PASSWORDS_BF_KEY, - BREACHED_PASSWORDS_REDIS_SOCKET, -) - - -REDIS = Redis(unix_socket_path=BREACHED_PASSWORDS_REDIS_SOCKET) - - -def generate_redis_protocol(*elements: Any) -> str: - """Generate a command in the Redis protocol from the specified elements. - - Based on the example Ruby code from - https://redis.io/topics/mass-insert#generating-redis-protocol - """ - command = f"*{len(elements)}\r\n" - - for element in elements: - element = str(element) - command += f"${len(element)}\r\n{element}\r\n" - - return command - - -@click.group() -def cli() -> None: - """Create a functionality-less command group to attach subcommands to.""" - pass - - -def validate_init_error_rate(ctx: Any, param: Any, value: Any) -> float: - """Validate the --error-rate arg for the init command.""" - # pylint: disable=unused-argument - if not 0 < value < 1: - raise click.BadParameter("error rate must be a float between 0 and 1") - - return value - - -@cli.command(help="Initialize a new empty bloom filter") -@click.option( - "--estimate", - required=True, - type=int, - help="Expected number of passwords that will be added", -) -@click.option( - "--error-rate", - default=0.01, - show_default=True, - help="Bloom filter desired false positive ratio", - callback=validate_init_error_rate, -) -@click.confirmation_option( - prompt="Are you sure you want to clear any existing bloom filter?" -) -def init(estimate: int, error_rate: float) -> None: - """Initialize a new bloom filter (destroying any existing one). - - It generally shouldn't be necessary to re-init a new bloom filter very often with - this command, only if the previous one was created with too low of an estimate for - number of passwords, or to change to a different false positive rate. For choosing - an estimate value, according to the ReBloom documentation: "Performance will begin - to degrade after adding more items than this number. The actual degradation will - depend on how far the limit has been exceeded. Performance will degrade linearly as - the number of entries grow exponentially." - """ - REDIS.delete(BREACHED_PASSWORDS_BF_KEY) - - # BF.RESERVE {key} {error_rate} {size} - REDIS.execute_command("BF.RESERVE", BREACHED_PASSWORDS_BF_KEY, error_rate, estimate) - - click.echo( - "Initialized bloom filter with expected size of {:,} and false " - "positive rate of {}%".format(estimate, error_rate * 100) - ) - - -@cli.command(help="Add hashes from a file to the bloom filter") -@click.argument("filename", type=click.Path(exists=True, dir_okay=False)) -def addhashes(filename: str) -> None: - """Add all hashes from a file to the bloom filter. - - This uses the method of generating commands in Redis protocol and feeding them into - an instance of `redis-cli --pipe`, as recommended in - https://redis.io/topics/mass-insert - """ - # make sure the key exists and is a bloom filter - try: - REDIS.execute_command("BF.DEBUG", BREACHED_PASSWORDS_BF_KEY) - except ResponseError: - click.echo("Bloom filter is not set up properly - run init first.") - raise click.Abort - - # call wc to count the number of lines in the file for the progress bar - click.echo("Determining hash count...") - result = subprocess.run(["wc", "-l", filename], stdout=subprocess.PIPE, check=True) - line_count = int(result.stdout.split(b" ")[0]) - - progress_bar: Any = click.progressbar(length=line_count) - update_interval = 100_000 - - click.echo("Adding {:,} hashes to bloom filter...".format(line_count)) - - redis_pipe = subprocess.Popen( - ["redis-cli", "-s", BREACHED_PASSWORDS_REDIS_SOCKET, "--pipe"], - stdin=subprocess.PIPE, - stdout=subprocess.DEVNULL, - encoding="utf-8", - ) - - for count, line in enumerate(open(filename), start=1): - hashval = line.strip().lower() - - # the Pwned Passwords hash lists now have a frequency count for each hash, which - # is separated from the hash with a colon, so we need to handle that if it's - # present - hashval = hashval.split(":")[0] - - command = generate_redis_protocol("BF.ADD", BREACHED_PASSWORDS_BF_KEY, hashval) - redis_pipe.stdin.write(command) # type: ignore - - if count % update_interval == 0: - progress_bar.update(update_interval) - - # call SAVE to update the RDB file - REDIS.save() - - # manually finish the progress bar so it shows 100% and renders properly - progress_bar.finish() - progress_bar.render_progress() - progress_bar.render_finish() - - -if __name__ == "__main__": - cli() diff --git a/tildes/tildes/lib/password.py b/tildes/tildes/lib/password.py index 7d9206f..153c002 100644 --- a/tildes/tildes/lib/password.py +++ b/tildes/tildes/lib/password.py @@ -10,13 +10,6 @@ from tildes import settings from tildes.metrics import summary_timer -# unix socket path for redis server with the breached passwords bloom filter -BREACHED_PASSWORDS_REDIS_SOCKET = "/run/redis_breached_passwords/socket" - -# Key where the bloom filter of password hashes from data breaches is stored -BREACHED_PASSWORDS_BF_KEY = "breached_passwords_bloom" - - @summary_timer("breached_password_check") def is_breached_password(password: str) -> bool: """Return whether the password is in the breached-passwords list. diff --git a/tildes/tildes/templates/includes/password_restrictions.jinja2 b/tildes/tildes/templates/includes/password_restrictions.jinja2 index dff53a8..21e3a0b 100644 --- a/tildes/tildes/templates/includes/password_restrictions.jinja2 +++ b/tildes/tildes/templates/includes/password_restrictions.jinja2 @@ -8,7 +8,6 @@
  • Does not contain the username, and is not contained in the username.

  • Has not been previously exposed in a data breach (checked locally against a list downloaded from Troy Hunt's "Have I been pwned?").

    -

    Note: this check uses a Bloom filter, so false positives are possible (but very rare). Even if it is a false positive, you must choose a different password.