Add ACME support by jordanruthe · Pull Request #165 · 7ritn/VaulTLS

jordanruthe · 2026-04-12T22:50:02Z

Add ACME support. Currently it supports the HTTP-01 challenge. I intend to also add the DNS-01 challenge before making this ready to merge. I wanted to open a PR so it's available to review the general methodology, but need some more time to continue working the DNS-01 and do some touching up.

This feature must be manually enabled, either through the settings page or with an ENV variable. Admins can choose to be notified on a new ACME certificate issuance. I've tried to re-use the issuance code where I can, but the ACME protocal leverages CSRs so the private key never leaves the server requesting the certificate and that required some flow changes.

Each ACME account has to be manually created, which will create an External Account Binding (EAB) for different clients. Each client can be limited to specific domains, to include a wildcard. Wildcards do not allow another level (i.e. no dots).

For transparency, I have leveraged Claude to speed up development of this PR in terms of the README, testing, and ensuring that this PR's logic is sound and does not implement any vulnerabilities.

7ritn · 2026-04-13T19:49:20Z

Thanks for the PR. I will try to get the testing fixed when I find time. When you are ready for me to take a look just mark the PR as ready ^^

jordanruthe · 2026-04-15T22:19:05Z

I modified the workflow to not push in a PR. That's what kept failing for the test. I think we're good to go here, let me know if you have any questions or need anything changed. I've tested this with traefik and acme.sh, both http and dns challenges are working. Since an account has to be made by an admin, use the right credentials to bind, and it's limited to specific domain names, there is an option to bypass the challenges.

I found in testing, and likely in half a year with production, tables would get to long so I've also added pagination and a toggle to hide acme certs. Personally I have 40 certs that would renew quarterly.

7ritn · 2026-04-17T21:03:15Z

Thanks I will take a look when I get time. Could you split out the changes to the Github Workflow into a new PR so I can already merge that?

7ritn · 2026-04-26T14:51:32Z

Could you rebase this PR please?

7ritn

I didn't have time yet to look into the main ACME changes, since I will need quite a lot of time due to lack of experience. Please move pagination changes into a seperate PR. Thanks for the PR.

7ritn · 2026-04-26T15:06:25Z

+/// Applies the standard end-entity X509 extensions and signs the certificate.
+/// Used by both `build_common` (PKCS#12 output) and `build_server_pem` (PEM output).
+fn apply_end_entity_extensions(x509: &mut X509Builder, ca_cert: &X509, ca_key: &PKey<Private>) -> Result<()> {
+    let basic_constraints = BasicConstraints::new().build()?;
+    x509.append_extension(basic_constraints)?;
+
+    let key_usage = KeyUsage::new()
+        .digital_signature()
+        .key_encipherment()
+        .build()?;
+    x509.append_extension(key_usage)?;
+
+    x509.set_issuer_name(ca_cert.subject_name())?;
+
+    let subject_key_identifier = SubjectKeyIdentifier::new()
+        .build(&x509.x509v3_context(None, None))?;
+    x509.append_extension(subject_key_identifier)?;
+
+    let authority_key_identifier = AuthorityKeyIdentifier::new()
+        .keyid(true)
+        .build(&x509.x509v3_context(Some(ca_cert), None))?;
+    x509.append_extension(authority_key_identifier)?;
+
+    x509.sign(ca_key, MessageDigest::sha256())?;
+    Ok(())
+}


While sensible, please do not include refactoring changes that are not necessary / move them to a new PR. Refactoring changes should be in a separate commit and clearly marked as not changing function.

I'm happy to move refactoring to separate commits in this PR. Or would you prefer I keep the certificate issuance logic separated for ACME? I was trying to re-use as much code as possible to make it easier for auditing.

7ritn · 2026-04-26T15:51:26Z

