Re: [DNSOP] Secdir early review of draft-ietf-dnsop-domain-verification-techniques-01

[Benjamin Kaduk <[e-mail]>]

Hi Paul, all,

On Thu, Apr 20, 2023 at 11:46:30AM -0400, Paul Wouters wrote:
> On Wed, 19 Apr 2023, Benjamin Kaduk via Datatracker wrote:
> 
> [co-author hat on]
> 
[...]
> 
> > ### 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.

Thanks!

> > ### 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.

Thanks.  I probably should have asked my question differently, in that I
was pretty sure that the underscore was important, and I just wanted the
document to be clear that it was required (or just recommended, if that's
the case, but I see no reason to do anything other than required).

> > ### 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.

Thanks.  As was hopefully clear, I read the paragraph several times and had
a couple different takes as I kept reading.  It seems that probably the key
points are:

- there's a recommended format (and what it is)
- even if you don't use the recommended format, you have to have the token
  in it (and per above, maybe some metadata to identify what is the token)
- you should have some human-understandable guidance on when it's safe to
  remove the record, in the content

and currently the text covers all three together, and can feel a bit mixed
up.  Splitting them into separate points would (IMHO) aid readability.

> > ### 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.

Ok.  I'm happy to hear that it was thought about.

> > ### 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.

Ok, thanks.

> > ### 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.

Ah, good point about 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).

Maybe (or maybe not) we could say something about how our recommendations
are shaped by the prior art (documented in Appendix A) and combine
practices used by many different examples, so that any given existing
technique will not necessarily be doing everything that we recommend.  And
perhaps that future techniques published by the IETF are expected to adopt
the best practices specified here.

> > 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 !

Happy to help!

-Ben