On Wed, 19 Apr 2023, Benjamin Kaduk via Datatracker wrote:

[co-author hat on]

Reviewer: Benjamin Kaduk
Review result: Has Issues

# SecDir review of draft-ietf-dnsop-domain-verification-techniques-01

Thanks for the review!

On the whole the content is reasonable, but read on for some suggestions on
how to tighten things up and some questions about why all the pieces are
needed.

I also made a PR with some editorial suggestions, https://github.com/ietf-wg-dnsop/draft-ietf-dnsop-domain-verification-techniques/pull/51

Thanks. We will have a look at those. We have a few more issues that
need merging in from IETF 116 dicussions.

### Continuing Ownership

While I see the example in Appendix A.1.4.1 about Atlassian, when we say in
the introduction that sometimes the DNS record must be kept in the zone to
prove continued ownership of the domain, that doesn't exactly seem to hold
true, or at least only provides a weak indication of continued ownership.  I
think that to really prove continued ownership the challenge would need to be
rotated periodically; just putting one record in once and leaving it there
mostly only shows that you had control of the domain at the one point in time
it was added and that there isn't anything cleaning up old records.  A
wholesale migration of the domain to a different DNS server would clean up old
records, but staffing changes within a large organization would not.

That's a great point. We will add those considerations to the document.

### Underscore for attrleaf

In §3.1 we say "The provider constructs the validation domain name by
prepending a provider-relevant prefix followed by "-challenge" to the domain
name being validated (e.g. "_foo-challenge.example.com")." which implicitly
uses an underscore-prefixed atterleaf name component.  Is the underscore part
of the required or recommended behavior?

We can clarify it. The reason for the underscore is to avoid any
accidental real hostname matching. Eg someone could validly already use
"foo-challenge.example.com" to host a website. As underscores are not
valid hostnames (but are valid DNS names), the underscore prevents such
collissions.

### Hashing the random token

The recommendations in §3.1 requires computing the SHA-256 digest of the
Random Token before base64url encoding and use as RDATA, but provides no
justification for the SHA-256 step that I can see.  If the intent is for the
Random Token to be a non-public identifier for the challeng with only the
digest value being public, we should say that.  Otherwise it's not clear what
we gain over just using base64url(random-data) as the RDATA.

Good point. Let me get back to my co-authors on that.

### TXT RDATA contents

We say that the RDATA of the recommended TXT resource record construction must
"contain" a certain token construction (rather than "consist of").  Does the
format/structure of the RDATA also need to be one that unambiguously
identifies where the token is (or is a raw substring match sufficient)?

I believe we were trying to avoid issues where DNS presentation format can
have multi-line and spaces in them. Let me also take this back to my
coauthors for some discussion. Thanks for spotting this!.

I think we might provide clearer guidance if we laid out the recommended
comma-separated key-value format first and talked about the token= contents,
and then only afterward mention that even if other formats are used, the RDATA
MUST contain the token value (and whether it must be identifiable as such via
some metadata).  Or we could state up front what properties we want from the
RDATA before going on to describe how we achieve those properties.  But the
current formulation starts out with a MUST-level requirement without much
context and then tries to build the context around it, which tends to cause
confusion in the reader.

Reading again, I think that the core of what is confusing me here is that the
SHOULD-level guidance for using the comma-separated key-value pair format is
buried as an aside in the mechanism for conveying the instructions for
removing a resource record.  It seems like it's a more generic recommendation
and should be framed as such.

We will look at clarifying that.

### Instructions for TXT record removal

We say

Providers MUST provide clear instructions on when a verifying record
can be removed.  The user SHOULD de-provision the resource record
provisioned for a DNS-based domain verification challenge once the
one-time challenge is complete.  These instructions SHOULD be encoded
in the RDATA via comma-separated ASCII key-value pairs [RFC1464]
using the key expiry.  If this is done, the token should have a key
token.  For example:

I think that the "these instructions" refers to the "clear instructions"
provided by the provider.  But the TXT record is something provisioned by the
user, not the provider.  If the instructions are to be encoded in the RDATA,
does that require that the RDATA contents themselves are provided by the
provider to the user?  I'd strongly recommend including some more description
of the workflow that's envisioned if it includes the provider giving the user
the entire RDATA contents to copy/paste in place (and also using those RDATA
contents as a separate information channel to the user).

