Unbound 1.9.6rc1 pre-release

[Wouter Wijngaards]

Hi Paul,

On 06/12/2019 01:18, Paul Wouters via Unbound-users wrote:
> On Thu, 5 Dec 2019, Paul Wouters via Unbound-users wrote:
> 
>> Seems sldns_wire2str_ipseckey_scan() was changed from:
>>
>> int sldns_wire2str_ipseckey_scan(uint8_t** d, size_t* dl, char** s,
>> size_t* sl,
>>         uint8_t* pkt, size_t pktlen)
>>
>> to:
>>
>> int sldns_wire2str_ipseckey_scan(uint8_t** d, size_t* dl, char** s,
>> size_t* sl,
>>         uint8_t* pkt, size_t pktlen, int* comprloop)
>>
>> But the only call to it was not updated for the added argument comprloop.
>>
>> It looks like the variable is to prevent infinite cname/dname
>> loops?
> 
> I was wrong :)
> 
> Seems to be handling name compression. I guess the variable name "dname"
> in the code predates the DNAME RRTYPE :P
> 
> Since RFC 4025 for IPSECKEY states:
> 
> 3  A wire-encoded domain name is present.  The wire-encoded format is
>       self-describing, so the length is implicit.  The domain name MUST
>       NOT be compressed.  (See Section 3.3 of RFC 1035 [2].)
> 
> I guess we can just pass in an extra 0?

Yes this is the solution I picked too (I read Tuomo's email before I
read your email).  That should be fine because there is no packet here
for the dname loop issue to happen, and the memory area referenced
contains uncompressed dnames (as well as the RFC says it should, but I
mean, if it was, it would have been decompressed).  The function
argument is there for use when there is a packet and potential
compression (even if the RFC does not allow it, the robust defence
sounds like it is not a bad idea).  So, really, it could have also been
solved in the way you described earlier, because the RFC says no
compression, but it would have been less lenient and robustness for that
lenience.

Best regards, Wouter

> 
> diff -Naur unbound-1.9.6rc1/ipsecmod/ipsecmod.c
> unbound-1.9.6rc1-new/ipsecmod/ipsecmod.c
> --- unbound-1.9.6rc1/ipsecmod/ipsecmod.c    2019-12-05
> 10:26:37.000000000 +0000
> +++ unbound-1.9.6rc1-new/ipsecmod/ipsecmod.c    2019-12-06
> 00:07:45.319917335 +0000
> @@ -337,7 +337,7 @@
>          /* Save the buffer pointers. */
>          tempstring = s; tempstring_len = slen;
>          w_temp = sldns_wire2str_ipseckey_scan(&tempdata, &tempdata_len,
> &s,
> -            &slen, NULL, 0);
> +            &slen, NULL, 0, 0);
>          /* There was an error when parsing the IPSECKEY; reset the buffer
>           * pointers to their previous values. */
>          if(w_temp == -1) {
> diff -Naur unbound-1.9.6rc1/sldns/wire2str.c
> unbound-1.9.6rc1-new/sldns/wire2str.c
> --- unbound-1.9.6rc1/sldns/wire2str.c    2019-12-05 10:26:37.000000000
> +0000
> +++ unbound-1.9.6rc1-new/sldns/wire2str.c    2019-12-06
> 00:08:14.938307559 +0000
> @@ -1544,7 +1544,7 @@
>  static int sldns_wire2str_ipseckey_scan_internal(uint8_t** d, size_t* dl,
>      char** s, size_t* sl, uint8_t* pkt, size_t pktlen, int* comprloop)
>  {
> -    /* http://www.ietf.org/internet-drafts/draft-ietf-ipseckey-rr-12.txt*/
> +    /* https://tools.ietf.org/html/rfc4025 */
>      uint8_t precedence, gateway_type, algorithm;
>      int w = 0;
> 
> Paul