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

feat: support dns-01 challenge #114

Merged
merged 23 commits into from
May 22, 2024
Merged

Conversation

yuweizzz
Copy link
Contributor

@yuweizzz yuweizzz commented May 7, 2024

support dns-01 challenge.

Copy link
Owner

@fffonion fffonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR! I will do a detailed review shortly.

lib/resty/acme/autossl.lua Outdated Show resolved Hide resolved
lib/resty/acme/autossl.lua Outdated Show resolved Hide resolved
end
local trim_domain = domain:gsub("*.", "")
local txt_record = calculate_txt_record(response)
local result, err = dnsapi:post_txt_record("_acme-challenge." .. trim_domain, txt_record)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to wait the dns record to propogate after we set the record. use openresty/lua-resty-dns to query the record would be good to avoid the acme server try to validate it prematurely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can handle it in challenge_start_callback? I will check it.

lib/resty/acme/dnsapi/dynv6.lua Outdated Show resolved Hide resolved
@yuweizzz
Copy link
Contributor Author

we still need to return a wildcard domain in domain_whitelist/domain_whitelist_callback if we quire a wildcard domain cert, do you think the new way is ok?

@fffonion
Copy link
Owner

fffonion commented May 15, 2024

we still need to return a wildcard domain in domain_whitelist/domain_whitelist_callback if we quire a wildcard domain cert, do you think the new way is ok?

I think you missed part of the code in commit, but I get your idea, it works for me. Basically we can do:

local WILDCARD_MATCHED = {}
function is_domain_whitelisted(domain)
  if whitelist[domain] then
    return domain
  else if regex match then
    return WILDCARD_MATCHED 
  end
  return false
end

then

local matched = is_domain_whitelisted(domain)
if matched WILDCARD_MATCHED  then
  --is wildcard match
else if matched then
  --is exact match
else
 --not match
end

lib/resty/acme/autossl.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Show resolved Hide resolved
lib/resty/acme/autossl.lua Outdated Show resolved Hide resolved
Copy link
Owner

@fffonion fffonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good now, some logging changes

lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/challenge/dns-01.lua Outdated Show resolved Hide resolved
lib/resty/acme/client.lua Outdated Show resolved Hide resolved
@yuweizzz yuweizzz requested a review from fffonion May 22, 2024 03:53
@fffonion fffonion changed the base branch from master to pr-114 May 22, 2024 04:03
@fffonion fffonion merged commit e1dd573 into fffonion:pr-114 May 22, 2024
5 checks passed
@fffonion
Copy link
Owner

fffonion commented May 22, 2024

I will do a final cleanup of commits and merge to master #115

@fffonion
Copy link
Owner

Thanks for this big PR, awesome work! @yuweizzz

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 this pull request may close these issues.

2 participants