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

[Paul Wouters <[e-mail]>]

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