Subject: [Anima] GRASP details - WGLC draft-ietf-anima-bootstrapping-keyinfra-15

From: Toerless Eckert <[email-masked]>
Date: Fri, 1 Jun 2018 08:23:43 +0200

Ok, lets separate out the threads here.

On Thu, May 31, 2018 at 03:35:31PM -0400, Michael Richardson wrote:
> 
> Toerless Eckert <[email-masked]> wrote:
>     > 1. The GRASP specification of 4.1.1 should only describe what is required
>     > and valid for the standard of GRASP objective, which is the TCP proxy.
> 
>     > Appendix C proxy option is not full/formally worked out, thats why
>     > its in an appendix. If the authors want to propose a formal GRASP
> 
> It's not mandatory to implement, which is why it got pushed to the appendix.
> If it wasn't worked out, then it would be removed.

Ok. *sigh*. I guess i didn't quite get that. So, if we want to make GRASP in BRSKI
fwork correctly for IPinIP instead of just making it sugestive, there are a bunch
of fixes necessary. Some of them where part of the shepherd review suggestion
you dismissed as bikeshed. You didn't like the idea of using objective-value strings
to identify the proxy method. But the fix i suggested also fixed other GRASP
details.

Anyhow. let me just list what i think is necessary to fi up the GRASP so it works
for both TLS and IPinIP.

let me know if/how i can best help to get this done,I'd be happy to write up a
diff if we can finally get through this.

Proposed changes to 4.1.1:

   Note: This 4.1.1 text fixes are not complete wrt. to the IPinIP port
   constraints you seem to want to have according to the 4.2 text. See below
   for the 4.2 text discus, and if thats fine with you, then of course, we would
   also need to incude the locator-options for permitted TCP/UDP ports of the IPinIP
   proxy into the 4.1.1 AN_Proxy objective, because otherwise the Pledge wouldn't
   know which TCP or UDP (address)/port numbers to use across the IPinIP proxy (i think).

   a) Insert after:
      | A proxy uses the DULL GRASP M_FLOOD mechanism to announce itself.
      | This announcement can be within the same message as the ACP
      | announcement detailed in [I-D.ietf-anima-autonomic-control-plane].

      The optional IPinIP proxy as described in Appendix C requires
      the following extension to the syntax of [GRASP]:

      transport-proto /= IPPROTO_IPINIP
      IPPROTO_IPV6 = 41  ; [RFC2473]

      Figure xx: GRASP syntax extensions for BRSKI

      Note: Process wise we might need to declare BRSKI RFC to be an update to
      GRASP RFC because of these two lines, but i would just wait until this gets
      discussed with IANA and then suggest that we change instead GRASP before
      it becomes RFC to: 

       transport-proto = 0..255

      Aka: that way we can define IPPROTO_IPV6 perfectly in BRSKI without
      extending GRASP (IMHO). We can still later on expend it beyond 255 when we need to,
      but thats not a BRSKI RFC problem then anymore.

   b) Change:
      | The M_FLOOD is formatted as follows:

      To:
      An example of M_FLOOD (without ACP) is as follows:

      (aka: "is formatted as" is not the correct lead in for an example).

   c) Fix up example picture:

      [M_FLOOD, 12340815, h'fe800000000000000000000000000001', 180000,
          [ ["AN_Proxy", 4, 1 ],
            [O_IPv6_LOCATOR,
             h'fe800000000000000000000000000001', IPPROTO_TCP, 4443]],
          [ ["AN_Proxy", 4, 1 ],
            [O_IPv6_LOCATOR,
             h'fe800000000000000000000000000001', IPPROTO_IPV6, 0]] ]
      
      Figure 6b: AN_Proxy Example

      This is actually multiple fixes
       - Better title for picture (match AN_Proxy with following
         CDDL picture title and 'example')
       - the objective-value is not used, so "" for objective-value
         is not really correct IMHO, the field is just not encoded:
          ["AN_Proxy", 4, 1, "" ] -> ["AN_Proxy", 4, 1 ] 
       - I think it helps a lot to understand how to announce
         multiple proxy options, so i highly suggest to have the
         above example include the TCP circuit proxy and IPinIP options.
       - Whether or not you use one or two objective announcements,
         you always need one more structure level around objective
         and locator-option according to GRASP syntax,
         aka:
         changed:    ["AN_Proxy", 4, 1 ], ... 4443]
         to:       [ ["AN_Proxy", 4, 1 ], ... 4443] ]
      
   c) Fix syntax:
       from: flood-message = [M_FLOOD, session-id, initiator, ttl,
                           +[objective, (locator-option / [])]]

       to:  flood-message = [M_FLOOD, session-id, initiator, ttl,
                           +[objective, locator-option ]]
       
       Aka: for this objective, locator-option is not optional,
       but mandatory, because it indicates the proxy-type via the
       transport-type which is part of the locator-option.

    d) from: locator         = [ O_IPv6_LOCATOR, ipv6-address,
       to:   locator-option  = [ O_IPv6_LOCATOR, ipv6-address,

       textual consistency with prior use of locator-option and GRASP RFC.
   
    e) Add at end of 4.1.1 suggested text:
       
       The transport-proto of the locator-option indicates the mechanism(s)
       supported by the proxy to the pledge. IPPROTO_TCP indicates the
       mandatory ANI TLS circuit proxy. IPPROTO_IPV6 indicates the optional
       IPinIP proxy, see Appendix C. IPPROTO_UDP would indicate a future
       CoAP mechanism, see Section 4.2. For IPPROTO_IPV6, proto-number
       MUST be 0.
      
       The above example shows a proxy supporting both ANI TLS circuit proxy
       and IP in IP proxy.

       Notes:

       I hope i understand the IP in IP proxy correctly, but given that
       there is no port number in IPinIP, and given that port-number is
       mandatory in GRASP syntax in this case, 0 seems to be the logical
       number to use for port-number in this case.

       I am just guessing you want IPPROTO_UDP to indicate CoAP in the
       future. If so, then it would be prudent to notice this here,
       otherwise describe what IPPROTO_UDP should mean, and if there is
       no predefined meaning, just remove it from the CDDL syntax.

   f) If you do not fancy to include the IP in IP example and text in
      4.1.1, then i would strongly suggest to remove ALL indications of
      IP in IP, including the mentioning of IPPROTO_IPv6 from 4.1.1
      (also the extension to the GRASP syntax) - and instead put that
      all into appendix C. Your choice. I think the text is more succinct
      if all the grasp stuff is clearly in 4.1.1.
      
2) Changes for 4.3

   a) The objective-value "EST-TLS" serves no purpose and is wrong.
      Please remove it from the syntax, ak: no objective-value, like
      also no objective-value in 4.1.1
      - Its unnecessary, because if its used for all possible methods
        (TLS, IPinIP, ..) then it doesn't provide additional information
        and is redundant.
      - It is wrong, because the protocol would not just be EST, but BRSKI.
      - It is wrong, because for CoAP there would be no TLS.

   b) Please consider improving the example as above for 4.1.1:
      - lead in text for example
      - example title
      - [ [ objective, locator-option ] ] structure fix
      - Ideally also include both TLS and IPinIP options in example

      Also:
      - I find the use of port 80 in the example highly confusing given how
        the TCP connection MUST use TLS. Please change to AB80 (anything but 80).

   c) The CDDL syntax should be more consistent with 4.1.1:
      - locator-option not optional
      - locator, ipv6-address, transport-proto, port-number could
        be included in syntax.
    
   d)  "network administrator policy (EST server configuration)"

       You where making during ACP review the point that BRSKI Registrar (for enrollment)
       would not necessarily be the same as the EST server for cert renewal,
       therefore the mentioning of "EST server" is confusing. would be
       less confusing if the term (BRSKI) Registrar was used everywhere,
       and "EST server" only when there is a real reason. 

   e)  "A protocol of 41 indicates that packets may be IPIP proxy'ed"

       So, unless we get back to my shepherd review "bikeshed" of introducing
       different objective value strings for different proxy methods,
       the "transport-proto" is the only way for the proxy to identify the
       method, so this sentence needs to say "will be" instead of "may be".

       Also: Delete following "In the case of that IPIP proxying is used, then".

       Also: "MUST be advertised on the" -> "MUST be advertised by the Proxy on the".
   
   f) the mechanism you are describing with locator1, locator2 and locator3
      would be highly obfuscated:

      In GRASP, every objective can have only one locator as seen from the syntax.
      What you are proposing would mean that you are announcing three separate
      AN_join_registrar objectives (of course, they can all be together in a single 
      M_Flood), but then effectively you're saying that those three objectives
      do really describe a single IPinIP mechanism. 

      So, your full example locators with objectives would be:

       [["AN_join_registrar", 4, 255], [O_IPv6_LOCATOR, fd45:1345::6789, 6,  443]] ]
       [["AN_join_registrar", 4, 255], [O_IPv6_LOCATOR, fd45:1345::6789, 17, 5683] ]
       [["AN_join_registrar", 4, 255], [O_IPv6_LOCATOR, fe80::1234, 41, 0] ]

      Is this join registrar supporting ANI TLS proxy ?
      Aka: i can't distinguish for the TCP locator whether it just indicates
      a permitted TCP port for the IPinIP proxy or whether it indicates
      the TCP port supported for IPinIP. And even if the proxy supports both,
      its not clear to me that the TCP ports for "native" would be the same as
      for IPinIP. Maybe its different code-paths == different ports.

      Likewise UDP port could be CoAP proxy or again a parameter for IPinIP.

      I would suggest to only use a single objective for IPinIP, aka: the line with
      41, 0, and encode the TCP/UDP locators to use inside of the IPinIP proxy
      mechanisms as the objective-value when using the IPinIPi method:

      objective-value = *( [ ipv6-locator-option ] ) ; only for IPinIP proxy
      
      Then we could show an example like this:
     
      [M_FLOOD, 12340815, h'fda379a6f6ee00000200000064000001', 180000,
        [ ["AN_join_registrar", 4, 255 ],
          [O_IPv6_LOCATOR, h'fda379a6f6ee0::200000064000001', 6, 443]
        ],
        [ ["AN_join_registrar", 4, 255,
            [[O_IPv6_LOCATOR, h'fda379a6f6ee0::200000064000001', 6, 4443],
             [O_IPv6_LOCATOR, h'fda379a6f6ee0::200000064000001',17, 5683]]
          ],
          [O_IPv6_LOCATOR, fe80::1234, 41, 0]
        ]
      ]

      That would include your two constraining locators for the IPinIP proxy
      and separately the "native" circuit proxy.  And as mentioned in the
      intro for 4.1.1, the same structure of information for the IPinIP proxy
      would be necessary for AN_Proxy.

I'll stop here. Let me know what you think. I'll be happy to help apply the
fixes / send diff if this is ok. with you.

Cheers
    Toerless

> -- 
> ]               Never tell me the odds!                 | ipv6 mesh networks [ 
> ]   Michael Richardson, Sandelman Software Works        | network architect  [ 
> ]     [email-masked]  http://www.sandelman.ca/        |   ruby on rails    [ 
> 	

> _______________________________________________
> Anima mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/anima