Yes, often there are people who are not DNS experts who are told "to use
our service, put this in your DNS", and they can only relay those
instructions as a blob because they do not have engineering skills on
DNS. I'll see how to clarify this further.

Additionally, do we make any requirement/recommendation of the format used for
the value corresponding to the "expiry" key?

We did not. We did talk about it but didn't want to mandate too much.
The basic feature is that it should be operator human readable and
understandable without documentation or previous knowledge.

### CNAME chaining

In §3.2 we see the text "Another issue with CNAME records is that they must
not point to another CNAME" provided, with no reference.  But CNAME chains are
in heavy use on the internet in practice with no ill effect (case in point: my
employer's CDN business), so I'd recommend removing the discussion of CNAME
chaining, or supplying a reference and discussion around them that is closer
to the deployment reality.

Looking up the exact reference, I do find in 
https://www.rfc-editor.org/rfc/rfc1034.html#section-3.6.2

        CNAME chains should be followed

So perhaps our advise here was a bit too strong. We already had an open
item at looking to change the language there to make it less restrictive
on CNAMEs so we will take item this into that discussion.

### Relationship to other work

While I don't mind a focus on DNS-based domain-verification techniques in this
document, it does seem like we might want to spend a little more time placing
this work in the context of other schemes.  For example, ACME has a number of
challenge types, and one of them is essentially the DNS-based stuff covered by
this document.  Is what RFC 8555 specifies compliant with this document's
requirements and recommendations?

This document is about DNS based domain validation techniques. We did
not want to mixup DNS based techniques with other techniques. Eg for
HTTP(S) based techniques, other working groups likely have better
knowledge about the specific technology pitfalls and recommendations.

Do we expect ACME to use our guidance for DNS-based verification

ACME (dns-01) is not entirely following this advise, but if they ever
did an updated version, we would hope they would take this document
into consideration. Most importantly, it is not self-documenting.

Meaning that a DNS admin might not know what _acme-challange is and the
record is not clear on when it can be removed from the zone (expired).

and keep doing what they do for HTTP, TLS-SNI, etc.

See above.

### AVOID-FRAGMENTATION reference classification

The actual text in Appendix A.1.1 that references [AVOID-FRAGMENTATION] does
not contain any usage that would obviously promote it to a normative
reference.  If there is intent to require implementors of this document to
read that one, some additional discussion of which parts are normative
requirements and why seems in order.

Noted. will fix.

### Motivation for application-specific name prefix

In Appendix A.1.1 we have some good text about an attack where a malicious
service can provide to the user the challenge from a different provider, when
the TXT verification is on the actual domain name being verified (no
prefix/suffix).  I would say this fits better in the main document (security
considerations, probably) rather than the appendix with survey of current
usage.

Good point, will look at better placement.

## Nits

### APEX Capitalization

I don't remember seeing APEX used as a defined term in all-caps elsewhere.
What value do we think we're adding by writing it this way?

Based on RFC8499 (DNS Terminology) you are right and it is lower case. We will 
change it.

### Verifying Record

We seem to implicitly introduce the term "verifying record" in §3.1; it might
benefit from having an explicit definition.

Will add a sentence somewhere.

### Self-referential Appendix

Appendix A.1.1 contains the text "See Appendix A for a survey of different
implementations.", which is perhaps not a very useful reference.

:)

### DNS name component ordering

In Appendix A.1.1 we write "Some providers use a suffix of
_PROVIDER_NAME-challenge in the Name field of the TXT record challenge", when
that value is a suffix in the wire format of the name but a prefix in the
presentation form of the name.  This is especially confusing when the next
sentence goes on to include an example in presentation form,
_acme-challenge.<YOUR_DOMAIN>, where the _PROVIDER_NAME-challenge is a prefix.
Maybe talking about "leaf" rather than prefix or suffix would be most clear.

Will think about clarifying this.

Again, thanks for the secdir review. This is great feedback !

Paul

_______________________________________________
DNSOP mailing list
DNSOP@ietf.org
https://www.ietf.org/mailman/listinfo/dnsop

Reply via email to