gmpassos/shelf_letsencrypt

doAcmeChallenge ignores alternate port no.

Closed this issue · 19 comments

For my dev environment I start the server on port 8080.

The call to doACMEChallenge is failing is it builds a URL without a port number with the result that it tries to connect on port 80 and fails.

This seems to be happening in acme_client getHttpDcvData

  HttpDcvData getHttpDcvData() {
    var keyAuthorization = getKeyAuthorizationForChallenge(VALIDATION_HTTP);
    var token = keyAuthorization!.split('.').elementAt(0);
    return HttpDcvData(
      '${identifier!.value}/.well-known/acme-challenge/$token',
      keyAuthorization,
      getChallengeByType(VALIDATION_HTTP),
    );
  }

the identifier.value has the domain name but no port no.

We see the following message logged before the call is made (and fails)

Self test challenge... {type: HTTP, fileName: squarephone.biz/.well-known/acme-challenge/ksPAAcFzl5CSUQNXwymuoLcZ0NEpsGTHK00Y5AFQXRs, fileContent: ksPAAcFzl5CSUQNXwymuoLcZ0NEpsGTHK00Y5AFQXRs.wZQeeUT1GoiIsb-N7YRgyTccyL7GBnF_QfeYc6w2rVc, challenge: Instance of 'Challenge'

So I think the problem is here:

Future<bool> _selfChallengeTest(
      AcmeClient client, HttpDcvData challengeData) async {
    var url = challengeData.fileName;
    if (!url.startsWith('http:') && !url.startsWith('https:')) {
      final idx = url.indexOf(':/');
      if (idx >= 0) {
        final schema = url.substring(0, idx);
        var rest = url.substring(idx);
        rest = rest.replaceFirst(RegExp('^/+'), '');
        url = '$schema://${rest.replaceAll('//', '/')}';
      } else {
        final rest = url.replaceFirst(RegExp('^/+'), '');
        url = 'http://${rest.replaceAll('//', '/')}';
      }
    }

We are pre-pending HTTP:// but we ignore the need to add in the port.

I will see if I can fix it.

I have a fix for it, it does mean passing the httpPort around quite a bit.

The issue is only on shelf_letsencrypt?

Nice 👍

Since your PR includes a lot of improvements, I will take a few days to review all of the code and make adjustments on my machine.

Additionally, considering your significant contribution, please include your name and GitHub page in the "Author" section.

...Also, I will need to test it in the staging environment of some internal projects that are using shelf_letsencrypt.

So this looks to be a bigger issue.
Anywhere we do a call back to the local server the code is broken because it ignores the current port.

Worry the current api the porr isn't known until the users starts the server but we have a number methods that are valid independently of starting the server.

It kids to be like we need to move the port and bind address to the letsencrpt constructor.

This would be a breaking change but I think it's the right way to do it.

Thoughts?

The point is that LetsEncrypt should be able to handle multiple domains in the same address:port.

The custom port is only needed for the HTTP challenge?
If it's true, it knows the listening port of the HTTP server that it started. (should be inserted in the HTTP URL only if it's different than 80).

The point is that LetsEncrypt should be able to handle multiple domains in the same address:port.
I don't believe that I've made any changes to inhibit this. Have you raised this due to a concern or problem?

The custom port is only needed for the HTTP challenge?

I had to make changes in two spots:

  • _selfChallengeTest
  • isDomainHttpsOk
    So the custom port details were required for both HTTP and https.

should be inserted in the HTTP URL only if it's different than 80).
I've been a little lazy by always inserting the port. It's essentially invisible to the user and valid so I don't really see the issue. If you want I can make it conditional?

Make the changes. I don't think they will be backward incompatible.

Do you want to make any new adjustment or wait some dependency fix/patch?

Fixed via:
#6