feat: check it out and use "TOFU" (trust on first use) for self-signed certificates

[oleonardolima]

Instead of just ignoring and not validating the self-signed certificates, we probably should use the "TOFU" (trust on first use) for the self-signed certificates.

[oleonardolima]

see: frostsnap/frostsnap#278

[tnull]

How will this work without adding persistence to electrum-client that would allow to store previously-seen certificates?

[oleonardolima]

I didn't put much thought into it yet, need to review thoroughly the frostnsnap PR. At first I thought to have it as an option that you pass the map with trusted certificates when you're creating the client, but you'd need to store it somewhere else.

It would make previously-seen certificates usage easier if we add persistance such as frostsnap did, but still not sure if the overhead of yet another dependency is worth it.

[lucasdbr05]

I didn't put much thought into it yet, need to review thoroughly the frostnsnap PR. At first I thought to have it as an option that you pass the map with trusted certificates when you're creating the client, but you'd need to store it somewhere else.

It would make previously-seen certificates usage easier if we add persistance such as frostsnap did, but still not sure if the overhead of yet another dependency is worth it.

I’m working on this issue in a fork. The part about persisting already-seen certificates in a SQLite database (as done in Frostsnap) would still be simple to set up, but it would introduce more dependencies. I’ve been thinking about saving a JSON file that I read from and write to, storing the corresponding certificate for each host that has been seen. However, even though this avoids adding more dependencies, I don’t consider it the best option.

Do you think it would be better to keep using SQLite, or should I switch to this approach of saving a JSON file?

[tnull]

Do you think it would be better to keep using SQLite, or should I switch to this approach of saving a JSON file?

IMO neither is really acceptable for a utility crate such as rust-electrum-client, which shouldn't assume any particular kind of IO/persistence. If we want to support TOFU, it would probably need to happen behind a feature that exposes a trait or callback letting the user know they need to persist a certain certificate. The persisted certificates could then also be given as an argument to an alternative constructor.

[lucasdbr05]

Do you think it would be better to keep using SQLite, or should I switch to this approach of saving a JSON file?

IMO neither is really acceptable for a utility crate such as rust-electrum-client, which shouldn't assume any particular kind of IO/persistence. If we want to support TOFU, it would probably need to happen behind a feature that exposes a trait or callback letting the user know they need to persist a certain certificate. The persisted certificates could then also be given as an argument to an alternative constructor.

I have a few updates regarding my progress on this issue.

Indeed, I was heading in a not-so-correct direction by adding a database dependency for our own implementation. Instead, the solution now consists of exposing a trait responsible for certificate persistence.

It is up to the user to provide their own implementation and pass it through the ConfigBuilder. During client initialization, we check whether an implementation was provided; if so, the client establishes an SSL connection using TOFU logic. This implementation was adapted to handle the specifics of both rustls and open-ssl, ensuring correct behavior regardless of which one is chosen.

Could you give me some feedback on this approach?

[tnull]

I have a few updates regarding my progress on this issue.

Indeed, I was heading in a not-so-correct direction by adding a database dependency for our own implementation. Instead, the solution now consists of exposing a trait responsible for certificate persistence.

It is up to the user to provide their own implementation and pass it through the ConfigBuilder. During client initialization, we check whether an implementation was provided; if so, the client establishes an SSL connection using TOFU logic. This implementation was adapted to handle the specifics of both rustls and open-ssl, ensuring correct behavior regardless of which one is chosen.

Could you give me some feedback on this approach?

Generally sounds good, a point of discussion could be whether to pass it as part of ConfigBuilder or via a separate constructor. For further feedback it would help if you open a draft PR, which would allow us to see how the concrete code changes look like.

[lucasdbr05]

I have a few updates regarding my progress on this issue.
Indeed, I was heading in a not-so-correct direction by adding a database dependency for our own implementation. Instead, the solution now consists of exposing a trait responsible for certificate persistence.
It is up to the user to provide their own implementation and pass it through the ConfigBuilder. During client initialization, we check whether an implementation was provided; if so, the client establishes an SSL connection using TOFU logic. This implementation was adapted to handle the specifics of both rustls and open-ssl, ensuring correct behavior regardless of which one is chosen.
Could you give me some feedback on this approach?

Generally sounds good, a point of discussion could be whether to pass it as part of ConfigBuilder or via a separate constructor. For further feedback it would help if you open a draft PR, which would allow us to see how the concrete code changes look like.

Draft PR opened (see #193).

@tnull @oleonardolima