Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Certificate meta-information file #77

Open
breard-r opened this issue Jan 28, 2023 · 9 comments
Open

Certificate meta-information file #77

breard-r opened this issue Jan 28, 2023 · 9 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@breard-r
Copy link
Owner

There will be a point where it is necessary to store informations about a certificate that do not belong to the configuration. For example, this may be necessary to implement STAR certificates (RFC 8739). I have not read the full specification, but from the few I read it seems necessary to keep track of some state on the client side, at least in order to know if it's a "traditional" certificate or a STAR.

Where to store this file?

There is multiple possibilities, the one I find the more promising being:

  • one file per certificate stored along with the private key and the certificate
  • one file per certificate stored in a dedicated directory
  • one unique file stored I don't know where which contains the entries for all the certificates

I would prefer the one file per certificate approche since it would be coherent with what has already be done for the accounts and is easier for people to manage.
Using this approche, the file_name_format will be used with:

  • file_type set to something like meta
  • ext set to bin (if bincode is chosen)

Which file format?

The file format itself should be the same one as the account's. Currently that is the binary format provided by the bincode crate, however it might evolve if necessary.

Because the file's content may evolve in the future, this should be taken into consideration from the beginning. I don't remember how bincode allows this, but I think the following method should work: try to load the latest structure, if it fails try the previous ones until it matches and upgrade to the latest.

When?

I would like to limit new features before the async rewrite, however it doesn't prevent discussing this future improvement.

@breard-r breard-r added enhancement New feature or request help wanted Extra attention is needed labels Jan 28, 2023
@jcgruenhage
Copy link
Contributor

I think it makes sense to consider prior art here, to see how other ACME clients tackle this, if at all. I'll start by listing the ones I know of the top of my head:

Regarding the format: bincode itself doesn't handle schema differences at all, and from what I can tell, it might accidentally parse data even if the structs did change, so automatically falling back if it doesn't parse is dangerous.

It would be possible to split this into a wrapper instead, that contains a version number and a byte vec. After parsing the version number, the byte vec could be parsed again, this time through the correct struct. A fancy version of this, which would allow us to skip all the replication of the fields for the different versions is provided by https://lib.rs/crates/obake.

@SinnySupernova
Copy link

Possibly related way of breaking things:

  1. create a cert with LE staging
  2. change endpoint from LE staging to LE production
  3. restart

no new certificate is craeted because a cert with this file name already exists, even if technically invalid at this point

@breard-r breard-r self-assigned this Dec 31, 2024
@breard-r breard-r removed the help wanted Extra attention is needed label Dec 31, 2024
@breard-r breard-r added this to the acmed-ng milestone Dec 31, 2024
@breard-r
Copy link
Owner Author

Over time, I completely changed my mind on how to handle this. Currently, I'm planning to have a RDBMS which holds at least informations related to the accounts and certificates. Using sqlite3 would allow to fix the current issue using the single file model. It would also support changes to the schema and possibility to detect a change in the configuration, which solves the issues already mentioned (by the way, thank you both for your inputs).

@jcgruenhage
Copy link
Contributor

Using a rdbms sounds like a good path forward.

Would you consider also supporting postgresql in addition to (or instead of) sqlite? My experience with sqlite is not great, and while postgres surely also has some complexity, I feel it's easier to run something in a production ready way on top of postgres compared to sqlite.

@breard-r
Copy link
Owner Author

Would you consider also supporting postgresql in addition to (or instead of) sqlite?

Yes. Since I plan to use SQLx that would be quite easy.

My experience with sqlite is not great

I'm surprised, I always had very good experiences and feedbacks from sqlite.

I feel it's easier to run something in a production ready way on top of postgres compared to sqlite.

Quite the opposite. The sqlite storage file would be created on the fly if it doesn't already exists and therefore there is nothing to do, whereas PostgreSQL requires you to deploy and configure the server before starting ACMEd.

@jcgruenhage
Copy link
Contributor

I'm surprised, I always had very good experiences and feedbacks from sqlite.

There's two major data consistency footguns:

The rest is probably personal gripes with how the sqlite project is run.

Quite the opposite

Yes, getting started with sqlite is easier, but that's why I specifically said "production ready".

Both for inspecting an sqlite database and for taking atomic snapshots for backup purposes, you need to stop the service accessing the sqlite database. With postgres, you can do both of these while the service is online. Both are important for production setups IMO, because insight into what is happening and backups for durability are important.

You can build the inspection features into the service, and you can build the snapshot features into the service as well, but both of those are harder than just using a RDBMS that already has those capabilities.

@SinnySupernova
Copy link

I'm a big fan of using something portable like sqlite here. This would work on a really low spec system and allow me to not spin up any extra containers.

I'm not sure acmed would ever need to access the db in a capacity that would realistically benefit from it being postgres over sqlite. Just learned that sqlite has backup api (also usable from cli) so that part should be no issue.

SQLx supports both sqlite and postgres, so sqlite could be default and postgres could be feature gated. While I generally wouldn't use SQLx for postgres (personally more in favor of tokio-postgres and cornucopia), having a common API with simple queries would likely be much easier to maintain.

@jcgruenhage
Copy link
Contributor

@SinnySupernova I think sqlite support does make sense, I'm only saying that I'd prefer having the choice which RDBMS to use as a backend. Having postgres support behind a feature flag sounds good to me.

@breard-r
Copy link
Owner Author

breard-r commented Jan 2, 2025

Types aren't enforced by default, which they think is good: https://www.sqlite.org/flextypegood.html

Yes, that's something I know. But since the DB should be only accessed by ACMEd, that won't be an issue since Rust is strongly typed.

Foreign keys aren't enforced by default, for backwards compat: https://www.sqlite.org/foreignkeys.html#fk_enable

And SQLx enables them by default: https://docs.rs/sqlx/0.8.2/sqlx/sqlite/struct.SqliteConnectOptions.html#method.foreign_keys

You can build the inspection features into the service

I'm actually working on #81, which will drastically limits the need to peek into the database.

This would work on a really low spec system and allow me to not spin up any extra containers.

Yes, that's the spirit. By the way, I also considered DuckDB (which looks really cool), but thought that it's not the best choice for ACMEd since the database will be really small. Also, because DuckDB isn't supported by SQLx, it would make integration of other databases harder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants