Subject: [Drip] Implementation questions from students

From: Andrei Gurtov <[email-masked]>
Date: Mon, 12 Oct 2020 11:34:51 +0200

Hello,

A few questions from a group of students working on DRIP implementation.
I guess mostly to Bob. Or it should be sent to hipsec instead?

br Andrei

-------- Forwarded Message --------

	

We had some questions about the HIP project that you could perhaps
forward to the correct person:

 1. In the function /build_tlv_hmac()/ in src/protocol/hip_output.c,
    there is the following bit of code:

    /* get lower 160-bits of HMAC computation */

     memcpy(hmac->hmac,

             &hmac_md[hmac_md_len-sizeof(hmac->hmac)],

             sizeof(hmac->hmac));

    (https://bitbucket.org/openhip/openhip/src/37972774633783905ec4f7961d2a5476092ed529/src/protocol/hip_output.c#lines-1373)

    The expression used as index into /hmac_md//[]/ seems to be
    calculated erroneously, as /sizeof(hmac->hmac)/ evaluates to 64
    bytes and /hmac_md_len/ is the output size of the hash function
    used. Since SHA256 for example produces 256 bits / 8 = 32 bytes as
    output, this calculation evaluates to 32 - 64 = -32 when treated as
    signed integers. This results in the data between /hmac_md[-32]/ and
    /hmac_md[32]/ being read, which reads out of bounds for the array.
    Using GDB we have observed that this causes other values located on
    the stack (such as /hmac_md_len/) to be copied into /hmac->hmac//./
    Also, per RFC 7401 (HIPv2), "The size of the HMAC is the natural
    size of the hash computation output depending on the used hash
    function". Thus there does not seem to be a need to truncate this
    value to 160 bits? At least one implementation
    (https://www.cryptosys.net/manapi/api_kmac.html) of the KMAC
    function (that we have added as an option to HMAC) also states that
    the output of the KMAC function cannot be truncated when used as a
    message authentication code.

 2. When parsing incoming R1 packets (and probably some other types as
    well), the peer's HIT from the packet seems to be validated against
    a locally computed HIT from the peer's HI. This occurs before any
    parameters other than the puzzle, host ID, and signature have been
    parsed. In /hip_parse_R1()/, /validate_hit()/ uses /hi_to_hit()/ to
    compute this and compares the returned value to the one received
    from the peer. However, since the HIT_SUITE_LIST parameter has not
    yet been parsed, the variable /hip_a->peer_hi->hit_suite_id/ still
    has its default value of 0. There is a fallback in /hi_to_hit()/
    which silently uses SHA256 as the hashing algorithm for creating a
    HIT from a HI if the HIT suite is set to an invalid value, which
    enables the verification to succeed if the received HIT happens to
    also be calculated with SHA256. Other algorithms (such as cSHAKE
    which we are adding) will cause this to fail, as
    /hip_a->peer_hi->hit_suite_id/ is only set correctly after the HIT
    has been verified. We have worked around this issue by first
    ignoring all parameters other than the HIT_SUITE_LIST (thus ensuring
    that the HIT suite variable is set before any other parameters are
    parsed), then continuing with parsing the signature-related
    parameters and finally the others. Is this an acceptable solution to
    this problem? Those silent fallbacks seem to cause more problems
    that they solve, maybe they should be removed or at least emit a
    warning when triggered?

 3. What should the lengths for the key and IV be for River/Lake Keyak?
    It seems that these lengths are variable and can be chosen at runtime.

 4. There does not seem to be any support for EdDSA25519ph or EdDSA448ph
    in OpenSSL, and we cannot find any C implementations from a
    reputable source. Would it be OK if we held off on adding these for
    the time being? EdDSA25519 and EdDSA448 are implemented in OpenSSL
    1.1.1 and seem to work for use in OpenHIP.

 5. In section 6 of
    https://www.ietf.org/id/draft-moskowitz-hip-new-crypto-05.html,
    there is a comment about how SHAKE, cSHAKE or KMAC could be used as
    a pseudo-random generator. Should we do something with this
    information, or is it just there for future reference? Isn't
    randomness usually handled by the kernel since it has access to
    higher-quality entropy sources, and if so, is there any place in the
    OpenHIP code where it would be beneficial to use SHAKE/cSHAKE/KMAC
    for this purpose?

 6. Should the new encryption- and signature algorithms we are adding be
    used as default in OpenHIP or just be available as a command line
    option?

 7. (This is mostly a minor thing) When we are adding KMAC as an
    alternative to HMAC, is it proper to put this into a function called
    /build_tlv_*hmac*()/? Seeing as this function will then sometimes
    calculate a HMAC and other times a KMAC, should it be renamed to
    /build_tlv_mac()/? Or will it be less intrusive to others (that
    might be used to the name) to not change it even though the name
    might be misleading? Or should we add a new function called
    /build_tlv_kmac()/? (Although this might lead to a lot of
    switch-case statements everywhere that /build_tlv_hmac()/ is called)

---

From: Robert Moskowitz <[email-masked]>
Date: Mon, 12 Oct 2020 08:43:55 -0400

I will give this a try.  Some of these you will want to ask on hipsec.

On 10/12/20 5:34 AM, Andrei Gurtov wrote:
>
> Hello,
>
> A few questions from a group of students working on DRIP 
> implementation. I guess mostly to Bob. Or it should be sent to hipsec 
> instead?
>
> br Andrei
>
>
>     output, this calculation evaluates to 32 - 64 = -32 when treated
>     as signed integers. This results in the data between
>     /hmac_md[-32]/ and /hmac_md[32]/ being read, which reads out of
>     bounds for the array. Using GDB we have observed that this causes
>     other values located on the stack (such as /hmac_md_len/) to be
>     copied into /hmac->hmac//./ Also, per RFC 7401 (HIPv2), "The size
>     of the HMAC is the natural size of the hash computation output
>     depending on the used hash function". Thus there does not seem to
>     be a need to truncate this value to 160 bits?
>

This needs to be asked on hipsec and to the openhip developers.

>  1. At least one implementation
>     function (that we have added as an option to HMAC) also states
>     that the output of the KMAC function cannot be truncated when used
>     as a message authentication code.
>

When HMAC and 7401 RHASH were written, I doubt if anyone envisioned a 
true variable length output hash.  So what you use KMAC, you set its 
output to the size needed so that there is no truncation.

IMHO, KMAC is really powerful.  The whole sponge function approach is a 
significant change to past approaches.

> 1.
>     well), the peer's HIT from the packet seems to be validated
>     against a locally computed HIT from the peer's HI. This occurs
>     before any parameters other than the puzzle, host ID, and
>     signature have been parsed. In /hip_parse_R1()/, /validate_hit()/
>     uses /hi_to_hit()/ to compute this and compares the returned value
>     to the one received from the peer. However, since the
>     HIT_SUITE_LIST parameter has not yet been parsed, the variable
>     /hip_a->peer_hi->hit_suite_id/ still has its default value of 0.
>     There is a fallback in /hi_to_hit()/ which silently uses SHA256 as
>     the hashing algorithm for creating a HIT from a HI if the HIT
>     suite is set to an invalid value, which enables the verification
>     to succeed if the received HIT happens to also be calculated with
>     SHA256. Other algorithms (such as cSHAKE which we are adding) will
>     cause this to fail, as /hip_a->peer_hi->hit_suite_id/ is only set
>     correctly after the HIT has been verified. We have worked around
>     this issue by first ignoring all parameters other than the
>     HIT_SUITE_LIST (thus ensuring that the HIT suite variable is set
>     before any other parameters are parsed), then continuing with
>     parsing the signature-related parameters and finally the others.
>     Is this an acceptable solution to this problem? Those silent
>     fallbacks seem to cause more problems that they solve, maybe they
>     should be removed or at least emit a warning when triggered?
>

Definitely a question for hipsec.

> 1.
>
>
>  2. What should the lengths for the key and IV be for River/Lake
>     Keyak? It seems that these lengths are variable and can be chosen
>     at runtime.
>

Grumble.  I will get back to you on this one.  Plus NIST is having its 
Lightweight Crypto Workshop next week, and the output of that MAY be the 
round 3 LWC selection which could change the selected LWC!  definitely a 
grumble.

Though I THINK the answer is somewhere in:

https://keccak.team/files/Keyakv2-doc2.2.pdf

> 1.
>
>
>  2. There does not seem to be any support for EdDSA25519ph or
>     EdDSA448ph in OpenSSL, and we cannot find any C implementations
>     from a reputable source. Would it be OK if we held off on adding
>     these for the time being? EdDSA25519 and EdDSA448 are implemented
>     in OpenSSL 1.1.1 and seem to work for use in OpenHIP.
>

And we are not using EdDSA25519ph or EdDSA448ph; there is still 
controversy on the prehash mode!  They really don't work in this 
context.  I believe.

> 1.
>  2. In section 6 of
>     there is a comment about how SHAKE, cSHAKE or KMAC could be used
>     as a pseudo-random generator. Should we do something with this
>     higher-quality entropy sources, and if so, is there any place in
>     the OpenHIP code where it would be beneficial to use
>     SHAKE/cSHAKE/KMAC for this purpose?
>

Read NIST SP800-185, Appendix B.  This is for implementations that are 
looking for a PRF using the Keccak code base.

For right now, hip-new-crypto is abandoned, with only those parts needed 
for drip-uas-rid.  Though for drip-secure-nrid-c2 it could well be 
important...

> 1.
>
>
>  2. Should the new encryption- and signature algorithms we are adding
>     be used as default in OpenHIP or just be available as a command
>     line option?
>

this is for hipsec.

> 1.
>
>
>  2. (This is mostly a minor thing) When we are adding KMAC as an
>     alternative to HMAC, is it proper to put this into a function
>     called /build_tlv_*hmac*()/? Seeing as this function will then
>     sometimes calculate a HMAC and other times a KMAC, should it be
>     renamed to /build_tlv_mac()/? Or will it be less intrusive to
>     others (that might be used to the name) to not change it even
>     though the name might be misleading? Or should we add a new
>     function called /build_tlv_kmac()/? (Although this might lead to a
>     lot of switch-case statements everywhere that /build_tlv_hmac()/
>     is called)
>

definitely hipsec...

Thanks for your contributions!

---

From: "Wiethuechter, Adam" <[email-masked]>
Date: Mon, 12 Oct 2020 16:43:44 -0400

Andrei,

Comments in line from me as I can respond. Note that this is all personal
opinion of how I would approach the given problems.

On Mon, Oct 12, 2020 at 5:35 AM Andrei Gurtov <[email-masked]> wrote:

> Hello,
>
> A few questions from a group of students working on DRIP implementation. I
> guess mostly to Bob. Or it should be sent to hipsec instead?
>
> br Andrei
>
> We had some questions about the HIP project that you could perhaps forward
> to the correct person:
>
>    1. In the function *build_tlv_hmac()* in src/protocol/hip_output.c,
>     memcpy( hmac->hmac,
>             &hmac_md[hmac_md_len - sizeof(hmac->hmac)],
>
>             sizeof(hmac->hmac));
>    (
>    https://bitbucket.org/openhip/openhip/src/37972774633783905ec4f7961d2a5476092ed529/src/protocol/hip_output.c#lines-1373
>    )
>
>    The expression used as index into *hmac_md**[]* seems to be calculated
>    erroneously, as *sizeof(hmac->hmac)* evaluates to 64 bytes and
>    *hmac_md_len* is the output size of the hash function used. Since
>    SHA256 for example produces 256 bits / 8 = 32 bytes as output, this
>    calculation evaluates to 32 - 64 = -32 when treated as signed integers.
>    This results in the data between *hmac_md[-32]* and *hmac_md[32]*
>    being read, which reads out of bounds for the array. Using GDB we have
>    observed that this causes other values located on the stack (such as
>    *hmac_md_len*) to be copied into *hmac->hmac**.* Also, per RFC 7401
>    (HIPv2), "The size of the HMAC is the natural size of the hash computation
>    output depending on the used hash function". Thus there does not seem to be
>    a need to truncate this value to 160 bits? At least one implementation (
>    https://www.cryptosys.net/manapi/api_kmac.html) of the KMAC function
>    (that we have added as an option to HMAC) also states that the output of
>    the KMAC function cannot be truncated when used as a message authentication
>    code.
>
> Judging by how you described the failure it seems like this is a change
that happened and no one has caught? Someone took the time to do this grab
of the lower 160 bits because they got more but now it sounds like the
functions always return what you need (eliminating the need for truncation
as you said).

I would ask the openhip devs this as I am unclear exactly why this would
still be used myself.

>
>
>    1. When parsing incoming R1 packets (and probably some other types as
>    well), the peer's HIT from the packet seems to be validated against a
>    locally computed HIT from the peer's HI. This occurs before any parameters
>    other than the puzzle, host ID, and signature have been parsed. In
>    *hip_parse_R1()*, *validate_hit()* uses *hi_to_hit()* to compute this
>    and compares the returned value to the one received from the peer. However,
>    since the HIT_SUITE_LIST parameter has not yet been parsed, the variable
>    *hip_a->peer_hi->hit_suite_id* still has its default value of 0. There
>    is a fallback in *hi_to_hit()* which silently uses SHA256 as the
>    hashing algorithm for creating a HIT from a HI if the HIT suite is set to
>    an invalid value, which enables the verification to succeed if the received
>    HIT happens to also be calculated with SHA256. Other algorithms (such as
>    cSHAKE which we are adding) will cause this to fail, as
>    *hip_a->peer_hi->hit_suite_id* is only set correctly after the HIT has
>    been verified. We have worked around this issue by first ignoring all
>    parameters other than the HIT_SUITE_LIST (thus ensuring that the HIT suite
>    variable is set before any other parameters are parsed), then continuing
>    with parsing the signature-related parameters and finally the others. Is
>    this an acceptable solution to this problem? Those silent fallbacks seem to
>    cause more problems that they solve, maybe they should be removed or at
>    least emit a warning when triggered?
>
> Your proposed solution is the way I would go about it. Start from the
atomic values and build from there.

>
>    1.
>    2. What should the lengths for the key and IV be for River/Lake Keyak?
>    3. There does not seem to be any support for EdDSA25519ph or
>    EdDSA448ph in OpenSSL, and we cannot find any C implementations from a
>    reputable source. Would it be OK if we held off on adding these for the
>    time being? EdDSA25519 and EdDSA448 are implemented in OpenSSL 1.1.1 and
>    seem to work for use in OpenHIP.
>
>
These were nice to haves I think - so no need to hurt your students trying
to crowbar them in if nothing exists for them.

>
>    1.
>    2. In section 6 of
>    https://www.ietf.org/id/draft-moskowitz-hip-new-crypto-05.html, there
>    is a comment about how SHAKE, cSHAKE or KMAC could be used as a
>    pseudo-random generator. Should we do something with this information, or
>    is it just there for future reference? Isn't randomness usually handled by
>    the kernel since it has access to higher-quality entropy sources, and if
>    so, is there any place in the OpenHIP code where it would be beneficial to
>    use SHAKE/cSHAKE/KMAC for this purpose?
>
>    3. Should the new encryption- and signature algorithms we are adding
>    be used as default in OpenHIP or just be available as a command line
>    option?
>
> Personally I would put it as a command line option. However others may be
more inclined to default to the newer crypto for those who are just
blasting through the setup. I think if you made them all command line
options then just have the fallback clause be the newer crypto option that
would kill both birds with one stone. I will need to check the code to see
if this is even possible but I assume it would be.

I always assume that the user is in a rush, lazy and ignorant of the best
option (AKA me when I start using my own tools under pressure).

>    *build_tlv_hmac()*? Seeing as this function will then sometimes
>    *build_tlv_mac()*? Or will it be less intrusive to others (that might
>    be used to the name) to not change it even though the name might be
>    misleading? Or should we add a new function called *build_tlv_kmac()*?
>    (Although this might lead to a lot of switch-case statements everywhere
>    that *build_tlv_hmac()* is called)
>
> Rename the function to *build_tlv_mac *and have an input parameter (the
first one preferably) to switch between HMAC and KMAC methods. Use a
constant to denote the two that can be accessed as needed. Then you could
have two lower level functions for each (*build_tlv_kmac* and
*build_tlv_hmac*) to allow for direct calls if needed that are really only
ever used in *build_tlv_mac*.

>
>
>
> --
> Tm-rid mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tm-rid
>

-- 
73's,
Adam T. Wiethuechter
AX Enterprize, LLC