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

Implement loopback #1

Open
insightfuls opened this issue Oct 12, 2016 · 9 comments
Open

Implement loopback #1

insightfuls opened this issue Oct 12, 2016 · 9 comments

Comments

@insightfuls
Copy link
Owner

See https://github.com/Daplie/le-challenge-webroot/issues/2

@creationix
Copy link

Head up, I added a placeholder for now in our copy https://git.daplie.com/Daplie/le-challenge-sni/commit/b09566aa22bae5ac575a49cad2a1c803a98d06ed

I don't yet know enough to implement it myself.

Are you able to add this? If not, do you mind adding me (creationix on npm) as a npm owner and I'll publish one when I get it added.

@creationix
Copy link

Also I read your comments in the webroot issue. Generally I agree with you about the cost to benefit ratio. If it turns out it's just too hard to make any meaningful check I'm cool with that.

@insightfuls
Copy link
Owner Author

I should be able to do this, and then implement it properly later. I just haven't got around to it yet, sorry.

@creationix
Copy link

I'm also trying to get it done, I got this far:

  , loopback: function (args, domain, token, cb) {
      var challengeDomain = (defaults.test || '') + defaults.acmeChallengeDns + domain;
      // resolving the domain name to an ip address manually
      dns.resolveAAsync(domain).then(function (ip) {
        // making a tls connection to the server by the ip address
        // setting the sni to the acme.invalid
        tls.connect({
          host: ip,
          port: 443,
          servername: challengeDomain,
          checkServerIdentity: function (servername, cert) {
            // checking the certificate meta data
            console.log("TODO: check me", servername, cert)
            done();
          }
        }, function (err) {
          if (err) return done(err);
        });
      }, done);
    }

  , test: function (opts, domain, token, keyAuthorization, cb) {
      var me = this;
      args.test = args.test || '_test.';
      defaults.test = args.test;
      me.set(args, domain, challenge, keyAuthorization || challenge, function (err, k) {
        if (err) { done(err); return; }
        me.loopback(defaults, domain, challenge, function (err, arr) {
          if (err) { done(err); return; }
          if (!arr.some(function (a) {
            return a.some(function (keyAuthDigest) {
              return keyAuthDigest === k;
            });
          })) {
            err = new Error("sni record '" + challenge + "' doesn't match '" + k + "'");
          }
          me.remove(defaults, domain, challenge, function (_err) {
            if (_err) { done(_err); return; }
            // TODO needs to use native-dns so that specific nameservers can be used
            // (otherwise the cache will still have the old answer)
            done(err || null);
            /*
            me.loopback(defaults, domain, challenge, function (err) {
              if (err) { done(err); return; }
              done();
            });
            */
          });
        });
      });
    }

But I'm new to this codebase and am still learning the ropes.

@creationix
Copy link

Ok, just as a heads up. I was able to implement a basic loopback and test that at least worked for the happy path. Good news for you, we decided that this code better belongs in greenlock itself with per-challenge-type logic. This means you can close this issue!

https://git.daplie.com/Daplie/node-greenlock/issues/84

@creationix
Copy link

@insightfuls
Copy link
Owner Author

Great! I think that's heaps cleaner architecture.

@insightfuls
Copy link
Owner Author

The link to the merge request is broken, though. :-)

@creationix
Copy link

You should be able to see the merge request now. I had made it semi-private so people wouldn't get confused and think it was the upstream version.

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

No branches or pull requests

2 participants