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

Use relative name instead #1

Closed
kurokobo opened this issue Mar 11, 2021 · 3 comments · Fixed by #2
Closed

Use relative name instead #1

kurokobo opened this issue Mar 11, 2021 · 3 comments · Fixed by #2

Comments

@kurokobo
Copy link
Collaborator

libdns/libdns#12

@kurokobo
Copy link
Collaborator Author

It appears to be a safe change while maintaining backward compatibility for Caddy.
In my understanding, about for the main use case, Caddy:

  • Caddy leaves certificate issuance to Certmagic.
  • Certmagic will now ask libdns to process with relative name records (so far FQDN)
  • Certmagic executes DeleteRecords after AppendRecords, but since Certmagic only passes the return value of AppendRecords to DeleteRecords as it is, libdns has no problem as long as the record name received and the record name to be returned match.
  • As for the libdns side, backward compatibility may be maintained if the process does not fail in either case of receiving a record name as an FQDN or a relative name.
  • Seems libdns.RelativeName can handle this.
    • libdns.RelativeName("hoge.example.com.", "example.com.") -> "hoge"
    • libdns.RelativeName("hoge", "example.com") -> "hoge"

@kurokobo
Copy link
Collaborator Author

kurokobo commented Mar 12, 2021

Apparently, my understanding was wrong. Backward compatibility may not be maintained.

The old Certmagic (with Caddy 2.3.0) request the record looks like this:

  • Name: _acme-challenge.test.example.com
  • Zone: example.com.

The zone is FQDN, but the name is not FQDN (no trailing dot). Because of this, once I simply pass those values to libdns.RelativeName, it returns _acme-challenge.test.example.com as a relative name. So if I implement libdns to treat record names as relative names, _acme-challenge.test.example.com.example.com will be created. This is unexpected behavior.

The new Certmagic (with latest upstream Caddy) request looks like this:

  • Name: _acme-challenge.test
  • Zone: example.com.

If the libdns implementation treats the record name as a relative name, it works as expected.

It is difficult to satisfy both record with the same code. When the record name is test.example.com, it is not possible to determine whether the record you actually want to manipulate is test.example.com. or test.example.com.example.com..

I have to choice dropping backword compatibility or dropping the feature to handle the record that have trailing doubled domain name. But I think follow caddy-dns/cloudflare is good way, so I'm considerring to drop backword compatibility.

@kurokobo
Copy link
Collaborator Author

I think I understood it wrong again 🤔

It seems that Cloudflare absorbs minor differences in the request payload on the API side. On cloudflare, The following three request payloads all yielded the same result.

  • {"type": "TXT", "name": "test", "content": "request with only relative name", "ttl":120, "priority":10, "proxied":false}
  • {"type": "TXT", "name": "test.example.com", "content": "request with fqdn without dot", "ttl":120, "priority":10, "proxied":false}
  • {"type": "TXT", "name": "test.example.com.", "content": "request with truly fqdn", "ttl":120, "priority":10, "proxied":false}

This means that backward compatibility about new libdns is preserved for Cloudflare provider, while Azure's API interprets requests as strictly relative names, which is different from Cloudflare.

...and this is a little off-topic, however, I think this may also be the cause of the problem that the latest certmagic can't handle www.example.com.example.com well with Cloudflare.

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

Successfully merging a pull request may close this issue.

1 participant