Skip to main content

Last Call Review of draft-ietf-dane-ops-12
review-ietf-dane-ops-12-genart-lc-davies-2015-07-10-00

Request Review of draft-ietf-dane-ops
Requested revision No specific revision (document currently at 16)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2015-08-03
Requested 2015-06-12
Authors Viktor Dukhovni , Wes Hardaker
I-D last updated 2015-07-10
Completed reviews Genart Last Call review of -12 by Elwyn B. Davies (diff)
Genart Telechat review of -14 by Elwyn B. Davies (diff)
Secdir Last Call review of -12 by Tina Tsou (Ting ZOU) (diff)
Opsdir Last Call review of -12 by Fred Baker (diff)
Assignment Reviewer Elwyn B. Davies
State Completed
Request Last Call review on draft-ietf-dane-ops by General Area Review Team (Gen-ART) Assigned
Reviewed revision 12 (document currently at 16)
Result Almost ready
Completed 2015-07-10
review-ietf-dane-ops-12-genart-lc-davies-2015-07-10-00
  
  
    I am the assigned Gen-ART reviewer for this draft. For background on
    


    Gen-ART, please see the FAQ at
    







<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>

.
    





    Please resolve these comments along with any other Last Call
    comments
    


    you may receive.
    





    Document:
    draft-ietf-dane-ops-12.txt


    Reviewer:
    Elwyn Davies


    Review Date: 2015/06/19


    IETF LC End Date:
    2015/06/24


    IESG Telechat date: (if known) 
    -





    Summary: 
    Almost ready.  There are a couple of minor issues, and I think the
    authors need to reassess whether all the cases where RFC 2119
    keywoDANE and rds are actually protocol requirements as opposed to
    operation best practice recommendations.  Caveat: I am not a DNSSEC
    expert and I can't really be sure about the interactions of DNSSEC
    and DANE.    





    Major issues:
    


    None





    Minor issues:
    


    s3: I am not a security expert, but I suspect you may get some
    pushback on not making TLS 1.2 mandatory.


    Also I wonder whether SSL 2.0 and SSL 3.0 should be explicitly ruled
    out? The SECDIR review may say something here.





    s3: 







