Request feedback about my messaging app over Tor

code review
1

Hi

I really enjoy programming in Rust. I currently don't use it professionally but I want to change that once I am confident in my skills. Since I don't use Rust professionally (yet), on most of my code I don't receive the feedback necessary to learn and evolve.

Recently I am working on a hobby-project to improve my Rust skills called Arti Chat. It is a privacy-focused decentralized peer-to-peer e2ee messaging app which uses The Tor Network to provide anonymity. But without feedback I can't improve myself and get rid of bad code or habits.

Currently the project is still in draft but I'd really appreciate some interim feedback or tips.

Repository: GitHub - NielDuysters/arti-chat: Decentralized peer-to-peer messenger with reasonable anonymity over Tor.

Some things I'd like feedback on:

Trivial questions

  • What part of the current setup worries you most?
  • If you could fix one thing in this, what would it be?
  • Where would you try to attack this app if you were an advesary? Where do you suspect vulnerabilities exist?

Rust code

  • Do you see any anti-patterns in the code?
  • Am I currently applying bad habits in my code?
  • Is my code readable?
  • Am I handling async and threading correctly?
  • How is my error handling?
  • PS. cargo clippy will be fixed soon.

Full review notes: arti-chat/docs/review-notes.md at main · NielDuysters/arti-chat · GitHub

Thanks a lot!

2

FYI: I am not asking for a deep review of my code or an answer on all questions, some small tips or global feedback of your first impression would already help a lot.

3

My thoughts, from a brief look, as a non-expert myself:

  • It might help to clarify the threat model you have in mind. That is, what threats/attacks do you intend to protect things from, and what things are you protecting?
  • You have ~51 dependencies according to NielDuysters / arti-chat - Deps.rs. It might be a good idea to add cargo-audit and something like cargo-vet or cargo-crev to your CI. Vulnerable/compromised dependencies tend to be my first thought.
  • A lot of your Error types seem to be named <Type of error>Error. I personally struggle with this too, but it might be worth considering names without the Error on the end. They all live in the error module as it is, so I think it'd be nicer to say, for example, error::Daemon than error::DaemonError.
  • You seem to be relying on Drop to clear secrets from memory. My understanding, based on the ANSSI-FR Secure Rust Guidelines, is that you should not rely on Drop for such purposes, as Drop is not guaranteed to happen. I imagine you should probably manually call zeroize when you're done with secrets.
2 Likes
4

Thanks a lot for your reply. Your suggestions seem valid and I'll implement them.

I listed this in security.md.