Subject: [Hipsec] Implementation questions from students

From: Andrei Gurtov <[email-masked]>
Date: Tue, 13 Oct 2020 16:32:23 +0200

Hello,

Some questions from a group of students working on HIPv2 implementation.
I guess mostly to OpenHIP. A few were already answered on DRIP list.

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: Jeff Ahrenholz <[email-masked]>
Date: Wed, 14 Oct 2020 20:29:59 +0000

Hi Andrei et al.,

Some inline comments below...

> 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:
> memcpy( hmac->hmac,
>         &hmac_md[hmac_md_len - sizeof(hmac->hmac)],
>         sizeof(hmac->hmac));

We currently use the HMAC() function writing directly into the TLV buffer. We're not truncating the output hash when using AES-256.

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

This likely stems from the original RFC 5201 / RFC 5202 required algorithms. You're correct, with AES-256 and others you do not need to truncate the output hash. Although that is allowed by old RFCs such as RFC 2104.

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

Yes, I think you should remove the silent fallback.

You should be able to use the OGA ID field of the HIT in order to validate the HIT.
From RFC 7401 section 3.1:

"  One of these
   measures allows different hash functions for creating a HIT.  HIT
   Suites group the sets of algorithms that are required to generate and
   use a particular HIT.  The Suites are encoded in HIT Suite IDs.
   These HIT Suite IDs are transmitted in the ORCHID Generation
   Algorithm (OGA) field in the ORCHID.  With the HIT Suite ID in the
   OGA ID field, a host can tell, from another host's HIT, whether it
   supports the necessary hash and signature algorithms to establish a
   HIP association with that host."

The HIT_SUITE_LIST TLV is used to list the supported HIT SUITE IDs.

See RFC 7401 section 5.2.10 around this text:

"   The ID field in the HIT_SUITE_LIST is defined as an eight-bit field,
   as opposed to the four-bit HIT Suite ID and OGA ID field in the
   ORCHID."

I don't think you should parse the HIT_SUITE_LIST when validating the HIT.

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

The new name build_tlv_mac() sounds good.

regards,
-Jeff