TLS clients and servers using DANE SHOULD support the
   "Server Name Indication" (SNI) extension of TLS.





    Under what circumstances would it be reasonable for a DANE/TLSA
    server not to support SNI?   Maybe I could see that a single domain
    server could do without it.  I think this needs to be spelt out -
    earlier text seems to indicate that SNI support was pretty much
    mandatory.... Ah! Later I see that s5.1 gives a case where SNI is
    not mandatory - a forward pointer would help.





    s4.1:  I am not clear that this section adds any value to the
    discussion in s4.  Why should the protocol designer be less inclined
    to use PKIX-xx rather than DANE-xx when the protocol supports an OS
    mode as opposed to just fully authenticated or fully authenticated
    plus cleartext modes?  Is there some positive advantage to the
    DANE-xx authentication failing when it comes to deciding whether to
    proceed with OS?  Otherwise it seems to me that this section just
    says that you can use DANE with an OS protocol and the s4
    recommendation applies.  It is possible that I do not understand
    what is being said here.





    Nits/editorial comments: 


    =====================





    General: The use of 'we' in RFCs is generally deprecated.  There are
    about 20 examples in the draft that need remedying including:


    s1, last para: s/In Section 12, we summarize/Section 12 summarizes/


    s1.1, definition of Customer Domain:


    OLD:


          We will refer to


          the domain name used to locate the service (prior to any


          redirection) as the "Customer Domain".


    NEW:


          The domain name used to locate the service (prior to any


          redirection) will be referred to as the "Customer Domain".


    END


    s1.1, definition of TLSA parameters: s/in fourth/in the fourth/,
    s/we need/it is necessary/, 


    s/we will call the first three fields/the first three fields will be
    called/ 


    s2, para 5:


    OLD:


       We may think of TLSA Certificate Usage values 0 through 3 as a


       combination of two one-bit flags.


    NEW:


       The TLSA Certificate Usage values 0 through 3 can be interpreted
    as a


       combination of two one-bit flags.


    END


    s5.1, para 1: s/We also extend [RFC6698]/[RFC6698] is also extended/


    s6, next to last para: s/we describe an approach/an approach is
    described/


    s8, para 1:


    OLD:


       We describe a TLSA


       record update algorithm that ensures this requirement is met.


    NEW:


       This document describes a TLSA


       record update algorithm that ensures this requirement is met.


    END


    s9, para 1: 


    OLD:


       We specify such a protocol below.


    NEW:


       Such a protocol is specified below.


    END


    s12: Several instances of "we"... 


    - Bullets 1 - 3: s/we update [RFC6698]/[RFC6698] is updated/


    - Bullet 4: s/we explain/explains/


    - Bullet 4: s/With usage PKIX-TA(0) we note that/Note that with the
    usage of PKIX-TA(0)/, s/processes/process/


    - Bullets 5 and 8: s/In Section x, we recommend/Section x
    recommends/


    - Bullets 6 and 7: s/In Section x we specify/Section x specifies/


    ----------------





    General: I am dubious about the usage of RFC 2119 keywords for
    constraints that are recommendations for operational usage rather
    than protocol on the wire constraints.  In each case, I think the
    question you have to consider is "If the client 


    or server violates the constraint could the box at the other end of
    the wire tell that the constraint had been violated?"  If the far
    end can't tell then RFC 2119 language is probably inappropriate. 
    AFAICS in at least some of the instances this needs to be rechecked
    - I can't be sure in several of the cases.  Examples include:


    s4, next to last and last paras:  I think the "MAY" in the last para
    should be "may" - if not probably the "recommends" in the previous
    para should be RECOMMENDS. 


    s4.1: Again this is an operational suggestion, so s/SHOULD
    NOT/should not/ as the implementation can't really tell it is
    wrongly used.


    s5.1, para 6: s/SHOULD/should/ - operational practice rather than
    protocol specification again.


    s5.1, paras 7 and 8:  Probably s/SHOULD NOT/should not/ - I think
    (but am not certain in these cases) that these are again operational
    practice rather than something that is checked in the protocol.


    s5.2, paras 1 and 4:  Again the usage of RFC 2118 keywords needs to
    be checked.


    ----------------------------





    Other editorial nits:


      


    s1:  Probably worth repeating that TLSA is not an acronym.





    s1: Perhaps worth repeating the expansion of DANE here.





    s1, para 1: Need to expand DNSSEC on first use.





    s1: Need to expand TLS on first use (currently it happens later).





    s1.1, definition of SNI: suggest s/(i.e., "secure virtual
    hosting")./(i.e., provide "secure virtual hosting")./





    s2, next to last para:  ASN.1 DER needs a reference - to X.690 (as
    in RFC 6698).





    s4.2: I would be inclined to emphasize that RFC 6962 is experimental
    so that this is not an issue that would arise in current operational
    practice - and maybe that a standards track version of CT would be
    better integrated.  Maybe update para 1:


    OLD:




   Certificate Transparency (CT) [

RFC6962

] defines an approach to
   mitigate the risk of rogue or compromised public CAs issuing
   unauthorized certificates.  This section clarifies the interaction of
   CT and DANE.
NEW:
   Certificate Transparency (CT) [RFC6962] defines an experimental approach 
   that could be used to mitigate the risk of rogue or compromised public 
   CAs issuing unauthorized certificates.  This section clarifies the 
   interaction of the experimental CT and DANE.  This interaction may need 
   to be revisited in the event that a standards track version of CT is 
   developed in future.
END



    s4.3:  There appears to be a conflict between the title and second
    sentences vs. the  third sentence:







When transitioning between PKIX-TA/PKIX-EE and
   DANE-TA/DANE-EE, clients begin to enable support for the new
   certificate usage values.





    and







If the new preferred certificate usages
   are PKIX-TA/EE this requires installing and managing the appropriate
   set of CA trust anchors.    





    I don't see why 'new' appears in the third sentence if the scenario
    is transitioning away from PKIX-TA/PKIX-EE.


    Am I confused?





    s5, title: s/Certificate-Usage/Certificate Usage/ for consistency
    with the rest of the section.





    s5.1, para 4: The term "server's primary certificate" does not
    appear to be defined.  I think I can probably guess (something
    associated with the physical host rather than any virtual hosts?)
    but defining it explicitly would be good.  





    s8.3, para 1: s/each combinations/each combination/





    s9, para 3: Expand acronym "MTA".





    s9, last para: If it is decided that this is a protocol requirement:
    s/SHOULD not/SHOULD NOT/





    s11, para 2: Expand acronym "gTLD".





    s17.2: I think RFC7250 is probably normative.





    s17.2: I can't decide whether I would like RFC6781 to be normative.
    It would of course be a downref.  This is always going to be a
    problem since the draft is combining operational considerations and
    protocol updates.   The one explicit reference indicates you have to
    know something about how to run DNSSEC to run DANE - in practice I
    think you have to know a great deal about how to run DNSSEC if you
    are going to run DANE so I would be inclined to make this normative
    (and maybe refer to it in the introduction as well).