ffMathy / FluffySpoon.AspNet.EncryptWeMust

MIT License
146 stars 29 forks source link

Failure to order certificate for multiple domains #216

Closed PreferLinux closed 3 years ago

PreferLinux commented 3 years ago

https://github.com/ffMathy/FluffySpoon.AspNet.EncryptWeMust/blob/ab67a89d24f0c3518bde2ddc2345fb9ab5ff2361/src/FluffySpoon.AspNet.EncryptWeMust/Certes/LetsEncryptClient.cs#L109

var anyValid = challenges.Any(x => x.Status == ChallengeStatus.Valid);
var allInvalid = challenges.All(x => x.Status == ChallengeStatus.Invalid);

if (anyValid || allInvalid)
    break;

Surely the All / Any is around the wrong way here? Shouldn't it be when all challenges are valid, or any are invalid? On line 41 we have var challengeContexts = await Task.WhenAll(allAuthorizations.Select(x => x.Http()));, and looking at Certes this is taking the first http-01 challenge for the authorization, and therefore each challenge represents an authorization. All authorizations need to be valid for the order to be valid. Therefore, this can exit before the order is valid.

I'm pretty sure this will be the cause of an exception I've been getting (my cert has four alternate names):

Certes.AcmeRequestException: Fail to load resource from 'https://acme-v02.api.letsencrypt.org/acme/finalize/...
urn:ietf:params:acme:error:orderNotReady: Order's status ("pending") is not acceptable for finalization

After the exception happens I still get more challenge verification requests coming in, so obviously Let's Encrypt hasn't finished validating the order, also indicated as the order status being pending. If I retry issuing certificates (i.e. restart the server) a couple of times it manages to get the cert.

I haven't actually tested the suggested change. I also haven't looked at your unit tests in detail, but the client tests seem to only test with one domain – might be a good idea to add a test with a few.

ffMathy commented 3 years ago

That makes a lot of sense. Do you have time for a PR?

PreferLinux commented 3 years ago

It'll probably take me a few days to get around to it, but that should be no problem.