[noise] Test Vector Specification

[Trevor Perrin]

On Wed, Nov 22, 2017 at 2:45 AM, Alex <alex at centromere.net> wrote:
>> >
>> > In general, do you think it would be best to change the `fail` key
>> > from a boolean to a string whose value indicates the type of failure
>> > expected? In keeping with your example:
>> >
>> > "fail": "invalid_public_key"
>>
>> Not sure about this.  Implementations might not provide granular error
>> messages, and there are reasons to avoid fine-grained error messages
>> (thinking of the Bleichenbacher or Vaudenay attacks on TLS).
>> Tentatively I think just indicating "this message is allowed to fail"
>> / "required to fail" / "must succeed" seems enough.
>>
>
> I can understand why you might not want to communicate to a remote
> party what kind of error occurred. However, shouldn't that be the
> responsibility of the application, not the underlying implementation?

Yes.  Though I'm a tiny bit worried that encouraging fine-grained
error messages in the library makes it more likely they'll end up sent
over the network, or creating some observable timing difference.


> In other words, I want to create a Noise implementation which returns
> extremely specific error messages to the application. It's the
> application's responsibility not to communicate them outward. I think
> this would make debugging easier and more transparent.

That's reasonable too, so I guess there's pros and cons to
fine-grained library error messages.  And the crypto we're using
shouldn't be susceptible to things like Vaudenay or Bleichenbacher,
anyways.

I guess if we add the type of failure into the test vectors, like
you're suggesting, then implementations with fine-grained error
messages can check it, but other implementations can just check that
*some* error occurred, so this works for everyone?


>> > If we choose to adopt failure cases, I think each case should be
>> > represented in the test vector spec as a first-class citizen,
>> > including whether or not the failure is optional/ignorable. The
>> > failure cases I can think of are:
>> >
>> > 1. Invalid public key (ignorable)
>> > 2. Static key overwrite -- the remote party sent a static key when
>> > it was already provided by the local party (fatal)
>> > 3. Decryption error -- a key or payload failed to decrypt (fatal)
>> > 4. Key missing -- a particular key is required by the given
>> > handshake pattern but was not provided (fatal)
>>
>> Steps 2 and 4 are testing the API more than protocol implementation.
>> I.e. the test vector's arguments are immediately invalid given the
>> pattern, and this is detectable before any messages are sent.
>>
>
> This is true.
>
>> Hadn't thought about this before.  Is this an invalid test vector
>> (don't do it), or do we allow such vectors to ensure the
>> implementation's API can detect misuse?
>>
>
> Defined failure cases also ensure that an implementation's API throws
> the right kind of exception under the circumstances.

Yeah, I guess I don't mind this, as long as we indicate with the
failure message or something what is being tested.


>> > If `[init|resp]_remote_ephemeral` is provided, would that obviate
>> > the need for `fallback` and `fallback_pattern`?
>>
>> Yes, at the level of testing individual, "linear" Noise protocols.
>>
>> No, if you want to test a complete "Noise Pipes" IK->XXfallback or
>> some other compound protocol.
>>
>> But "compound protocols" are a work in progress (see the recent
>> NoiseSocket, NoiseLink) threads.  So it might be best to leave that
>> out of scope in this test vector format, and we can have a separate
>> NoiseSocket / NoiseLink vectors spec later, that tests more
>> complicated / compound cases.
>>
>
> Ok. If I understand correctly, you want me to remove the `fallback` and
> `fallback_pattern` keys, and you want me to /not/ add
> `[init|resp]_remote_ephemeral`. Correct?

Sorry to be unclear - I was proposing to remove the "fallback" and
"fallback_pattern" keys, but add the remote_ephemeral ones, so we
could test things like XXfallback in an individual test vector.

Trevor