Re: ISSUE-44 and ISSUE-46

On Feb 12, 2014 7:29 PM, "Richard Barnes" <rlb@ipv.sx> wrote:
>
> Generally:
>
> In researching these, I went back and read the WebIDL spec on
dictionaries, and discovered that all dictionary entries are in fact
optional.
> <http://www.w3.org/TR/WebIDL/#idl-dictionaries>
>

This has long been known and is already being addressed. The spec accounts
for them in prose.

> This is something that I don't think the current spec really accounts
for, and might merit being it's own ISSUE.  Comments on 44 and 46 follow.
>
>
> On ISSUE-44:
>
> If we want to do this, I think it could be done without a huge amount of
effort:
> -- Define a string (say, "auto") that can be used as a parameter where an
IV would be
> -- In the encrypt() method, instead of calling with resolve() with the
ciphertext, call it with a dictionary containing the algorithm, key, and
ciphertext.
> -- (Not technically required, but I would do the same thing with the
other "CryptoOperation" methods for symmetry.)
>

CryptoOperation is long since gone. Not sure what you meant here.

> That seems to me to be a cleaner API, even disregarding auto-generation.
 If you're going to do something with a ciphertext or signature value, you
need to have the algorithm and parameters too.  For example, if you're
going to send the ciphertext to someone, you need to send them the IV as
well, so that they can decrypt it.  If the crypto API doesn't pass this
stuff through, then the JS has to store it elsewhere.
>

Why make the implementation do it? We specifically _removed_ reflecting
algorithm back in that it introduces a whole host of unneeded complexities
(parallel interfaces for every dictionary, handling unhandled keys)

> (If there's any concern that the holder of the promise won't be
authorized to have the key/algorithm, you can strip it with one line,
crypto.subtle.encrypt().then(function(result) { return result.ciphertext;
}); )
>
> This is no more complex to implement, since the code that implements the
encryption will need to store the algorithm and IV anyway.
>

It actually introduces significant complexity for both the spec (parallel
interfaces) and the implementation. I'm not at all convinced in the added
value.

Since you didn't address the concerns from last time we discussed this,
during the F2F, I want to highlight the asymmetry it introduces - both for
when the caller explicitly supplies an IV (does it get reflected or not? If
so, you're forcing a buffer copy every time for all users - including those
that have no use for it) and for situations where the IV is mutated (such
as CTR) - do you return the initial IV (needed for the auto case) or the
final IV (+1 or not)? This argument  for simplicity is provably and
needlessly complex, and can justify further undesirable complexity.

>
>
> On ISSUE-46:
>
> This one has three parts.
> 1. Make several fields default to empty ArrayBufferViews
> 2. Make AesGcmParams.iv non-optional
> 3. Make AesGcmParams.tagLength non-optional or provide a default
>
> Number (2) has been implemented.
>
> Reviewing the WebIDL spec has given me some pause about this issue. In
the section on dictionaries, there is the following phrase: "On a given
dictionary value, the presence of each dictionary member is optional."  So
in fact, the algorithms in this document need to handle any of the fields
not being present, either by throwing or by providing a default value.  In
the cases highlighted in the issue, it seems much simpler to define default
values in the WebIDL than to specify them in the text.
>

That still doesn't address nullable.

The latest edits by Mark have made it clear all fields are required. This
again resolves the defaults argument.

Again, I'm strongly opposed to defaults for SubtleCrypto. Its a half
measure based on the argument of making it easier. We should want no half
measures in the spec. The real way to address the usability concern is to
provide a reasonable problem space that a high level API can solve.

It cannot and will not satisfy everyone, which is why we have avoided it.

>
>
>