Re: [Gen-art] Gen-art Last Call/Telechat Review of draft-ietf-ipfix-mib-variable-export-09

[Colin McDowall]

Hi Elwyn,

Many thanks for the review, agree this one is a little dry :-)
Please see below for the replies to your comments. The changes have been made 
for draft 10.

PJ:  Paul Aitken
CM: Colin McDowall

Thanks,
Colin and Paul


>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 wait for direction from your document shepherd or AD before
>posting a new version of the draft.
>
>For more information, please see the FAQ at
>
>.
>
>Document: draft-ietf-ipfix-mib-variable-export-09.txt
>Reviewer: Elwyn Davies
>Review Date: 2015/11/14
>IETF LC End Date: 2015/10/26
>IESG Telechat date: 2015/11/19
>
>Summary: Ready except for some minor nits, chiefly associated with the
>unexplained use of 'magic numbers' that are defined elsewhere in the IPFIX
>specifications (see comments on ss5.3, 5.4.2 and 6.3).  I was (however)
>impressed by the quality of the document, which is consistent in its
>presentation and deals clearly with a complex (and, frankly, pretty dry as
>dust) specification.  Most of the points below are primarily to make the
>document more easily accessible to people (like me) with limited exposure to
>IPFIX. Caveat:  I have read through the examples (Section 6) but I cannot say
>that I have analysed them in gory detail.
>
>Apologies for the late delivery of this review.  I missed the assignment
>during the last call period.  In the light of the quality of the document, I
>don't think this should have any effect on the progress of the document!
>
>Major issues:
>None.
>
>Minor issues:
>None.
>
>Nits/editorial comments:
>General: s/i.e./i.e.,/g (4 instances)
>s/e.g./e.g.,/ (1 instance in s10)

PJ: done.

>s2, para 1:  Probably good to provide a pointer to the doc/section where
>Template Record and Data Record are defined as this section precedes s3 where
>the terminology is specified.

PJ: done.

>s4, para 2: s/non columnar/non-columnar/ (?)

PJ: done.

>s4, third para from end: s/One common type/Two common types/

PJ: done.

>s5, para 5:
>> However, future versions of IPFIX may export the required MIB
>> metadata as part of newer set versions.
>Is the phrase 'newer set versions' a term of art here?  Maybe change to 'newly
>defined version(s)' or maybe 'newly defined IPFIX Set versions [or just Sets]'?

PJ: Each IPFIX Sets has a version number, so "newly defined Set versions".

>s5.1, para after Table 1: s/encoding references/encoding reference/

PJ: done.

>s5.3, bullet points: To clarify Set ID entries in the figures describing the
>various templates/data sets it would be worth noting the SetID(s) that can be
>used with the various template and data records and a reference to RFC 7011
>Section 3.3.2.  I think that bullet 1 has Set ID 2, bullet 2 has Set ID 3 and
>bullets 3 and 4 have Set IDs from 256 upwards (implementation choice).

PJ: done.

>s5.4.2, last para and Figure 5:  It would be helpful to remind people that the
>value 0x/65535 indicates variable length encoding  per RFC 7011 Section
>3.2 and that the RECOMMENDED use of variable length encoding for
>mibObjectIdentifier fields is indicated in subsequent figures by placing 65535
>in the relevant length fields.  Presumably Collector implementations  MUST
>accept a specific length encoding in the usual IETF spirit!  It might be worth
>being explicit about this (this might usefully be said in Section 8).

PJ: done.

>s5.7.2, para 1:  The MUST ought to be qualified by 'except as allowed by the 
>caveat of Section 5.7.1'.

PJ: done.

>s5.7.2: is there any need to explain how withdrawal is achieved?  I am not an
>IPFIX expert so I am not aware how the withdrawal might be achieved.
>
>s5.8, para 5: s/may be used purely use as a data type./may be used purely as a 
>data type./ ( I think)

PJ: done.

