Creating Present/Cleanup methods providing just fqdn, value as arguments ?

[mdbraber]

Hi,

I'm writing acme-proxy to relay dns challenges using the httpreq provider and integrating with lego DNS providers. It's easy to write a "RAW" method that calls the Present() and Cleanup() methods using domain, token and keyAuth arguments.

I'd also like to write a version that integrates the "default" method that calls a method using just the fqdn and value. But there isn't a constructor that I can call for lego DNS providers with just those two values (and logically it's impossible to reverse value to a keyAuth as it's sha256.

I'm wondering if anyone has a good suggestion to leverage the existing DNS providers. A complete "solution" would of course make a separate constructor with the fqdn and value parameter that is called by the 'outer' constructor that uses the domain, token and keyAuth parameters. That might solve the issue but probably this is considered overkill. OTOH having a constructor that can be called with just fqdn and value for DNS challenges might be useful for other uses cases too.

I'm happy to rewrite the DNS providers if this would be interesting, but first would like to see if this would be the best way to go or other solutions might be better (or just not having a "default" mode for acme-proxy might be just best)

/cc @ldez

[mdbraber]

@ldez any thoughts on this? I'm trying to figure out if there's a way to use lego if I only have the FQDN and Value (TXT record) parameters?

[ldez]

I think this is a edge-case and that there are some design issues.
For example, the Present / Cleanup methods are defined by the Provider interface.

I think that the RAW mode allow to meet acme-proxy needs.

[mdbraber]

@ldez thanks for your reply! The RAW mode is indeed great for outgoing requests to (in this case) acmeproxy using httpreq that are passed on to the lego Present / CleanUp methods.

My question is how to make use of lego as a backend when for handling incoming requests which are issued through acmeproxy. As an example: when I want to use acmeproxy with acme.sh the following takes place:

1. solve a challenge with acme.sh using a new httpreq provider
2. send fqdn, value to acmeproxy
3. acmeproxy receives fqdn, value
4. acmeproxy creates TXT record based on fqdn, value using lego as backend

Step 1-3 work great, step 4 is not possible yet with lego because it only accepts unsolved challenges with domain, keyauth. But all the relevant logic for creating TXT records is there.

As lego is great at providing interfaces to all possible DNS providers, it's the best (and only choice as far as I can see) that can solve ACME challenges using Go. The problem is that presented challenges are already solved (and can't be unsolved) and so there's no way to use the DNS providers from lego, even though all the relevant logic is there.

As I see (and I'm sure I might be mising something), the logic for most (all?) DNS providers is exactly like is described in the wiki entry Writing a Challenge Solver:

func (d *DNSProviderBestDNS) Present(domain, token, keyAuth string) error {
    fqdn, value := dns01.GetRecord(domain, keyAuth)
    // make API request to set a TXT record on fqdn with value and ttl
    return nil
}

As far as I can see all the DNS providers use the same logic:

1. get fqdn, value using dns01.GetRecord(domain, keyAuth).
2. make API request to set a TXT record on fqdn with value and ttl

It could be easy to make the //make API request to set a TXT record on fqdn with value and ttl-logic a separate constructor (e.g. PresentSolved(fqdn, value), that possibly could be called directly, as well as from Present(domain, token, keyAuth). That would make lego much more widely usable as backend for other implementations (without the need to rewrite all the DNS logic of setting DNS challenge TXT records for each provider already included).

I understand that it's definitely a design decision. I'm wondering if you'd be willing to consider this because it would make lego more valuable as package to be integrated in other projects.

If not I'd be at least curious if you would have any suggestions on what would be a good (clean) approach to solve this using Go.

[mdbraber]

@ldez practical what I'd suggest would be the following:

Update the interfaces in challenge/provider.go to look like this:

type Provider interface {
	Present(domain, token, keyAuth string) error
	CleanUp(domain, token, keyAuth string) error
}

type DNSProvider interface {
	Provider
	CreateRecord(fqdn, value string) error
	RemoveRecord(fqdn, value string) error
}

As DNSProvider also implements Provider it should be usable wherever a Provider inteface is required.

Next to creating this interface it would be necessary to update func NewDNSChallengeProvider(name string) (challenge.Provider, error) in providers/dns/dns_providers.go to return a challenge.DNSProvider instead.

This would mean that the providers in providers/dns need to be updated to implement a CreateRecord and RemoveRecord method. This would make the already existing functionality in lego much wider applicable, without giving up on flexibility.

I'd be happy to submit a PR for that if you'd agree this would be a good way to offer this functionality.

[ldez]

Those interfaces must not be changed because they are used in a lot of projects.

[mdbraber]

@ldez the Provider interface would stay the same. it would only add an extra interface DNSProvider with additional functionality (or it could be called ProviderSolved if it would conflict with DNSProvider). Or am I missing that this would screw up many other things? I'm trying to see if this can be done with minimal impact and understand that it would need some good designing. But it would be unfortunate if it would need a fork of lego to replicate already existing functionality of creating/removing DNS records for ACME validation.

[ldez]

The new interface will be introduce breaking changes on exported methods.
And I'm not agree with your design proposal.

I don't see any way to do what you want without break the public contract of the lego API.

could you explain why you use acme.sh instead of Lego, Traefik or Caddy?

[mdbraber]

@ldez first, thanks for taking the time to give feedback - I'm trying to figure this out, but have much less experience in using good design patterns than most of you, so happy to understand more of how this would break exported methods. I've tried understanding from reading https://golang.org/ref/spec#Interface_types how this works, but can't see how a new interface breaks exported methods.

Obviously I don't want to break the public contract of the lego API, which is why introducing a new interface would offer an extra option (e.g. a ProviderSolved interface), rather than replacing an existing one.

The design goal for acmeproxy is to be universally useable, independent of the client. Personally I'm using acme.sh because it's part of the (FreeBSD) pfSense router I'm using. For all other parts of my network I'm using Lego and Caddy. Of course I could try and change this, but there would be many more circumstances that might want to make use of fqdn, value (just as why httpreq offers both a default and RAW mode.

[mdbraber]

Could this be a workable idea without breaking anything?

• func NewDNSChallengeProvider(name string) (challenge.Provider, error) in providers/dns/dns_providers.go stays unchanged and returns a challenge.Provider
• A new interface ProviderSolved (with extra methods) is added, but it's left to each provider if it wants to implement the relevant methods to satisfy this interface

type Provider interface {
	Present(domain, token, keyAuth string) error
	CleanUp(domain, token, keyAuth string) error
}

type ProviderSolved interface {
	Provider
	CreateRecord(fqdn, value string) error
	RemoveRecord(fqdn, value string) error
}

We can then try to cast a Provider interface into a ProviderSolved interface where needed, like such:

p, err := dns.NewDNSChallengeProviderByName(config.Provider)
if err != nil {
    panic(err)
}

provider, ok := p.(ProviderSolved)
if ok {
    // this provider implements ProviderSolved
} else {
    // this is a regular Provider
}

/EDIT: we could even do without creating the ProviderSolved interface separately and just check in place like such:

if provider, ok := p.(interface{CreateRecord(),RemoveRecord()}); ok { 
  // this provider offers CreateRecord and RemoveRecord methods
} 

This would mean that we could rewrite DNS providers that want to offer an export CreateRecord / RemoveRecord functionality without changing much else.

[ldez]

I need time to think about the best solution.

[mdbraber]

Going through the different options it would be enough if there would be an exported method that accepts fqdn, value. Thanks for considering this @ldez