Skip to content

Implement TLS in arti-hyper

Ian Jackson requested to merge Diziet/arti:tls into main

Most of this ought to be reasonably uncontroversial.

I have tested it and it WFM. Yesterday I found it suffered from #365 (closed). Today it doesn't, even with the exact code from yesterday (the commit named there). So I think there is still a latent bug, but the output in #365 (closed) doesn't suggest to me that this bug is in arti-hyper.

Controversial points I'd like to draw your attention to:

  • The new ErrorKind::OtherRemote ought to be debated. There are other possibilities. I'm not firmly set on ErrorKind::OtherRemote. I do think that arti_hyper::ConnectionError ought to impl HasKind, and that none of the kinds currently available are right.
  • One .clone() here relies on !352 (merged). So this branch is therefore on top of a merge of !352 (merged) with current main.
  • I chose to use tls-api to make the arti-hyper not depend on a particular TLS library. I think this was the right choice. The alternatives would seem to be to extend tor_rtcompat::TlsConnector etc. so that these things can be used as general TLS libraries (urgH) or to weld the arti-hyper library to a particular TLS implementation (worse).

Merge request reports