security: replace retired MFA diagnostic credential discovery with scoped checks

Assistant: codex
Assistant-Model: gpt-6-astra
Assistant-Session: 01a1346b-72a4-7d40-b8bd-8b8ca346cfa2

tests/mfa_state_native_fixture.py

@@ -0,0 +1,123 @@
+"""Disposable real-provider acceptance, invoked in a separate Python process."""
+import contextlib
+import importlib.util
+import io
+import json
+import logging
+import os
+from pathlib import Path
+import sys
+import tempfile
+import threading
+import time
+
+
+def run():
+    for name in list(os.environ):
+        if name.startswith('PRIVACYIDEA_'): del os.environ[name]
+    logging.disable(logging.CRITICAL)
+    root_repo = Path(__file__).resolve().parents[1]
+    spec = importlib.util.spec_from_file_location('mfa_state', root_repo/'sso-mfa/k8s/privacyidea/mfa_state.py')
+    m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m)
+    from privacyidea.app import create_app
+    from privacyidea.models import db
+    from privacyidea.lib.policy import set_policy, enable_policy
+    from privacyidea.lib.resolver import save_resolver
+    from privacyidea.lib.realm import set_realm
+    from privacyidea.lib.auth import create_db_admin
+    from privacyidea.lib.resolvers.PasswdIdResolver import crypt_ctx
+    from privacyidea.lib.token import init_token
+    from privacyidea.lib.user import User
+    from werkzeug.serving import make_server
+    result = {}
+    with tempfile.TemporaryDirectory(prefix='mfa-native-') as directory:
+        root = Path(directory); (root/'enckey').write_bytes(os.urandom(96))
+        cfg = root/'fixture.cfg'
+        config = dict(SQLALCHEMY_DATABASE_URI='sqlite:///'+str(root/'db.sqlite'),
+                      SECRET_KEY='disposable-native-fixture', PI_PEPPER='disposable-native-pepper',
+                      PI_NO_RESPONSE_SIGN=True, PI_AUDIT_NO_SIGN=True, PI_LOGFILE=str(root/'log'),
+                      PI_ENCFILE=str(root/'enckey'), PI_TRUSTED_JWT=[],
+                      PI_AUDIT_SQL_URI='sqlite:///'+str(root/'audit.sqlite'))
+        cfg.write_text('\n'.join(k+'='+repr(v) for k,v in config.items()))
+        app = create_app(config_name='testing', config_file=str(cfg), silent=True)
+        with app.app_context():
+            assert str(db.engine.url) == config['SQLALCHEMY_DATABASE_URI']; db.create_all()
+            from privacyidea.lib.audit import getAudit
+            from privacyidea.lib.auditmodules.sqlaudit import LogEntry
+            LogEntry.__table__.create(getAudit(app.config).engine, checkfirst=True)
+            passwd = root/'users'
+            password = crypt_ctx.hash('fixture-user-password', scheme='sha512_crypt')
+            passwd.write_text('alice:'+password+':1001:1001:Fixture:/tmp:/bin/false\nbob:'+password+':1002:1002:Fixture:/tmp:/bin/false\n')
+            save_resolver({'resolver':'fixture-users','type':'passwdresolver','fileName':str(passwd)})
+            set_realm('coulomb',[{'name':'fixture-users'}])
+            create_db_admin('fixture-reader', password='fixture-reader-password')
+            set_policy(name='fixture-baseline',scope='admin',action='*',adminuser=['*','!fixture-reader'])
+            set_policy(name='fixture-reader',scope='admin',action='tokenlist',adminuser='fixture-reader',realm='coulomb')
+            probe = init_token({'serial':m.PROBE,'type':'hotp','genkey':1,'realm':'coulomb'})
+            probe.token.active = False; probe.token.save()
+            for i in range(18):
+                init_token({'serial':'FIXTURE'+str(i),'type':'hotp','genkey':1},user=User('alice','coulomb'))
+            pending = init_token({'serial':'FIXTUREPENDING','type':'hotp','genkey':1},user=User('alice','coulomb'))
+            pending.token.rollout_state = 'verify'; pending.token.save()
+            inactive = init_token({'serial':'FIXTUREINACTIVE','type':'hotp','genkey':1},user=User('alice','coulomb'))
+            inactive.token.active = False; inactive.token.save()
+        native = app.test_client()
+        def login(user, password, realm=None):
+            args={'username':user,'password':password}
+            if realm: args['realm']=realm
+            response=native.post('/auth',json=args)
+            assert response.status_code==200 and response.get_json()['result']['status']
+            return response.get_json()['result']['value']['token']
+        token=login('fixture-reader','fixture-reader-password')
+        tokenfile=root/'token';tokenfile.write_text(token);tokenfile.chmod(0o600)
+        assert m.credential(tokenfile)==token
+        server=make_server('127.0.0.1',0,app); thread=threading.Thread(target=server.serve_forever,daemon=True);thread.start()
+        try:
+            client=m.Client(f'http://127.0.0.1:{server.server_port}',token)
+            counts=m.diagnose(client,'alice')['factors']
+            assert counts=={'total':20,'active_confirmed':18,'active_pending':1,'inactive':1},counts
+            result['native_counts_beyond_default_page']=True
+            assert m.diagnose(client,'bob')['factors']['total']==0
+            result['native_empty_with_visibility']=True
+            def refused(fn):
+                try: fn()
+                except m.Refused: return True
+                return False
+            with app.app_context(): enable_policy('fixture-reader',False)
+            raw=native.get('/token/?user=alice&realm=coulomb',headers={'Authorization':token})
+            assert raw.status_code==200 and raw.get_json()['result']['value']['count']==0
+            assert refused(lambda:m.diagnose(client,'alice'))
+            result['native_withdrawal_empty_success_refused']=True
+            with app.app_context(): enable_policy('fixture-reader',True)
+            assert m.diagnose(client,'alice')['factors']['total']==20
+            result['native_permission_recovery']=True
+            usertoken=login('alice','fixture-user-password','coulomb')
+            assert refused(lambda:m.diagnose(m.Client(client.base,usertoken),'bob'))
+            result['native_self_service_cannot_prove_other_user_visibility']=True
+            assert refused(lambda:m.diagnose(m.Client(client.base,token[:-5]+'XXXXX'),'alice'))
+            result['native_tampered_signature_refused']=True
+            with app.app_context():
+                set_policy(name='fixture-short-session',scope='webui',action='jwt_validity=2')
+            short=login('fixture-reader','fixture-reader-password')
+            import base64
+            claims=json.loads(base64.urlsafe_b64decode(short.split('.')[1]+'==='))
+            remaining=claims['exp']-time.time();assert 0 < remaining < 3
+            time.sleep(remaining+0.2)
+            assert refused(lambda:m.diagnose(m.Client(client.base,short),'alice'))
+            result['native_expired_provider_jwt_refused']=True
+        finally:
+            server.shutdown();thread.join();server.server_close()
+    return result
+
+
+if __name__=='__main__':
+    try:
+        with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
+            result=run()
+        print(json.dumps(result,sort_keys=True))
+    except Exception as exc:
+        # Fixture-only, sanitized: no database/HTTP body/credential traceback.
+        import traceback
+        frame = traceback.extract_tb(exc.__traceback__)[-1]
+        print(json.dumps({'result':'failed','type':type(exc).__name__, 'file':Path(frame.filename).name, 'line':frame.lineno, 'counts':exc.args[0] if exc.args and isinstance(exc.args[0],dict) and set(exc.args[0])=={'total','active_confirmed','active_pending','inactive'} else None}))
+        raise SystemExit(1)