>s5.8, last para: is missing its terminal period/full stop. :-(

PJ: done.

>s5.8.1, last para: s/be exported/to be exported/

PJ: done.

>s6.2, bullet 2 after Table 3: is missing its terminal period/full stop. :-(
CM:
Fixed.

>
>s6.2, 3rd from last para (top of page 40): s/encoded/encode/;
CM: Fixed

>It would also be useful to point the reader back to the template for
>mibObjectValueGauge in Table 23 where the encoding size is specified.
CM:
Good idea : Added a reference.

>
>s6.3:  This section is somewhat politically incorrect in that it deals
>(only) with IPv4 addresses ;-)
CM:

Well the OSPFv2 MIB RFC4750 is IPv4 only. Since SNMP has a native type
for IPv4 addresses but not IPv6 it is sort of justified to have an example
using them. Having an IPv6 example as well would be nice but the mechanism works
identically.

>
>s6.3, Table 4 (also Table 9):  The aesthetics of this table could be improved
>by reducing the width of the Object column by 7 characters and reallocating
>them to the ID (+4) and mibObjectValue (+3) columns.  Similarly in Table 5,
>moving a character

[Gen-art] Gen-ART Last Call review of draft-ietf-manet-olsrv2-dat-metric-08

[Paul Kyzivat]

[In progress - not ready for sending]

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-manet-olsrv2-dat-metric-08
Reviewer: Paul Kyzivat
Review Date: TBS
IETF LC End Date: 2015-11-23
IESG Telechat date: 2015-12-03

Summary:

This draft is basically ready for publication as an Experimental RFC, 
but has nits that should be fixed before publication.


Major: 0
Minor: 0
Nits:  5

Nits:

* Section 1:

"Appendix A contains a few possible steps to improve the DAT metric."

This is the first occurrence of the "DAT" acronym. It took me a bit to 
realize this must be referring to "Directional AirTime". Could you 
please define the acronym *before* the first use? (Perhaps in the prior 
paragraph where "Directional Airtime" is first used.)


* Section 2:

"These networks employ link layer retransmission to increase the 
delivery probability and multiple unicast data rates."


I'm not sure how to parse this sentence. Is it intended to be:

"These networks employ link layer retransmission to increase (the 
delivery probability) and (multiple unicast data rates)."


OR "These networks employ link layer retransmission to (increase the 
delivery probability) and (multiple unicast data rates)."


Either way I don't understand what "multiple unicast data rates" means. 
Can you clarify this?


* Section 7:

You call these *constants*, in contrast to the *parameters* defined in 
section 6. But you then suggest conditions under which they could be 
changed. Perhaps they should simply be included with the parameters, but 
with strong warnings about diverging from the recommended values.


Else, it would be good to define these *before* the parameters, because 
that would avoid the forward reference to DAT_MAXIMUM_LOSS.


* Section 9.3/9.4:

Are you considering these to be mutually exclusive? Or is a HELLO 
message processed first by 9.3, and then by 9.4?


Since there is considerable overlap in processing between the two, and 
it would presumably be wrong to do that twice, I guess you must assume 
them to be mutually exclusive. But I presume HELLO messages arrive in 
incoming packets, so 9.3 sounds like it ought to apply to them.


If I guessed right, then I suggest revising 9.3 to start "For each 
incoming packet that is not a HELLO message, ..."


* Section 10.2:

IIUC steps 5.3 & 5.4 are just the hard way of saying:

  bitrate := MAX(L_DAT_rx_bitrate, DAT_MINIMUM_BITRATE)

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

[Gen-art] Gen-ART Last Call review of draft-ietf-dnsop-qname-minimisation-07

[Ralph Droms (rdroms)]

I am the assigned Gen-ART reviewer for 
draft-ietf-dnsop-qname-minimisation-07.txt.

For background on Gen-ART, please see the FAQ at 
.

Please resolve these comments along with any other Last Call comments you may 
receive.


Document: draft-ietf-dnsop-qname-minimisation-07
Reviewer: Ralph Droms
Review Date: 2015-11-20
IETF LC End Date: 2015-11-23
IESG Telechat date: unknown

Summary:

This draft is on the right track but has open issues, described in the review.

Major issues:

The document is well-written and easy to understand.  Thank you.

Has the working group considered publishing this document as "Informational" 
rather than "Experimental"?  If the document is published as "Experimental", is 
the intention to publish a subsequent proposed standard or BCP?

In my opinion, the document needs a little more work if it is to be published 
as "Experimental", especially if the intention is to publish a proposed 
standard based on the results of experiments with the techniques described in 
this document.  I found the descriptions in the document understandable, but 
not quite detailed enough to build an interoperable implementation.

Is Appendix A intended as the specification for the QNAME minimization 
techniques described in this document?  The appendix is titled "An algorithm to 
find the zone cut" and the introductory text indicates the algorithm is 
intended for locating the zone cut.  However, as I read the algorithm, it finds 
and traverses all zone cuts until the original QNAME can be resolved.

