search

fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
580 stars 226 forks source link

Make gateway password required on first startup #6427

Open tvolk131 opened 1 day ago

tvolk131 commented 1 day ago

When starting the gateway, we call Gateway::get_gateway_configuration() which loads the GatewayConfiguration from the DB if it exists, and attempts to create one and save it to the DB if it doesn't exist yet:

https://github.com/fedimint/fedimint/blob/c660335d1e48f1d0909d66b4aaf0ac68731638a5/gateway/ln-gateway/src/lib.rs#L1737-L1772

It can return None, but only if a gateway config isn't present in the DB and a password is not provided in the GatewayParameters:

https://github.com/fedimint/fedimint/blob/c660335d1e48f1d0909d66b4aaf0ac68731638a5/gateway/ln-gateway/src/lib.rs#L1749

If we instead panicked on this condition, requiring the gateway operator to provide a password when starting up a brand new gateway for the first time, we would be able to significantly simplify the gateway, including removing the GatewayState::Configuring state and changing the type of Gateway::gateway_config from Arc<RwLock<Option<GatewayConfiguration>>> to Arc<RwLock<GatewayConfiguration>> which allows for lots of downstream simplifications.

m1sterc001guy commented 1 day ago

Historically, the gateway did not have a database so the only way to provide the password was via an environment variable. When we added a database, we wanted to maintain backwards compatibility and allow devimint to still provide the password via the environment variable. That's how we ended up here, which I agree is not super clean.

I would actually be more in favor of not allowing the user to specify the password via an environment variable at all and require them to do set it the first time the gateway boots, which would then get persisted in the database. We would still keep the Configuring state, but I believe a lot of the other simplifications mentioned here can also be done if we do that.

I don't really like forcing the user to set the password via an environment variable that then gets persisted to the database. Because after the first boot of the gateway, that environment variable is now useless which I think is confusing to the operator (i.e they might think it still does something).

elsirion commented 1 day ago

I don't really like forcing the user to set the password via an environment variable that then gets persisted to the database. Because after the first boot of the gateway, that environment variable is now useless which I think is confusing to the operator (i.e they might think it still does something).

One problem the gateway has that fedimintd doesn't is that it might already be connected to a funded LN node on startup, so it's not safe to stay in an "uninitialized" state, where anyone can just set a password and become admin. Even for fedimintd that's not ideal.

I like how for example Nodana generates a random password for you when setting up a gateway (gateway service not publicly announced and broken right now, but guardians work great!). That's a use case we should still support imo, the way we do it might not be env vars though and should make clear it's only an initial thing (or we pull the password out of the config and just always use the supplied one).

One thing we could improve is allowing the password to be supplied pre-hashed. That way the password env var could be safely set in config that is world-readable (e.g. nix config).