Previously when encountering a CA-signed cert that failed verification with "Hostname mismatch",
we would
1. erroneously mark it as self-signed
2. save its cert to pin it
3. when connecting to it later, and being served a CA-signed cert, we would reject the connection
- I think this is because we use the saved cert (the peer cert, just the last cert in the chain) as if it was a root CA,
and then during the connection we try to verify against that root. This fails as we are served a different root then.
Error logged in step(3):
```
3.85 | W | i/interface.[wirg2tsto7rme7n26lkd3ivbvxmjyy2pktlozwjuep22jcsfsghfqbqd.onion:50002] | Cannot connect to main server due to SSL error (maybe cert changed compared to "/home/user/.electrum/testnet/certs/wirg2tsto7rme7n26lkd3ivbvxmjyy2pktlozwjuep22jcsfsghfqbqd.onion"). Exc: ConnectError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1007)'))
```
This commit fixes step(1), we won't mark the cert as self-signed, instead the error is propagated out and the connection closed.
```
35.05 | I | i/interface.[wirg2tsto7rme7n26lkd3ivbvxmjyy2pktlozwjuep22jcsfsghfqbqd.onion:50002] | disconnecting due to: ErrorGettingSSLCertFromServer(ConnectError(SSLCertVerificationError(1, "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'wirg2tsto7rme7n26lkd3ivbvxmjyy2pktlozwjuep22jcsfsghfqbqd.onion'. (_ssl.c:1007)")))
```
Compare:
- SSLCertVerificationError(1, "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'wirg2tsto7rme7n26lkd3ivbvxmjyy2pktlozwjuep22jcsfsghfqbqd.onion'. (_ssl.c:1007)")
- verify_code=62
- SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:1007)')
- verify_code=18
Note: the verify_code constants look stable, though they might be openssl-specific. I guess that's ok(?)
140540189c/include/openssl/x509_vfy.h.in (L224)
instead, schedule a queued finished signal to unregister the listener after the handler has finished.
See PythonActivity.java in P4A for why this probably causes the most often occurring crash we see on the Play Store:
```
Exception java.lang.RuntimeException:
at android.app.ActivityThread.deliverResults (ActivityThread.java:5164)
at android.app.ActivityThread.handleSendResult (ActivityThread.java:5205)
at android.app.servertransaction.ActivityResultItem.execute (ActivityResultItem.java:51)
at android.app.servertransaction.TransactionExecutor.executeCallbacks (TransactionExecutor.java:135)
at android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:95)
at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2136)
at android.os.Handler.dispatchMessage (Handler.java:106)
at android.os.Looper.loop (Looper.java:236)
at android.app.ActivityThread.main (ActivityThread.java:8061)
at java.lang.reflect.Method.invoke
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:656)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:967)
Caused by java.util.ConcurrentModificationException:
at java.util.ArrayList$Itr.next (ArrayList.java:860)
at org.kivy.android.PythonActivity.onActivityResult (PythonActivity.java:218)
at android.app.Activity.dispatchActivityResult (Activity.java:8501)
at android.app.ActivityThread.deliverResults (ActivityThread.java:5157)
```
The low-S rule for ecdsa signatures is mandated by Bitcoin Core policy/standardness (though not by consensus). If we get signatures from untrusted sources, we should mandate they obey the policy rules. (e.g. from LN peers)
Note that we normalize the signatures in the sig format conversion methods (DER <-> (r,s) <-> sig64).
The BOLTs treat high-S signatures as invalid, and this changes our behaviour to that.
(previously we would silently normalize the S value)
see https://github.com/bitcoin/bitcoin/pull/6769
see https://github.com/lightning/bolts/pull/807
get_preimage_script should really have been private API...
looks like everywhere it is used outside of transaction.py, it is actually abused :/
Other existing usages in plugin code I don't dare to touch without lots of manual testing...
In fact, semantically it might be more correct to only trigger 'blockchain_updated' and not 'network_updated' here...
Anyway, 'blockchain_updated' should be triggered whenever the Blockchain object gets longer (or changes otherwise).
In particular, in qml, the NetworkOverview only updates the displayed height on 'blockchain_updated'.
SomberNight
ThomasV
accumulator
jinjiadu
Ondřej Vejpustek