If Appendix A is not the specification for the QNAME minimization techniques, 
then I don't know exactly what specific behavior or algorithm is referred to by 
"minimising resolver" in this sentence from section 2:  "The minimising 
resolver works perfectly when it knows the zone cut [...]".

My suggestion would be to include another algorithm description, similar to 
that in Appendix A, but that describes how to use knowledge of zone cuts.

Editorial
-

In section 2, is the phrase "closest known parent of the original QNAME" 
something that most DNS developers would understand?  Would the phrase "closest 
enclosing NS RRset" from Appendix A be more precise?

I wasn't sure at first whether "(section 6)" in the 4th paragraph of section 2 
referred to section 6 of RFC 2181 or section 6 of this document.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

[Gen-art] Gem-Art Last Call review of draft-ietf-dnsop-qname-minimisation-07

[Ralph Droms (rdroms)]

I am the assigned Gen-ART reviewer for 
draft-ietf-dnsop-qname-minimisation-07.txt.

For background on Gen-ART, please see the FAQ at 
.

Please resolve these comments along with any other Last Call comments you may 
receive.


Document: draft-ietf-dnsop-qname-minimisation-07
Reviewer: Ralph Droms
Review Date: 2015-11-20
IETF LC End Date: 2015-11-23
IESG Telechat date: unknown

Summary:

This draft is on the right track but has open issues, described in the review.

Major issues:

The document is well-written and easy to understand.  Thank you.

Has the working group considered publishing this document as "Informational" 
rather than "Experimental"?  If the document is published as "Experimental", is 
the intention to publish a subsequent proposed standard or BCP?

In my opinion, the document needs a little more work if it is to be published 
as "Experimental", especially if the intention is to publish a proposed 
standard based on the results of experiments with the techniques described in 
this document.  I found the descriptions in the document understandable, but 
not quite detailed enough to build an interoperable implementation.

Is Appendix A intended as the specification for the QNAME minimization 
techniques described in this document?  The appendix is titled "An algorithm to 
find the zone cut" and the introductory text indicates the algorithm is 
intended for locating the zone cut.  However, as I read the algorithm, it finds 
and traverses all zone cuts until the original QNAME can be resolved.

If Appendix A is not the specification for the QNAME minimization techniques, 
then I don't know exactly what specific behavior or algorithm is referred to by 
"minimising resolver" in this sentence from section 2:  "The minimising 
resolver works perfectly when it knows the zone cut [...]".

My suggestion would be to include another algorithm description, similar to 
that in Appendix A, but that describes how to use knowledge of zone cuts.

Editorial
-

In section 2, is the phrase "closest known parent of the original QNAME" 
something that most DNS developers would understand?  Would the phrase "closest 
enclosing NS RRset" from Appendix A be more precise?

I wasn't sure at first whether "(section 6)" in the 4th paragraph of section 2 
referred to section 6 of RFC 2181 or section 6 of this document.



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Re: [Gen-art] Gen-ART Last Call review of draft-josefsson-scrypt-kdf-03

[Paul Kyzivat]

I did want to comment on one other thing:


>>> (Issue-4): Identifiers reused for different meanings


In scrypt, "B" is an array of "p" vectors, each of which is 128*r
octets. In scryptROMix, "B" is a single vector of 128*r octets. In
scryptBlockMix, "B" is a vector of 2*r 64-octet blocks.


I'm not sure that changing variable names here is a good idea.  For any
performant implementation, these variables will refer to the same memory
area and thus a consistent variable name helps to signal that.  It is
just the interpretation of that memory area that is different in these
functions.



The document would be clearer if distinct identifiers were used for
each unique concept.


I believe the identifier refers to unique concepts.  For B, what differs
is how that memory area is interpreted in each algorithm description.


I don't think it works for a *spec* to assume that a "memory area" can 
be interpreted differently, sometimes as a number, sometimes as an array 
of bytes, sometimes as an array of arrays, etc.


Making such assumptions depends on the language being used mapping these 
things to the same memory in a consistent way. Some languages don't 
allow this at all, and those that do often lay out memory with padding 
for efficiency, according to little/big endianness, etc.


So I think it is important to clearly define any type coercions that are 
needed.


Thanks,
Paul

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

Re: [Gen-art] Gen-ART Last Call review of draft-josefsson-scrypt-kdf-03

[Paul Kyzivat]

On 11/20/15 10:22 AM, Stephen Farrell wrote:


(This combines the distributions for Simon's mail to gen-art
and secdir. I think one thread responding to my question(s)
below will work better.)


+1

