fix: the derivedchildkeyhandler in handlers.py

[anupamme]

Summary

Fix critical severity security issue in plugins/keyrotation/handlers.py.

Vulnerability

Field	Value
ID	V-002
Severity	CRITICAL
Scanner	multi_agent_ai
Rule	V-002
File	plugins/keyrotation/handlers.py:780
Assessment	Likely exploitable
Chain Complexity	2-step

Description: The DerivedChildKeyHandler.post() method performs critical key rotation operations without verifying that the requester owns the identity associated with the public_key. While the handler validates that the public_key matches the prerotated_key_hash in the KEL, any user who knows a valid public_key and second_factor can trigger a rotation for any identity. The second_factor alone provides insufficient protection against determined attackers.

Evidence

Exploitation scenario: Attacker obtains victim's public_key (publicly visible on blockchain) and guesses or steals the second_factor, then POSTs to /key-rotation/derived-child-key with {"public_key":.

Scanner confirmation: multi_agent_ai rule V-002 flagged this pattern.

Production code: This file is in the production codebase, not test-only code.

Threat Model Context

This is a web service - vulnerabilities in request handlers are directly exploitable by remote attackers.

Changes

• plugins/keyrotation/handlers.py

Behavior Preservation

The change is scoped to 1 file on the vulnerable path.

Security Invariant

Property: The security boundary is maintained under adversarial input

Regression test

import pytest
import json
from unittest.mock import MagicMock, patch


@pytest.mark.parametrize("payload", [
    # Exploit: attacker knows valid public_key and second_factor, tries to rotate another's key
    {"public_key": "ATTACKER_KNOWN_VALID_KEY", "second_factor": "KNOWN_SECOND_FACTOR", "relationship": ""},
    # Boundary: empty/missing identity verification
    {"public_key": "", "second_factor": "", "relationship": ""},
    # Valid structure (should fail without proper auth, not crash)
    {"public_key": "VALID_FORMAT_KEY_12345", "second_factor": "valid_second_factor_123", "relationship": "test"},
])
def test_derived_child_key_handler_requires_identity_ownership(payload):
    """Invariant: Key rotation operations MUST verify the requester owns the identity before processing."""
    from plugins.keyrotation.handlers import DerivedChildKeyHandler
    
    handler = DerivedChildKeyHandler(MagicMock(), MagicMock())
    handler.request = MagicMock()
    handler.request.body = json.dumps(payload).encode()
    handler.set_status = MagicMock()
    handler.render_as_json = MagicMock()
    
    # Mock KEL lookup to simulate valid public_key exists
    with patch('yadacoin.core.keyeventlog.KeyEventLog.get') as mock_kel:
        mock_kel.return_value = MagicMock(prerotated_key_hash="mock_hash")
        
        # Execute the handler
        import asyncio
        asyncio.run(handler.post())
        
        # Security invariant: must NOT succeed without identity ownership verification
        # Handler should reject (400/403) or require additional auth, never allow rotation
        status_calls = handler.set_status.call_args_list
        render_calls = handler.render_as_json.call_args_list
        
        # If status is 200/OK, verify response indicates failure (not successful rotation)
        if status_calls and status_calls[-1][0][0] == 200:
            response = render_calls[-1][0][0] if render_calls else {}
            assert response.get("status") is not True, "Security violation: rotation allowed without identity ownership"

This test guards against regressions — it's useful independent of the code change above.

---

Automated security fix by OrbisAI Security