tests/test_mfa_state_diagnostic.py

@@ -0,0 +1,233 @@
+"""Diagnostic regressions: private inputs, actual HTTP transport, fail-closed counts."""
+import base64
+import copy
+import importlib.util
+import json
+import os
+from pathlib import Path
+import subprocess
+import sys
+import threading
+import time
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from urllib.parse import parse_qs, urlsplit
+
+import pytest
+
+ROOT = Path(__file__).resolve().parents[1]
+HELPERS = ROOT / 'sso-mfa/k8s/privacyidea'
+spec = importlib.util.spec_from_file_location('mfa_state', HELPERS / 'mfa_state.py')
+m = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(m)
+
+
+def jwt(**changes):
+    claims = {'role': 'admin', 'exp': time.time() + 300, **changes}
+    encoded = base64.urlsafe_b64encode(json.dumps(claims).encode()).rstrip(b'=').decode()
+    return 'eyJhbGciOiJIUzI1NiJ9.' + encoded + '.syntheticSignature'
+
+
+def page(rows):
+    return {'tokens': rows, 'count': len(rows), 'current': 1, 'next': None, 'prev': None}
+
+
+def factor(serial='synthetic-factor', **changes):
+    return {'serial': serial, 'active': True, 'user_id': '1001', 'username': 'alice',
+            'user_realm': 'coulomb', 'realms': ['coulomb'], 'rollout_state': 'enrolled', **changes}
+
+
+def probe(**changes):
+    return factor(m.PROBE, active=False, user_id='', username='', **changes)
+
+
+@pytest.fixture
+def server():
+    state = {'user': page([factor()]), 'proof': page([probe()]), 'requests': [], 'calls': 0}
+    class Handler(BaseHTTPRequestHandler):
+        def log_message(self, *_): pass
+        def do_GET(self):
+            state['requests'].append((self.path, dict(self.headers)))
+            state['calls'] += 1
+            if state.get('status'):
+                self.send_response(state['status'])
+                self.send_header('Location', state.get('location', '/stolen'))
+                self.end_headers(); self.wfile.write(b'private-provider-message'); return
+            query = parse_qs(urlsplit(self.path).query)
+            value = state['proof'] if 'serial' in query else state['user']
+            if state.get('withdraw_after_user') and state['calls'] >= 3: value = page([])
+            body = json.dumps({'result': {'status': True, 'value': value}}).encode()
+            self.send_response(200); self.end_headers(); self.wfile.write(state.get('raw', body))
+    s = ThreadingHTTPServer(('127.0.0.1', 0), Handler)
+    t = threading.Thread(target=s.serve_forever, daemon=True); t.start()
+    try: yield f'http://127.0.0.1:{s.server_port}', state
+    finally: s.shutdown(); s.server_close(); t.join()
+
+
+@pytest.fixture
+def token_file(tmp_path):
+    private = tmp_path / 'private'; private.mkdir(mode=0o700)
+    path = private / 'input'; path.write_text(jwt() + '\n'); path.chmod(0o600)
+    return path
+
+
+def invoke(token_file, url, *extra):
+    return subprocess.run(['bash', '-x', str(HELPERS / 'check-user-mfa-state.sh'), 'alice',
+                           '--token-file', str(token_file), '--pi-url', url, *extra],
+                          capture_output=True, text=True, timeout=15)
+
+
+def test_real_http_cli_reports_complete_private_counts(server, token_file, monkeypatch):
+    url, state = server
+    state['user'] = page([factor(str(i)) for i in range(18)] +
+                         [factor('pending', rollout_state='verify'), factor('inactive', active=False)])
+    monkeypatch.setenv('http_proxy', 'http://127.0.0.1:1'); monkeypatch.setenv('HTTP_PROXY', 'http://127.0.0.1:1')
+    monkeypatch.setenv('no_proxy', ''); monkeypatch.setenv('NO_PROXY', '')
+    p = invoke(token_file, url)
+    assert p.returncode == 0, p.stderr
+    data = json.loads(p.stdout)
+    assert data['factors'] == {'total': 20, 'active_confirmed': 18, 'active_pending': 1, 'inactive': 1}
+    assert data['user_existence'] == 'not_checked' and data['mfa_login'] == 'not_tested'
+    assert len(state['requests']) == 3
+    for path, headers in state['requests']:
+        assert urlsplit(path).path == '/token/' and 'Content-Type' not in headers
+        assert headers['Authorization'] == token_file.read_text().strip()
+    assert token_file.read_text().strip() not in p.stdout + p.stderr
+    assert str(token_file) not in p.stdout + p.stderr
+    assert 'synthetic-factor' not in p.stdout
+
+
+@pytest.mark.parametrize('mutation', ['empty', 'assigned', 'active', 'realm', 'serial', 'count', 'withdraw'])
+def test_scope_uncertainty_never_reports_zero(server, mutation):
+    url, state = server; state['user'] = page([])
+    if mutation == 'empty': state['proof'] = page([])
+    elif mutation == 'assigned': state['proof']['tokens'][0]['user_id'] = 'someone'
+    elif mutation == 'active': state['proof']['tokens'][0]['active'] = True
+    elif mutation == 'realm': state['proof']['tokens'][0]['realms'] = ['other']
+    elif mutation == 'serial': state['proof']['tokens'][0]['serial'] = 'other'
+    elif mutation == 'count': state['proof']['count'] = 2
+    elif mutation == 'withdraw': state['withdraw_after_user'] = True
+    with pytest.raises(m.Refused): m.diagnose(m.Client(url, jwt()), 'alice')
+
+
+def test_empty_verified_result_is_not_proof_user_exists(server):
+    url, state = server; state['user'] = page([])
+    data = m.diagnose(m.Client(url, jwt()), 'alice')
+    assert data['factors']['total'] == 0 and data['user_existence'] == 'not_checked'
+
+
+@pytest.mark.parametrize('mutation', ['truncated', 'duplicate', 'next', 'count-bool', 'current-bool', 'wrong-user', 'wrong-realm', 'missing-active', 'missing-state', 'unknown-state'])
+def test_malformed_or_incomplete_user_result_refused(server, mutation):
+    url, state = server; value = state['user']; row = value['tokens'][0]
+    if mutation == 'truncated': value['count'] = 20
+    elif mutation == 'duplicate': value['tokens'].append(copy.deepcopy(row)); value['count'] = 2
+    elif mutation == 'next': value['next'] = 2
+    elif mutation == 'count-bool': value['count'] = True
+    elif mutation == 'current-bool': value['current'] = True
+    elif mutation == 'wrong-user': row['username'] = 'bob'
+    elif mutation == 'wrong-realm': row['user_realm'] = 'other'
+    elif mutation == 'missing-active': row.pop('active')
+    elif mutation == 'missing-state': row.pop('rollout_state')
+    elif mutation == 'unknown-state': row['rollout_state'] = 'new-unreviewed-state'
+    with pytest.raises(m.Refused): m.diagnose(m.Client(url, jwt()), 'alice')
+
+
+@pytest.mark.parametrize('mode', ['public', 'symlink', 'hardlink', 'fifo', 'directory', 'public-parent', 'bad-jwt', 'self-service', 'expired', 'oversized', 'inside-git'])
+def test_bad_input_precedes_any_network(server, token_file, mode):
+    url, state = server
+    if mode == 'public': token_file.chmod(0o644)
+    elif mode == 'symlink': token_file.rename(token_file.with_suffix('.real')); token_file.symlink_to(token_file.with_suffix('.real'))
+    elif mode == 'hardlink': os.link(token_file, token_file.with_suffix('.link'))
+    elif mode == 'fifo': token_file.unlink(); os.mkfifo(token_file, 0o600)
+    elif mode == 'directory': token_file.unlink(); token_file.mkdir(mode=0o700)
+    elif mode == 'public-parent': token_file.parent.chmod(0o755)
+    elif mode == 'bad-jwt': token_file.write_text('private-secret-not-a-jwt')
+    elif mode == 'self-service': token_file.write_text(jwt(role='user'))
+    elif mode == 'expired': token_file.write_text(jwt(exp=1))
+    elif mode == 'oversized': token_file.write_bytes(b'x' * 16386)
+    elif mode == 'inside-git': (token_file.parent / '.git').mkdir(); (token_file.parent / '.git/HEAD').write_text('ref: refs/heads/main')
+    p = invoke(token_file, url)
+    assert p.returncode == 1 and not p.stdout and 'unverified' in p.stderr
+    assert 'private-secret' not in p.stderr and not state['requests']
+
+
+@pytest.mark.parametrize('url', ['https://attacker.invalid', 'http://pink.coulomb.social', 'http://localhost:8000',
+                                'http://127.0.0.1:8000/path', 'http://127.0.0.1:8000?x=1',
+                                'https://user:password@pink.coulomb.social', 'http://127.1:8000'])
+def test_destination_is_checked_before_credential(url, tmp_path):
+    p = invoke(tmp_path/'absent', url)
+    assert p.returncode == 1 and not p.stdout and 'password' not in p.stderr
+
+
+@pytest.mark.parametrize('status', [301, 302, 307, 308, 401, 403, 500])
+def test_no_redirect_or_provider_error_leak(server, token_file, status):
+    url, state = server; state['status'] = status
+    p = invoke(token_file, url)
+    assert p.returncode == 1 and not p.stdout
+    assert len(state['requests']) == 1 and 'private-provider-message' not in p.stderr
+
+
+@pytest.mark.parametrize('raw', [b'{"result":{"status":false,"error":{"message":"private-provider-message"}}}',
+                                 b'{"result":{"status":true,"status":false}}', b'not-json', b'x'*(m.MAX_BODY+1)], ids=['error', 'duplicate', 'invalid', 'oversized'])
+def test_bad_provider_body_is_sanitized(server, token_file, raw):
+    url, state = server; state['raw'] = raw
+    p = invoke(token_file, url)
+    assert p.returncode == 1 and not p.stdout and 'private-provider-message' not in p.stderr
+
+
+def test_unknown_argument_is_not_echoed(server, token_file):
+    url, state = server
+    p = invoke(token_file, url, '--unknown=private-argument')
+    assert p.returncode == 1 and 'private-argument' not in p.stderr and not state['requests']
+
+
+@pytest.mark.parametrize('username', ['alice&realm=other', '*', 'alice@other', '../alice'])
+def test_provider_expansion_is_refused_before_request(server, username):
+    url, state = server
+    with pytest.raises(m.Refused): m.diagnose(m.Client(url, jwt()), username)
+    assert not state['requests']
+
+
+def test_real_privacyidea_contract():
+    pytest.importorskip('privacyidea')
+    p = subprocess.run([sys.executable, str(ROOT/'tests/mfa_state_native_fixture.py')],
+                       capture_output=True, text=True, timeout=180)
+    assert p.returncode == 0, p.stdout
+    data = json.loads(p.stdout)
+    assert len(data) == 7 and all(v is True for v in data.values())
+
+
+def test_untrusted_https_is_not_accepted_via_ambient_ca(tmp_path, token_file, monkeypatch):
+    import datetime
+    import ipaddress
+    import ssl
+    from cryptography import x509
+    from cryptography.hazmat.primitives import hashes, serialization
+    from cryptography.hazmat.primitives.asymmetric import rsa
+    from cryptography.x509.oid import NameOID
+    key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
+    name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, 'fixture')])
+    now = datetime.datetime.now(datetime.timezone.utc)
+    cert = (x509.CertificateBuilder().subject_name(name).issuer_name(name).public_key(key.public_key())
+            .serial_number(x509.random_serial_number()).not_valid_before(now-datetime.timedelta(minutes=1))
+            .not_valid_after(now+datetime.timedelta(hours=1))
+            .add_extension(x509.SubjectAlternativeName([x509.IPAddress(ipaddress.ip_address('127.0.0.1'))]), False)
+            .sign(key, hashes.SHA256()))
+    cert_path = tmp_path/'cert.pem'; key_path = tmp_path/'key.pem'
+    cert_path.write_bytes(cert.public_bytes(serialization.Encoding.PEM))
+    key_path.write_bytes(key.private_bytes(serialization.Encoding.PEM, serialization.PrivateFormat.PKCS8, serialization.NoEncryption()))
+    key_path.chmod(0o600)
+    seen = []
+    class Handler(BaseHTTPRequestHandler):
+        def log_message(self, *_): pass
+        def do_GET(self): seen.append(dict(self.headers)); self.send_error(500)
+    server = ThreadingHTTPServer(('127.0.0.1', 0), Handler)
+    context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER); context.load_cert_chain(cert_path, key_path)
+    server.socket = context.wrap_socket(server.socket, server_side=True)
+    thread = threading.Thread(target=server.serve_forever, daemon=True); thread.start()
+    monkeypatch.setenv('SSL_CERT_FILE', str(cert_path))
+    try:
+        p = invoke(token_file, f'https://127.0.0.1:{server.server_port}')
+        assert p.returncode == 1 and not p.stdout and not seen
+        assert token_file.read_text().strip() not in p.stderr
+    finally:
+        server.shutdown(); server.server_close(); thread.join()