Re: Review of draft-ecc-09

[Andrey Jivsov <[e-mail]>]

Hello Marco:

On 02/21/2012 01:49 AM, Marko Kreen wrote:
> On Mon, Feb 20, 2012 at 11:24:39PM -0800, Andrey Jivsov wrote:
>> On 02/18/2012 01:51 PM, Marko Kreen wrote:
>>> [ I updated the review with diff from -09.  Thanks for taking
>>>    my comments on the ref section into account. ]
>> Sorry, I was busy in recent days and this is the first time I take
>> close look at your feedback. The credit for -09 should go to Sean
>> Turner, IETF Security Area director.
>
> Heh.  I noticed few old references there, then the new draft
> fixed those without me sending anything.
>
> Note - the links in text are not updated (see [nits] on draft page).
>
>>>>     The point is encoded in MPI format.  The content of the MPI is the
>>>>     following:
>>>>
>>>>          B = B0 || x || y
>>>>
>>>>     where x and y are coordinates of the point P = (x, y), each encoded
>>>>     in big endian format and zero-padded to the underlying field size.
>>> *Then*, they are also padded to byte boundary.  As this is not mentioned
>>> anywhere, it caused me some confusion, because I assumed they already
>>> are on byte boundary, perhaps even power-of-two.
>>>
>>> As it only matters to P-521 keys, the bad assumtions work fine on
>>> P-256 and P-384 keys.  (Basically I assumed P-521 uses 512-bit values...)
>>
>> Note, that what you are suggesting would not work in practice, if I
>> understand your initial confusion correctly. Note that both x and y
>> are taken mod a 521 bit prime. They can be exactly 521 bits, but in
>> 50% of cases, 520 bits. Let's say that either x or y is 521 and
>> another one is 520 bits. How would we know which one of x and y is
>> the longest one?
>
> The text already states "zero-padded to the underlying field".
> So we already are talking about bits in field, not bits
> in actual bignum.
>
> The missing '*then*' part is about rounding field bits.
>
>> The algorithm is the following: given the MPI with the point,
>> * get its size in full bytes ( len=(MPI_bits+7)/8 )
>> * len = len-1 ( to remove the B0 byte )
>> * len = len/2 -- the 'len' is the size of x and y in bytes.
>
> I think it's better to show how to calculate proper size
> from field size, not from MPI size.  Because MPI is "untrusted",
> we need to validate it.
>
> Maybe just add:
>
>     where x and y are coordinates of the point P = (x, y), each encoded
>     in big endian format and zero-padded to the underlying field size,
>     then zero-padded to multiple of 8 if the field size is not multiple of 8.
>

Yes, this makes it better. Here is modified text:

	"The point is encoded in MPI format.  The content of the MPI is the 
following:

	B = B0 || x || y

	where x and y are coordinates of the point P = (x, y), each encoded in 
big endian format and zero-padded to the nearest 8-byte boundary that is 
higher or equal to the underlying field size. For example, for the 521 
bit underlying field each x and y always occupy 66 bytes."

... and adjust the formula bellow in the spec and change the example to 
use P-521 to get 1059 total bit size recorded in the MPI header.