I was out of my depth reviewing this because I am not familiar with 
common practice in defining security algorithms. I reviewed this from 
the perspective of a software engineer and found it very difficult to 
follow.


So it is probably better if secdir looks at my comments and decides if 
they have any merit and need further attention.


Thanks,
Paul


Hi Simon, all,

Thanks for the reviews and updates.

Does anyone think we need more review for this or is
it now ready for IESG eval? Modulo one question below,
I think it is ready to move forward, but there are a
lot of detailed changes in this revision, so it might
be prudent to try get a few eyeballs on those.

One other thing below...

On 20/11/15 12:29, Simon Josefsson wrote:

Executive Summary: I have submitted
https://tools.ietf.org/html/draft-josefsson-scrypt-kdf-04
that (hopefully) resolve the gen-art and secdir feedback.

Paul Kyzivat  writes:


This draft is on the right track but has open issues, described in the
review.


Hello Paul.  I am sorry for the delay in answering your thorough (much
appreciated!) review.  Your review was emailed to the wrong
@tools.ietf.org address, but I can't blame that for my delay since I was
notified of your review through the secdir review.


(Issue-1): Intended status

The intended status of this document is Informational. I question why
it is not a normative document. As best I can tell, this is the formal
specification of the algorithm. Those that use it would presumably
want to claim conformance to it. The introduction describes this as an
alternative to other KDF functions, including only one with an RFC
reference - RFC2898. That one is also informational, but it is a
restatement of an algorithm specified elsewhere, so that RFC can be
viewed as an informational supplement to the actual definition. The
same is not true of this document.

(Of course changing this to a normative document would require
significant changes, including adding 2119 language. And it probably
could then not be handled as an AD-sponsored document.)


As I understand it, the IETF tradition is that descriptions of crypto
algorithms that were invented elsewhere are not published as Standards
Tracks documents.  So this is in line with that tradition.


(Issue-2): Type mismatch on call to scryptROMix

The scryptROMix function calls scryptBlockMix with an octet vector of
length 128*r octets. But the definition of scryptBlockMix specifies
that the input argument should be a vector of 2*r 64-octet blocks.

Clearly these don't match. One way to make them match would be to
divide the single 128*r octet vector into two 64-octet vectors, and
then to treat r as 2 inside of scryptBlockMix. I don't know if that is
the intent.


I believe this is a presentation issue.  The intention is indeed that
conversion between these formats is transparent.  In performant
implementations, B will refers to the same memory area.  The document
was confusing here, and I believe the document was further problematic
since it described the inputs as an array of elements, rather than a
concatenation of octets.  It now reads:

Input:
 B[0] || B[1] || ... || B[2 * r - 1]
Input octet string (of size 128 * r octets),
treated as 2 * r 64-octet blocks.

Output:
 B'[0] || B'[1] || ... || B'[2 * r - 1]
Output octet string.

I hope this improves this aspect.


(Issue-3): Definition of Integerify

The Integerify function is not clearly defined. The following is given
in the document:

   j = Integerify (X) mod N
   where Integerify (B[0] ... B[2 * r - 1]) is defined
   as the result of interpreting B[2 * r - 1] as a
   little-endian integer.

I can make no sense of this definition of Integerify. The description
implies that B must be an array containing elements up to index
2*r-1. But the definition of B is "Input octet vector of length 128 *
r octets". Taking the definition literally, B[2*r-1] must be an octet,
and 2*r must be less than 128. That seems like nonsense to me.

I found the following in the [SCRYPT] paper:

"We expect that for reasons of performance and simplicity,
implementors will restrict N to being a power of 2, in which case
the function Integerify can be replaced by reading the first (or
last) machine-length word from a k-bit block."

Simply reading a machine-length word ignores the differences between
little-endian and big-endian machines, and machines with different
word sizes. Conveniently, [SALSA20SPEC] defines a littleendian
function that yields a 32-bit integer from four bytes. That should be
sufficient bits for computing "j". So Integerify(X) could be defined
as:

littleendian(X[0],X[1],X[2],X[3])

or

