Re: [Gen-art] Genart last call review of draft-ietf-dnsop-serve-stale-07

2019-12-04 Thread Alissa Cooper
Brian, thanks for your review. Puneet, thanks for your response and for 
updating the draft accordingly. I entered a No Objection ballot.

Alissa


> On Sep 18, 2019, at 12:20 PM, Puneet Sood 
>  wrote:
> 
> NOTE: Responding to the TTL concerns raised in multiple threads
> (thanks Viktor Dukhovni, Tony Finch) here since it seems to cover all
> the lists.
> 
> 
> On Mon, Sep 16, 2019 at 10:24 PM Brian Carpenter via Datatracker
>  wrote:
>> 
>> Reviewer: Brian Carpenter
>> Review result: Ready with Issues
>> 
>> Gen-ART Last Call review of draft-ietf-dnsop-serve-stale-07
>> 
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>> 
>> For more information, please see the FAQ at
>> .
>> 
>> Document: draft-ietf-dnsop-serve-stale-07.txt
>> Reviewer: Brian Carpenter
>> Review Date: 2019-09-17
>> IETF LC End Date: 2019-09-25
>> IESG Telechat date:
>> 
>> Summary: Ready with issue
>> 
>> 
>> Major issues:
>> -
>> 
>> "(It [RFC2181] also has the curious suggestion that a value in the
>> range 2147483648 to 4294967295 should be treated as zero.)"
>> 
>> I don't see why that is "curious". That is the range of unsigned
>> 32-bit integers that would be negative if treated as signed 32-bit
>> integers. And in any case, the statement seems unfair to the authors
>> of RFC2181, which actually says this:
>> 
>> "It is
>> hereby specified that a TTL value is an unsigned number, with a
>> minimum value of 0, and a maximum value of 2147483647... this value
>> shall be encoded in the less significant 31 bits..."
>> 
>> It's not a "suggestion"; since the RFC does not use RFC2119 keywords,
>> it's a requirement. It's unambiguous, and obviously motivated by
>> resilience to coding errors around signed vs unsigned integers. That
>> was a legitimate concern in 1997. As best as I can tell, in 1997
>> standard C did not include uint32; you needed to use unsigned long int
>> and hope it was mapped to 32 bits.
>> 
>> So the current draft overrides this choice made in RFC2181. Since that
>> choice had an obvious (but unstated) motivation, how do we know that
>> allowing TTLs above 2147483647 will not trigger long-standing coding
>> bugs?
>> 
>> There's an alarm bell later in the draft:
>> 
>> "Regarding the TTL to set on stale records in the response,
>> historically TTLs of zero seconds have been problematic for some
>> implementations, and negative values can't effectively be
>> communicated to existing software."
>> 
>> You bet. If the TTL is specified as an unsigned 32 bit integer, and
>> stored in a uint32, negative values are impossible. But if the coding
>> is sloppy and used long int, "if ttl > 0" could go horribly wrong.
>> 
>> Maybe it's all OK and there is no old resolver code out there with coding
>> errors for values above 2147483647. Did the WG discuss this? I think the
>> "curious suggestion" text above should be replaced by a brief discussion
>> of why RFC2181 made the change that it did, and why it's now safe to
>> reverse it.
>> 
>> 
> 
> The primary concern is around treating TTL values with the high bit
> set as positive numbers resulting in really long TTL value either
> intentionally (someone added that to a zone configuration) or
> unintentionally (e.g. software incorrectly decrementing a zero value
> or small positive value). The responses from Tony and Brian clarify
> why the wording in RFC 2181 is written the way it is. My co-authors
> have more context on the changes for TTL interpretation. I will follow
> up and we will come back with a response.
> 
> I will also address the editorial comments around the TTL text with tat.
> 
> Thanks,
> Puneet
> 
> ___
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art

___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art


[Gen-art] Genart last call review of draft-ietf-dnsop-serve-stale-07

2019-09-16 Thread Brian Carpenter via Datatracker
Reviewer: Brian Carpenter
Review result: Ready with Issues

Gen-ART Last Call review of draft-ietf-dnsop-serve-stale-07

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at
.

Document: draft-ietf-dnsop-serve-stale-07.txt
Reviewer: Brian Carpenter
Review Date: 2019-09-17
IETF LC End Date: 2019-09-25
IESG Telechat date:  

Summary: Ready with issue


Major issues:
-

"(It [RFC2181] also has the curious suggestion that a value in the
range 2147483648 to 4294967295 should be treated as zero.)"

I don't see why that is "curious". That is the range of unsigned
32-bit integers that would be negative if treated as signed 32-bit
integers. And in any case, the statement seems unfair to the authors
of RFC2181, which actually says this:

"It is
hereby specified that a TTL value is an unsigned number, with a
minimum value of 0, and a maximum value of 2147483647... this value
shall be encoded in the less significant 31 bits..."

It's not a "suggestion"; since the RFC does not use RFC2119 keywords,
it's a requirement. It's unambiguous, and obviously motivated by
resilience to coding errors around signed vs unsigned integers. That
was a legitimate concern in 1997. As best as I can tell, in 1997
standard C did not include uint32; you needed to use unsigned long int
and hope it was mapped to 32 bits.

So the current draft overrides this choice made in RFC2181. Since that
choice had an obvious (but unstated) motivation, how do we know that
allowing TTLs above 2147483647 will not trigger long-standing coding
bugs?

There's an alarm bell later in the draft:

"Regarding the TTL to set on stale records in the response,
historically TTLs of zero seconds have been problematic for some
implementations, and negative values can't effectively be
communicated to existing software."

You bet. If the TTL is specified as an unsigned 32 bit integer, and
stored in a uint32, negative values are impossible. But if the coding
is sloppy and used long int, "if ttl > 0" could go horribly wrong.

Maybe it's all OK and there is no old resolver code out there with coding
errors for values above 2147483647. Did the WG discuss this? I think the
"curious suggestion" text above should be replaced by a brief discussion
of why RFC2181 made the change that it did, and why it's now safe to
reverse it.


___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art