ACME V2 Integration #3063

nmengin · 2018-03-22T10:02:34Z

What does this PR do?

This PR allows Træfik to use the new ACME version.
It uses the new lego branch to access to the ACME V2 API.

ACME V2 API main features :

Suppression of TLS-SNI-01 challenge
Adding wildcard certificates generation with DNS-02 challenge

Motivation

Fix #2674

Allowing users to do ACME wildcard certificates

More

Added/updated tests
Added/updated documentation

Additional Notes

For the moment, wildcard domains can only be defined with the ACME domains option.
SANs are not allowed in wildcard certificates.
Another PR with end-to-end tests for wildcard domains will come ASAP.

dtomcej · 2018-03-22T14:22:40Z

acme/acme.go

+			return nil, fmt.Errorf("unable to generate a wildcard certificate for domain %q : ACME needs a DNSChallenge", strings.Join(domains, ","))
+		}
+		if len(domains) > 1 {
+			return nil, fmt.Errorf("unable to generate a wildcard certificate for domain %q : SANs are nor allowed", strings.Join(domains, ","))


SANs are not allowed

emilevauge

Great job @nmengin !
Few comments though ;)

emilevauge · 2018-03-22T13:59:45Z

acme/localStore.go

+
+		// Check if ACME Account is in ACME V1 format
+		if account != nil && account.Registration != nil {
+			isOldRegistration, err := regexp.MatchString(acme.OldRegistrationUrlPath, account.Registration.URI)


s/OldRegistration/ACMEv1Registration may be more explicit

emilevauge · 2018-03-22T14:17:30Z

acme/acme.go

+		return nil, errors.New("unable to generate a certificate when no domain is given")
+	}
+	if strings.HasPrefix(domains[0], "*") {
+		if a.DNSChallenge == nil && len(a.DNSProvider) == 0 {


I think there is an issue in this test

emilevauge · 2018-03-22T14:26:36Z

acme/acme.go

@@ -459,7 +459,7 @@ func (a *ACME) LoadCertificateForDomains(domains []string) {
 	a.jobs.In() <- func() {
 		log.Debugf("LoadCertificateForDomains %v...", domains)

-		domains, err := a.getValidDomain(domains)
+		domains, err := a.getValidDomain(domains, false)


s/getValidDomain/getValidDomains

dtomcej · 2018-03-22T16:05:41Z

Gopkg.lock

@@ -1214,10 +1214,11 @@
  revision = "0c8571ac0ce161a5feb57375a9cdf148c98c0f70"

 [[projects]]
-  branch = "master"
+  branch = "acmev2"


Are there any intentions to fork this for stability? Not really excited to use a feature branch on a third party repo as a basis for prod use...

Hello @dtomcej.

We are not enthusiastic either but, for the moment it's the only way we have if we want to integrate ACME V2.
Moreover, I tested a lot and the branch seems to be stable.

We are following the LEGO repo and, if a big problem appears, we will be able to fork the repo with a stable version thanks to the vendoring.

dtomcej · 2018-03-22T16:07:15Z

acme/acme.go

@@ -395,7 +387,7 @@ func dnsOverrideDelay(delay flaeg.Duration) error {

 func (a *ACME) buildACMEClient(account *Account) (*acme.Client, error) {
 	log.Debug("Building ACME client...")
-	caServer := "https://acme-v01.api.letsencrypt.org/directory"
+	caServer := "https://acme-v02.api.letsencrypt.org/directory"


Is this fully backwards compatible? Is it something that we should be defaulting back to v1 somehow?

@dtomcej It's not backward compatible.

That's why getAccount methods have been modified.
They can detect V1 Account format and delete is from the ACME structure.

Thus, Træfik reads the new configuration and create a new V2 ACME Account.

However, of course, ACME certificates generated in V1 are compatibles

juliens · 2018-03-23T12:24:57Z

docs/user-guide/examples.md

+
+[[acme.domains]]
+  main = "*.local1.com"
+  sans = ["test1.local1.com", "test2.local1.com"]


You can't do that (wildcard with sans) ?

No you can't do it for the moment!

mmatur

Nice work @nmengin 👏
Few comments in the review

mmatur · 2018-03-23T09:12:26Z

acme/acme_test.go

+	for _, test := range tests {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()


Could you add a new line please

mmatur · 2018-03-23T12:45:21Z

docs/configuration/acme.md

+
+### DNS-02 Challenge
+
+As described in [Let's Encrypt post](https://community.letsencrypt.org/t/staging-endpoint-for-acme-v2/49605), wildcard certificates can only be generated thanks to  a `DNS-02`Challenge.


Space missing between DNS-02 and Challenge

mmatur · 2018-03-23T12:51:37Z

acme/localStore.go

 		}
+
+		// Store the data in new format into the file ven if account is nil


even instead of ven

mmatur · 2018-03-23T12:56:03Z

docs/configuration/acme.md

@@ -137,6 +139,10 @@ entryPoint = "https"
    If `HTTP-01` challenge is used, `acme.httpChallenge.entryPoint` has to be defined and reachable by Let's Encrypt through the port 80.
    These are Let's Encrypt limitations as described on the [community forum](https://community.letsencrypt.org/t/support-for-ports-other-than-80-and-443/3419/72).

+!!!note
+    Wildcard certificates can be generated only if `acme.dnsChallenge`
+option is enable.


Why a new line for this part ? The note in the documentation is broken

It's another note but a space is missing I add it.

mmatur · 2018-03-23T13:02:28Z

docs/user-guide/examples.md

+[acme]
+email = "[email protected]"
+storage = "acme.json"
+caServer = "http://172.18.0.1:4000/directory"


You should maybe use "https://acme-staging-v02.api.letsencrypt.org/directory instead of http://172.18.0.1:4000/directory

mmatur · 2018-03-23T13:03:23Z

docs/user-guide/examples.md

+
+[[acme.domains]]
+  main = "*.local1.com"
+  sans = ["test1.local1.com", "test2.local1.com"]


You can not do that. If main domain is a wildcard you can not use sans

mmatur · 2018-03-23T13:10:20Z

provider/acme/provider_test.go

+	for _, test := range tests {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+


Could you please remove this line

…ACME Account version

mmatur · 2018-03-23T19:23:52Z

acme/acme.go

+// - Duplicated domains
+// - Domains which are checked by wildcard domain
+func (a *ACME) deleteUnecessariesDomains() {
+	var newDomains []types.Domain


deleteUnnecessariesDomains instead of deleteUnecessariesDomains

mmatur

LGTM

Great job @nmengin 👏

emilevauge

Great job @nmengin !
Very much awaited feature ;)
LGTM

sarkistlt · 2018-03-26T16:24:42Z

thanks! literally much awaited feature in my project!
I just have one question, trying to defined it like so:

[[acme.domains]]
  main = "mydomain.com"
  sans = ["*.mydomain.com"]

and getting error Error getting ACME certificate for domain [mydomain.com *.mydomain.com]: cannot obtain certificates map[*.mydomain.com:acme: Error 400 - urn:acme:error:malformed - Error creating new authz :: Wildcard names not supported],

so who then should I define wildcard?

nmengin · 2018-03-27T08:29:34Z

Hello @sarkistlt ,

Many thanks for your feedback.

For the moment, we do not support wildcard certificates with/in SANs.
Moreover, the error seems to come from ACME library.

You can use the configuration described below :

[[acme.domains]]
  main = "mydomain.com"
[[acme.domains]]
  main = ["*.mydomain.com"]

If you still have problems, I suggest you to open an issue or to join our Slack workspace for more community #support.

sarkistlt · 2018-03-27T16:08:35Z

thanks for response, I just downloaded source code of traefik and it still uses https://acme-v01.api.letsencrypt.org/directory, instead of https://acme-v02.api.letsencrypt.org/directory, I tried to use _ACME_SERVER_HOST: acme-staging-v02.api.letsencrypt.org but it ignores acme env. vars, so which tag of docker image does support acme2? I tried 1.5 and latest, no luck

sarkistlt · 2018-03-27T16:20:08Z

actually just saw version 1.6 deployed on docker hub 10hr ago and it works, thanks!

sarkistlt · 2018-03-27T16:40:54Z

getting another error unable to generate a wildcard certificate in ACME provider for domain \"*. mydomain.com\" : ACME needs a DNSChallenge, looks like it doesn't generate .well-known/acme-challenge
And for the main domain: Unable to obtain ACME certificate for domains \"mydomain.com\" : cannot obtain certificates map[mydomain.com:acme: Error 400 - urn:ietf:params:acme:error:connection - Fetching http://mydomain.com/.well-known/acme-challenge/some_hash: Timeout], should I open new issue?

e-nikolov · 2018-03-27T17:47:29Z

You can find info on how to configure the wildcard certs here: https://docs.traefik.io/v1.6/configuration/acme/.

Let's Encrypt uses a DNS challenge for issuing wildcard certificates, which requires setting a DNS TXT record. Traefik can automate this via the API of your DNS provider (if it is supported), for which it requires an API key.

ldez · 2018-03-27T18:15:08Z

⚠️ For all those who have problems with this feature, please open a dedicated issue. ⚠️

You can also join our Slack workspace for more community #support.

Remember, we dedicate the issue tracker to bug reports and feature requests only

iautom8things · 2018-04-12T18:22:03Z

docs/configuration/acme.md

+[acme]
+# ...
+[[acme.domains]]
+  main = "*local1.com"


Is this a typo?

https://docs.traefik.io/v1.6/configuration/acme/#wildcard-certificates

traefiker added area/acme area/provider/docker size/L status/0-needs-triage labels Mar 22, 2018

nmengin added status/2-needs-review and removed status/0-needs-triage labels Mar 22, 2018

nmengin added this to the 1.6 milestone Mar 22, 2018

nmengin added the bot/no-merge label Mar 22, 2018

ldez added kind/enhancement a new or improved feature. and removed area/provider/docker labels Mar 22, 2018

nmengin force-pushed the feature/acmev2-study branch from 5c4092f to 7796017 Compare March 22, 2018 10:57

dtomcej reviewed Mar 22, 2018

View reviewed changes

emilevauge requested changes Mar 22, 2018

View reviewed changes

traefiker added the contributor/waiting-for-corrections label Mar 22, 2018

dtomcej suggested changes Mar 22, 2018

View reviewed changes

nmengin force-pushed the feature/acmev2-study branch 3 times, most recently from 65e605e to 89478ab Compare March 22, 2018 17:38

nmengin removed the contributor/waiting-for-corrections label Mar 22, 2018

dtomcej approved these changes Mar 22, 2018

View reviewed changes

nmengin removed the bot/no-merge label Mar 23, 2018

juliens reviewed Mar 23, 2018

View reviewed changes

mmatur requested changes Mar 23, 2018

View reviewed changes

traefiker added the contributor/waiting-for-corrections label Mar 23, 2018

nmengin added 6 commits March 23, 2018 18:14

Modify dependencies

57fbe8c

Modify imports

aac7a04

Vendoring

240eb12

Check if ACME account is in V1 or V2 version

3fedf29

Modify migration between old and new ACME storage format to consider …

90922cd

…ACME Account version

Migrate ACME examples to the new containous boulder image

a1a9fac

nmengin added 5 commits March 23, 2018 18:14

Update Unit Tests and End-To-End Tests to use ACME V2

ad7281f

Check if wildcard certificate might be generated

b622aa8

Fix gofmt

02d5778

Add documentation about ACME V2

22e67a4

Improve domain checking before to launch ACME process

ae0fef7

nmengin force-pushed the feature/acmev2-study branch from fae09e6 to 8dbfd5a Compare March 23, 2018 17:15

nmengin removed the contributor/waiting-for-corrections label Mar 23, 2018

mmatur requested changes Mar 23, 2018

View reviewed changes

traefiker added the contributor/waiting-for-corrections label Mar 23, 2018

Fix for reviews

fb1417e

nmengin force-pushed the feature/acmev2-study branch from 8dbfd5a to fb1417e Compare March 24, 2018 13:15

nmengin removed the contributor/waiting-for-corrections label Mar 24, 2018

refactor: review.

55c17d8

mmatur approved these changes Mar 26, 2018

View reviewed changes

emilevauge approved these changes Mar 26, 2018

View reviewed changes

mmatur added the bot/no-merge label Mar 26, 2018

nmengin added status/3-needs-merge and removed bot/no-merge labels Mar 26, 2018

traefiker merged commit 16bb9b6 into traefik:master Mar 26, 2018

traefiker removed the status/3-needs-merge label Mar 26, 2018

nmengin deleted the feature/acmev2-study branch March 26, 2018 13:32

iautom8things reviewed Apr 12, 2018

View reviewed changes

ldez removed the status/2-needs-review label Nov 18, 2021


		### DNS-02 Challenge

		As described in [Let's Encrypt post](https://community.letsencrypt.org/t/staging-endpoint-for-acme-v2/49605), wildcard certificates can only be generated thanks to a `DNS-02`Challenge.

		}

		// Store the data in new format into the file ven if account is nil

ACME V2 Integration #3063

ACME V2 Integration #3063

Conversation

nmengin commented Mar 22, 2018 • edited Loading

What does this PR do?

Motivation

More

Additional Notes

Choose a reason for hiding this comment

emilevauge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmatur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmatur left a comment • edited Loading

Choose a reason for hiding this comment

emilevauge left a comment

Choose a reason for hiding this comment

sarkistlt commented Mar 26, 2018

nmengin commented Mar 27, 2018

sarkistlt commented Mar 27, 2018 • edited Loading

sarkistlt commented Mar 27, 2018

sarkistlt commented Mar 27, 2018

e-nikolov commented Mar 27, 2018

ldez commented Mar 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmengin commented Mar 22, 2018 •

edited

Loading

mmatur left a comment •

edited

Loading

sarkistlt commented Mar 27, 2018 •

edited

Loading