littleendian(X[128*r-4],X[128

Re: [Gen-art] Gen-ART Last Call review of draft-josefsson-scrypt-kdf-03

[Stephen Farrell]

(This combines the distributions for Simon's mail to gen-art
and secdir. I think one thread responding to my question(s)
below will work better.)

Hi Simon, all,

Thanks for the reviews and updates.

Does anyone think we need more review for this or is
it now ready for IESG eval? Modulo one question below,
I think it is ready to move forward, but there are a
lot of detailed changes in this revision, so it might
be prudent to try get a few eyeballs on those.

One other thing below...

On 20/11/15 12:29, Simon Josefsson wrote:
> Executive Summary: I have submitted
> https://tools.ietf.org/html/draft-josefsson-scrypt-kdf-04
> that (hopefully) resolve the gen-art and secdir feedback.
> 
> Paul Kyzivat  writes:
> 
>> This draft is on the right track but has open issues, described in the
>> review.
> 
> Hello Paul.  I am sorry for the delay in answering your thorough (much
> appreciated!) review.  Your review was emailed to the wrong
> @tools.ietf.org address, but I can't blame that for my delay since I was
> notified of your review through the secdir review.
> 
>> (Issue-1): Intended status
>>
>> The intended status of this document is Informational. I question why
>> it is not a normative document. As best I can tell, this is the formal
>> specification of the algorithm. Those that use it would presumably
>> want to claim conformance to it. The introduction describes this as an
>> alternative to other KDF functions, including only one with an RFC
>> reference - RFC2898. That one is also informational, but it is a
>> restatement of an algorithm specified elsewhere, so that RFC can be
>> viewed as an informational supplement to the actual definition. The
>> same is not true of this document.
>>
>> (Of course changing this to a normative document would require
>> significant changes, including adding 2119 language. And it probably
>> could then not be handled as an AD-sponsored document.)
> 
> As I understand it, the IETF tradition is that descriptions of crypto
> algorithms that were invented elsewhere are not published as Standards
> Tracks documents.  So this is in line with that tradition.
> 
>> (Issue-2): Type mismatch on call to scryptROMix
>>
>> The scryptROMix function calls scryptBlockMix with an octet vector of
>> length 128*r octets. But the definition of scryptBlockMix specifies
>> that the input argument should be a vector of 2*r 64-octet blocks.
>>
>> Clearly these don't match. One way to make them match would be to
>> divide the single 128*r octet vector into two 64-octet vectors, and
>> then to treat r as 2 inside of scryptBlockMix. I don't know if that is
>> the intent.
> 
> I believe this is a presentation issue.  The intention is indeed that
> conversion between these formats is transparent.  In performant
> implementations, B will refers to the same memory area.  The document
> was confusing here, and I believe the document was further problematic
> since it described the inputs as an array of elements, rather than a
> concatenation of octets.  It now reads:
> 
>Input:
> B[0] || B[1] || ... || B[2 * r - 1]
>Input octet string (of size 128 * r octets),
>treated as 2 * r 64-octet blocks.
> 
>Output:
> B'[0] || B'[1] || ... || B'[2 * r - 1]
>Output octet string.
> 
> I hope this improves this aspect.
> 
>> (Issue-3): Definition of Integerify
>>
>> The Integerify function is not clearly defined. The following is given
>> in the document:
>>
>>   j = Integerify (X) mod N
>>   where Integerify (B[0] ... B[2 * r - 1]) is defined
>>   as the result of interpreting B[2 * r - 1] as a
>>   little-endian integer.
>>
>> I can make no sense of this definition of Integerify. The description
>> implies that B must be an array containing elements up to index
>> 2*r-1. But the definition of B is "Input octet vector of length 128 *
>> r octets". Taking the definition literally, B[2*r-1] must be an octet,
>> and 2*r must be less than 128. That seems like nonsense to me.
>>
>> I found the following in the [SCRYPT] paper:
>>
>> "We expect that for reasons of performance and simplicity,
>> implementors will restrict N to being a power of 2, in which case
>> the function Integerify can be replaced by reading the first (or
>> last) machine-length word from a k-bit block."
>>
>> Simply reading a machine-length word ignores the differences between
>> little-endian and big-endian machines, and machines with different
>> word sizes. Conveniently, [SALSA20SPEC] defines a littleendian
>> function that yields a 32-bit integer from four bytes. That should be
>> sufficient bits for computing "j". So Integerify(X) could be defined
>> as:
>>
>>littleendian(X[0],X[1],X[2],X[3])
>>
>> or
>>
>>littleendian(X[128*r-4],X[128*r-3],X[128*r-2],X[128*r-1])
>>
>> (I don't think it matters which, as long as everyone does it the same way.)
>>
>> In any case, the language is ambiguous and needs to be clarified.
> 
> Right.  I have

Re: [Gen-art] Gen-ART Last Call review of draft-josefsson-scrypt-kdf-03

[Simon Josefsson]

Executive Summary: I have submitted
https://tools.ietf.org/html/draft-josefsson-scrypt-kdf-04
that (hopefully) resolve the gen-art and secdir feedback.

Paul Kyzivat  writes:

> This draft is on the right track but has open issues, described in the
> review.

Hello Paul.  I am sorry for the delay in answering your thorough (much
appreciated!) review.  Your review was emailed to the wrong
@tools.ietf.org address, but I can't blame that for my delay since I was
notified of your review through the secdir review.

> (Issue-1): Intended status
>
> The intended status of this document is Informational. I question why
> it is not a normative document. As best I can tell, this is the formal
> specification of the algorithm. Those that use it would presumably
> want to claim conformance to it. The introduction describes this as an
> alternative to other KDF functions, including only one with an RFC
> reference - RFC2898. That one is also informational, but it is a
> restatement of an algorithm specified elsewhere, so that RFC can be
> viewed as an informational supplement to the actual definition. The
> same is not true of this document.
>
> (Of course changing this to a normative document would require
> significant changes, including adding 2119 language. And it probably
> could then not be handled as an AD-sponsored document.)

As I understand it, the IETF tradition is that descriptions of crypto
algorithms that were invented elsewhere are not published as Standards
Tracks documents.  So this is in line with that tradition.

> (Issue-2): Type mismatch on call to scryptROMix
>
> The scryptROMix function calls scryptBlockMix with an octet vector of
> length 128*r octets. But the definition of scryptBlockMix specifies
> that the input argument should be a vector of 2*r 64-octet blocks.
>
> Clearly these don't match. One way to make them match would be to
> divide the single 128*r octet vector into two 64-octet vectors, and
> then to treat r as 2 inside of scryptBlockMix. I don't know if that is
> the intent.

I believe this is a presentation issue.  The intention is indeed that
conversion between these formats is transparent.  In performant
implementations, B will refers to the same memory area.  The document
was confusing here, and I believe the document was further problematic
since it described the inputs as an array of elements, rather than a
concatenation of octets.  It now reads:

   Input:
B[0] || B[1] || ... || B[2 * r - 1]
   Input octet string (of size 128 * r octets),
   treated as 2 * r 64-octet blocks.

   Output:
B'[0] || B'[1] || ... || B'[2 * r - 1]
   Output octet string.

I hope this improves this aspect.

> (Issue-3): Definition of Integerify
>
> The Integerify function is not clearly defined. The following is given
> in the document:
>
>   j = Integerify (X) mod N
>   where Integerify (B[0] ... B[2 * r - 1]) is defined
>   as the result of interpreting B[2 * r - 1] as a
>   little-endian integer.
>
> I can make no sense of this definition of Integerify. The description
> implies that B must be an array containing elements up to index
> 2*r-1. But the definition of B is "Input octet vector of length 128 *
> r octets". Taking the definition literally, B[2*r-1] must be an octet,
> and 2*r must be less than 128. That seems like nonsense to me.
>
> I found the following in the [SCRYPT] paper:
>
> "We expect that for reasons of performance and simplicity,
> implementors will restrict N to being a power of 2, in which case
> the function Integerify can be replaced by reading the first (or
> last) machine-length word from a k-bit block."
>
> Simply reading a machine-length word ignores the differences between
> little-endian and big-endian machines, and machines with different
> word sizes. Conveniently, [SALSA20SPEC] defines a littleendian
> function that yields a 32-bit integer from four bytes. That should be
> sufficient bits for computing "j". So Integerify(X) could be defined
> as:
>
>littleendian(X[0],X[1],X[2],X[3])
>
> or
>
>littleendian(X[128*r-4],X[128*r-3],X[128*r-2],X[128*r-1])
>
> (I don't think it matters which, as long as everyone does it the same way.)
>
> In any case, the language is ambiguous and needs to be clarified.

Right.  I have changed this into:

  j = Integerify (X) mod N
  where Integerify (X) is defined as the result of
  interpreting the last four octets of X as a little-
  endian integer, i.e.:
  littleendian(X[128*r-4], X[128*r-3],
   X[128*r-2], X[128*r-1])

> -
> Minor issues:
>
> (Issue-4): Identifiers reused for different meanings
>
> In scrypt, "B" is an array of "p" vectors, each of which is 128*r
> octets. In scryptROMix, "B" is a single vector of 128*r octets. In
> scryptBlockMix, "B" is a vector of 2*r 64-octet blocks.

I'm not sure that changing variable names here is a goo