Sweep: ship deterministic-nonce fix to master + make TLS verification fail closed

[RobGanon]

Issue draft — TruthMachine/ElectrumSVP

Title: Sweep: ship deterministic-nonce fix to master + make TLS verification fail closed

Context: Follow-up to closed PR #14 (sweep nonce non-determinism + TLS hostname verification). Two standalone, low-risk changes; neither is currently in a released build.

1. Sweep signing still uses a random nonce in the released build (v0.1.0)
electrumsv/gui/qt/sweep_helper.py signs sweep inputs with ecdsa.SigningKey.sign_digest(...), which falls back to an os.urandom-based nonce when no k is supplied. On a weak/broken RNG, nonce reuse across the multiple inputs a single key signs in a sweep can allow private-key recovery. Every other signing path in the wallet already uses RFC6979 (bitcoinx PrivateKey.sign → libsecp256k1); the sweep path is the only exception.

• master still has this line today; v0.1.0 AppImage ships it.
• Your dev branch reportedly already signs sweeps via bitcoinx/libsecp256k1 — please merge that to master and cut a (minor) release. Shipping it is what actually closes the hole for users.

2. ElectrumX fallback effectively disables server authentication
create_ssl_context() sets ctx.check_hostname = False while the socket's trust store includes the full certifi CA set. With hostname checks off and every public CA trusted, a MITM who obtains any public-CA-signed certificate can impersonate an ElectrumX server with no verification failure.

• I understand the self-signed servers (e.g. CN=electrumr, no SAN for sv2.satoshi.io) fail check_hostname=True as-is — removing the flag alone would break them, as discussed in Fix sweep nonce non-determinism and disabled TLS hostname verification #14.
• Proposed alternatives that keep current servers working and fail closed:
  1. Load only the server-specific <host>.pem (and skip certifi for these sockets) while keeping check_hostname = True; or
  2. Pin the exact server certificate/fingerprint (TOFU), failing on mismatch. Since this path is a direct TCP/TLS fallback, a dedicated pinned context is the safer design.
  3. Long-term: move servers to CA-issued certs with correct SAN.
• Impact of the current state is bounded (a MITM can feed wrong/incomplete UTXO data → failed or partial sweeps; it cannot forge signatures or divert funds), but it's below the bar we'd set for a wallet, and worth closing.

What I'd love from you

• Merge/release the deterministic-nonce sweep signing (item 1).
• Pick the pinned/fail-closed TLS approach you prefer for item 2 and let us know if you want help validating it.

[TruthMachine]

Thanks for opening this. Item 1 is already addressed in my current development build. The sweep signing path has been migrated from ecdsa.SigningKey.sign_digest() to bitcoinX.PrivateKey.sign(), so the sweep path now uses the same libsecp256k1-based signing implementation as the other wallet signing paths. I intend to include that in the next release.

I also want to clarify the severity of the deterministic-nonce issue. I agree that deterministic signing is preferable and I have already corrected the sweep signing path in my development build by moving it to bitcoinX/libsecp256k1. However, I don't believe the issue is as severe in practice as the description suggests. The concern depends on nonce reuse or sufficiently bad randomness resulting in private-key exposure across signatures. The existing sweep implementation did not deterministically reuse a nonce; it used the ECDSA library's normal random nonce generation. So I view this primarily as a robustness/security improvement rather than evidence that sweep transactions were routinely exposing private keys.

I will nevertheless include the deterministic signing change in the next release, since there is no reason for the sweep path to use a different signing implementation from the rest of the wallet.

Regarding TLS, I want to clarify one point about the current implementation. ElectrumSVP does use server-specific PEM certificates via load_verify_locations(cafile=...). So this isn't simply relying on the system/Certifi CA store with hostname verification disabled; the configured server certificate is explicitly trusted.

I agree, however, that disabling hostname verification means the certificate's identity is not checked against the configured hostname. I have now tested this with multiple servers. For example, electrumx.gorillapool.io currently presents a self-signed certificate for CN=gorillapool, while sv2.satoshi.io presents one for CN=electrumr; neither certificate identifies its configured hostname. Simply enabling check_hostname=True would therefore break those existing servers.

I think the TLS architecture should be addressed separately, with explicit server-specific certificate trust while retaining compatibility with the existing self-signed servers. Ideally, the servers would eventually use properly issued certificates with SANs matching their configured hostnames, which would allow standard hostname verification to be used.

So I'll address the deterministic signing fix in the next release and handle the TLS architecture separately. Thanks for raising both issues.