feat(connd): wifi profile encryption #833
Conversation
This reverts commit b5a6d96.
| Environment=DBUS_SESSION_BUS_ADDRESS=unix:path=/tmp/worldcoin_bus_socket | ||
| Environment=RUST_BACKTRACE=1 | ||
| ExecStart=/usr/local/bin/orb-connd | ||
| ExecStart=/usr/local/bin/orb-connd connd |
There was a problem hiding this comment.
Probably better to just default to the main entry point and make this subcommand the default
|
|
||
| let sp = SettingsProxy::new(&self.conn).await?; | ||
| let path = sp.add_connection(settings).await?; | ||
| let path = if persist { |
There was a problem hiding this comment.
It might be a better idea to delete the persistence codepaths now, wdyt?
There was a problem hiding this comment.
i'd rather leave this one in for now, it's an NM abstraction, its not supposed to be specific to connd 😅
orb-connd/src/profile_store/mod.rs
Outdated
|
|
||
| let path = self.store_path.join(Self::FILENAME); | ||
| fs::write(path, bytes).await?; | ||
| self.secure_storage.put(Self::KEY.into(), bytes).await?; |
There was a problem hiding this comment.
I recommend wrap_erring all the spots that interact with secure_storage so that if there are errors, we can more easily identify them
orb-connd/src/secure_storage/mod.rs
Outdated
| pub fn new(cancel: CancellationToken) -> Self { | ||
| self::subprocess::spawn(1, cancel) | ||
| pub fn new( | ||
| exe_path: impl AsRef<Path> + 'static, |
There was a problem hiding this comment.
why not just &Path? IMO its overcomplicating things to use generics for these sorts of things. Most of the stdlib uses &str not AsRef for example
There was a problem hiding this comment.
i honestly don't think it matters here at all 😅
went ahead and changed it to PathBuf to avoid lifetime complaints
| (EntryPoint::SecureStorage as u8).to_string(), | ||
| ) | ||
| let mut child = tokio::process::Command::new(exe_path.as_ref()) | ||
| .arg("ssd") |
There was a problem hiding this comment.
imo we should not make names too short, its not clear what ssd is.
There was a problem hiding this comment.
fair, switched it to secure-storage-worker
| result | ||
| } | ||
|
|
||
| fn connectivity_daemon() -> Result<()> { |
There was a problem hiding this comment.
I might rename it to just main_entry. Its kinda confusing to call it a daemon, it implies its not the primary process. I would also prefer to refer to the secure storage stuff as a "worker" rather than a daemon, to avoid confusion with systemd daemons.
There was a problem hiding this comment.
tbh i prefer to keep it connectivity_daemon here, i feel like its less ambiguous
when we run the binary it'll either be a connectivity daemon or a secure storage worker, i'd rather keep that explicit for anyone looking in from main.rs 🙏
orb-connd/tests/fixture.rs
Outdated
| .bin("orb-connd") | ||
| .current_target() | ||
| .current_release() | ||
| .manifest_path(concat!(env!("CARGO_MANIFEST_DIR"), "/Cargo.toml")) |
There was a problem hiding this comment.
| .manifest_path(concat!(env!("CARGO_MANIFEST_DIR"), "/Cargo.toml")) | |
| .manifest_path(env!("CARGO_MANIFEST_PATH)) |
|
I am really happy with how the tests all turned out. I think you should give a presentation about your approaches to testing in rust. |
ty!! i want to try to prepare something for January to help out with this and answer any questions 🙏 |
|
ready to merge, we are just blocked on CI |
TheButlah
force-pushed
the
vm/wifi-profile-encryption
branch
from
b79375d to
ab78f69
Compare
use secure storage for storing wifi profiles
todo