>>> Missing detail: In addition to size check, what other validation must
>>> be done when parsing a point?  This applies to when reading a public key,
>>> but especially when reading incoming ECDH/ECDSA message.  I suggest
>>> adding:  "You must check that the point is on curve." here.
>>
>> I will take it as a suggestion and think how to integrate it, given
>> that this is a general check for ECC and is not specific to OpenPGP.
>> This valid concern may be satisfied through sources in References
>> section.
>
> It's security detail, and it's simple to mention.
>
> The References are rather verbose, so it's easy to miss
> the important details.
>
>>>> 8. EC DH Algorithm (ECDH)
>>>>     The key wrapping method is based on [RFC3394].  KDF produces the
>>>>     AES key that is used as KEK as specified in [RFC3394].  Refer to
>>>>     section 13 for the details regarding the choice of the KEK
>>>>     algorithm, which MUST be one of three AES algorithms.
>>> This is only place which says that KEK *MUST* be AES-only.  (Ignoring 12.2)
>>> This is in conflict with section 12.1 and 13, where non-AES is not
>>> disallowed.  I'm not disagreeing with it, just I think it would be good
>>> to clarify why.
>>>
>>> It might be good idea to disallow cipher_ids<   AES128, but why disallow
>>> Camellia and Twofish?  I could imagine that because the algorithm
>>> is not per-message but comes from key is the reason - you may not
>>> know at key generation time all the people who want to send you messages
>>> and what features their software has.  But I think it would be
>>> good to put the reason in the doc.
>>>
>>> In any case, 12.1 and 13 should be synced with the requirement.
>>
>> I also see the need for clarity here.
>>
>> In reference to compatibility, if the code understands ECC, the
>> document assumes that it must supports AES. This is something
>> OpenPGP RFC 4880 cannot generally rely upon. One of main criteria of
>> this document is simplicity through limited choices, as I recall,
>> something that Ian G advocated. It's much easier to write "use AES"
>> than to define "strong enough" cipher.
>
> cipher_num>  7?
>
>> Suite B and other government
>> standards require AES, so most application will implement it to
>> comply, leaving us with the question why bother at all about the
>> non-AES case?
>>
>> I am leaning toward fixing KEK at AES. There is no backward
>> compatibility issue here. So, the action item for me is to make this
>> choice clear.
>
> Fine by me.
>
>>>>     For convenience, the synopsis of the encoding method is given
>>>>     below, however, this section, [NIST SP800-56A], and [RFC3394] are
>>>>     the normative sources of the definition.
>>>>
>>>>         Obtain authenticated recipient public key R
>>>>         Generate ephemeral key pair {v, V=vG}
>>>>         Compute shared point S = vR;
>>>>         m = symm_alg_ID || session key || checksum || pkcs5_padding;
>>>>         curve_OID_len = (byte)len(curve_OID);
>>>>         Param = curve_OID_len || curve_OID || public_key_alg_ID || 03 ||
>>>>             01 || KDF_hash_ID || AES_alg_ID for AESKeyWrap ||
>>>>            "Anonymous Sender    " || recipient_fingerprint;
>>>>         Z_len = key size for AES_alg_ID to be used with AESKeyWrap
>>>>         Compute Z = KDF( S, Z_len, Param );
>>>>         Compute C = AESKeyWrap( Z, m ) as per [RFC3394]
>>>>         VB = convert point V to octet string
>>>>         Output (MPI(VB) || len(C) || C).
>>>>
>>>>     The decryption is the inverse of the method given.  Note that the
>>>>     recipient obtains the shared secret by calculating
>>>>
>>>>         S = rV = rvG, where (r,R) is the recipient's key pair.
>>>>
>>>>     Consistent with section 5.13 Sym. Encrypted Integrity Protected
>>>>     Data Packet (Tag 18) of [RFC4880], the MDC SHOULD be used anytime
>>>>     symmetric key is protected by ECDH.
>>> Missing detail:  How to generate v?  What requirements it has?
>>> I suggest expanding the second step with:
>>>
>>>       Generate ephemeral key pair {v, V} where V=vG and v is random number
>>>       in range 0<   v<   n  (n - curve modulus)
>> This detail is also related to a question of external reference, but
>> I am glad you raised this question. FIPS 186-3 defines two methods
>> in B.4 on p.61. The idea is this:
>> B.4.1: if you do "v = v' mod order", the input v' must be 64 bits
>> longer than "order".
>> B.4.2: or you can repeatedly getting random bits until "v'<  order"
>> (probably too wasteful for the entropy)
>
> I think the "How" can be left to references.  I just would like
> to see the requirement "0<  v<  n" mentioned here.
>
> RFC6090 Appendix A seems even better reference for this.
>

It seems to reference only method B.4.2. I find that method B.4.1 is 
more practical.

>>> Final note: the section 8 was quite successful at describing ECDH,
>>> how hard would it be to have same level of description of ECDSA here?
>>> At least I would like to see packet format here, even if the
>>> algorithm is not described.
>>
>> Marko, did you realize that it happens that the signature format for
>> ECDSA is identical to DSA?
>
> Well, FIPS 186-3 tells that actual ECDSA is described in ANS X9.62..
>
>> The (r,s) signature pair consists of
>> (true) MPIs in both cases. The ECDSA key is defined in section 9.
>> Overall, ECDSA for OpenPGP is much simpler because it has no
>> complexities like key wrapping. With this in mind, what would you
>> like to see added to properly implement ECDSA.
>
> At minimum, it should mention that
>
>    Algorithm-Specific Fields for ECDSA signatures are:
>       - MPI of ECDSA value r.
>       - MPI of ECDSA value s.
>
> then the spec makes clear the mapping between packet layout
> and algorithm.
>
> I would like to see short algorithm description also here,
> with symbols consistent with rest of the document.
>
> But it's proably OK having good reference in the form "OpenPGP variant
> of ECDSA is described here: [REF]".  And if the REF describes 4
> different EC signature variants, then point directly to section.
>
> Eg: "How to pad/shorten H(m) for use in ECDSA" is another
> step that needs clear "in OpenPGP we do it like this"
> description.  (shorten: trunc-left/trunc-right/mod.  And how short?)
>

Thank you for your comments.