+    )
+
+
+def run_acme_flow(


I would appreciate more comments in this function since I am not super experienced with ACME

I've added more comments

7ritn

Okay I finally managed to perform a full review, just took me a few hours. Anyway the implementation in general is looking very good and I (and I am sure also a few users) am excited to get this merged. Thanks for the work you put in.

7ritn · 2026-05-06T12:13:56Z

+    let new_status: &str = if account.auto_validate {
+        "valid"
+    } else if challenge_type == "dns-01" {


Can we add some logging here? Since auto_validate can be quite popular adding a warning that we did not vaildate would be good. In general having a bit more debug logging would help us if a user reports an issue.

I've added a warn log here. I haven't had a chance to think through where other log statements would be helpful yet

7ritn · 2026-05-06T12:53:46Z

+            Ok(())
+        }
+
+        "RS256" => {


I am not a super big fan of supporting RSA based cryptography, but I think for compatibility it makes sense.

7ritn · 2026-05-06T13:10:42Z

+    let (header, _protected_bytes, payload_bytes, signature) = parse_jws(&body.0)?;
+
+    let nonce = header.nonce.as_deref().unwrap_or("");
+    let valid = state.db.validate_and_delete_nonce(nonce.to_string()).await


I would have probably used an in-memory cache, but since we don't really care about that extra bit of performance I think it is fine to leave as is.

7ritn · 2026-05-06T14:02:53Z

+pub struct CreateAcmeAccountRequest {
+    pub name: String,
+    pub allowed_domains: Vec<String>,
+    pub ca_id: Option<i64>,


What is your reason for making this optional? I think we should be explicit here.

I was torn here and happy to hear your input. I realize that the default CA timespan is 10 years and decently long, but I was trying to think through a renewal strategy. With the assumption that we're not trying to create multiple domains with one VaulTLS instance... Lets say a CA timespan is 1 year. If I have test.io for 2 years, I can create a new CA cert slightly before that and the acme account will migrate to the latest test.io certificate without manual intervention. If using multiple domains, this could be an issue unless you specify the ID.

jordanruthe · 2026-05-06T23:21:03Z

Thanks for the feedback, I'm definitely excited for this capability to get merged as well. I'm working through the comments and will get a new commit pushed soon.

… require save before settings change in sidebar.

…catchers for rocket to convey error information. Add tests for the guard

jordanruthe marked this pull request as draft April 12, 2026 22:50

jordanruthe force-pushed the feat-acme branch from d540efa to 08b87b7 Compare April 12, 2026 23:03

7ritn linked an issue Apr 13, 2026 that may be closed by this pull request

ACME server support #160

Open

jordanruthe force-pushed the feat-acme branch 3 times, most recently from 9c2190e to 2734a3e Compare April 15, 2026 00:26

jordanruthe marked this pull request as ready for review April 15, 2026 22:10

7ritn added this to the v1.2.0 milestone Apr 17, 2026

7ritn added the enhancement New feature or request label Apr 26, 2026

7ritn assigned jordanruthe Apr 26, 2026

jordanruthe added 5 commits April 26, 2026 10:58

Add acme feature with http-01. 7ritn#160

fa4d3c7

Add DNS-01 challenge type

bac7ae6

Add pagination with page size

40c4c91

Update ACME to not delete accounts. Hide deactivated accounts

436c48e

Add tests for ACME. Fix test.yml for PRs

fe4fae0

jordanruthe force-pushed the feat-acme branch from 4e857c1 to fe4fae0 Compare April 26, 2026 15:00

7ritn requested changes Apr 26, 2026

View reviewed changes

7ritn requested changes May 6, 2026

View reviewed changes

jordanruthe added 7 commits May 7, 2026 19:21

Change test files

cf78211

Change settings for ACME to have it's own section and fix frontend to…

1d27666

… require save before settings change in sidebar.

Use URL_SAFE_NO_PAD, add constants,

64da6cd

Move rocket guard to guard.rs, add guard for jws authentication. Add …

797fd9f

…catchers for rocket to convey error information. Add tests for the guard

Remove TLSCertificateBuilder and refactor how acme certs are built.

4b17f0e

Strip trailing slash from URL to prevent double slash

53d0ff1

Add rate limiting for ACME and make it configurable

c669c1b

jordanruthe added 3 commits May 9, 2026 12:52

Remove account status from edit window

009d6d9

Revert "Add pagination with page size". This reverts commit 40c4c91

5323880

Enum cert data for different certificate types

e472bb4

		)


		def run_acme_flow(

Conversation

jordanruthe commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7ritn commented Apr 13, 2026

Uh oh!

jordanruthe commented Apr 15, 2026

Uh oh!

7ritn commented Apr 17, 2026

Uh oh!

7ritn commented Apr 26, 2026

Uh oh!

7ritn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

7ritn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jordanruthe commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jordanruthe commented Apr 12, 2026 •

edited

Loading