[noise] NoiseSocket API feedback

[Nemanja Mijailovic]

I think it may be difficult to define the API that is easy to directly implement in all programming languages. The spec itself is clear enough, so I wouldn’t miss the API section much if we remove it. Also, we will soon have two implementations that are different enough so people can just take a look at the code if they need ideas for the API.

Nemanja

> On Apr 19, 2018, at 08:44, Trevor Perrin <trevp at trevp.net> wrote:
> 
> Thanks Nemanja
> 
> These comments make sense and I agree with most of them.
> 
> There's a larger question of whether we should even define an API
> here?  I'm not sure how useful it is?
> 
> 
> On Wed, Apr 18, 2018 at 8:08 PM, Nemanja Mijailovic
> <metalnem at mijailovic.net> wrote:
>> 
>> 1) Initialize
>> 
>> Current definition:
>> - INPUT: pattern, dh, cipher, hash
>> - OUTPUT: session object
>> 
>> It's strange that not all inputs are specified here. I would either mirror
>> the definition of Initialize in HandshakeState from the Noise specification:
>> INPUT: handshake_pattern, initiator, prologue, s, e, rs, re
>> 
>> Or maybe just say something more abstract like:
>> INPUT: noise_protocol
> 
> Second option makes sense:  there's no need to list all the parameters
> of a Noise protocol in Initialize and Reinitialize, particularly
> because it might change over time (e.g. signatures as well as DH, or
> different ways of doing symmetric crypto).
> 
> 
>> I think that the OUTPUT is not necessary here, so I would just remove
>> it—it's not clear what is the meaning of "session object", and it's not
>> mentioned anywhere else in the specification (except in Reinitialize).
> 
> Makes sense too.
> 
> 
>> 2) ReadHandshakeMessage
>> 
>> ReadHandshakeMessage takes handshake_message as a parameter. This may lead
>> the implementer to believe that the parameter is a byte sequence that
>> contains the whole message that the user has somehow obtained prior to the
>> call. In practice, you will most likely pass the input stream as a
>> parameter, and read the handshake_message from it (when I say stream, I mean
>> something that you can read a sequence of one or more bytes from, like Go's
>> io.Reader, or Java's InputStream). Maybe someting like this would be
>> clearer:
>> 
>> ReadHandshakeMessage:
>> 
>> - INPUT: input_stream
> 
> 
> Maybe.  Would you also replace WriteMessage and ReadMessage with Write
> and Read, that just operate on a stream?  Would that get more
> complicated, e.g. would we assume the library might do buffering and
> have a flush() function?
> 
> 
>> 3) PeekHandshakeMessage
>> 
>> PeekHandshakeMessage will probably completely consume the negotiation data,
>> so I would just rename it to ReadNegotiationData.
> 
> Makes sense.
> 
> 
>> 4) WriteEmptyHandshakeMessage
>> 
>> It does exactly the same thing as the WriteHandshakeMessage when the
>> message_body is omitted, so I would just drop this one.
> 
> WriteEmptyHandshakeMessage only does the exact same thing as
> WriteHandshakeMessage if the underlying Noise protocol doesn't encrypt
> the handshake payload.  Otherwise the zero-length message body would
> get padded and encrypted (with WriteHandshakeMessage).
> 
> So if Bob was able to negotiate a key with Alice but still wants to
> reject her initial message, he would use WriteEmptyHandshakeMessage,
> so I think we should keep it, or something like it.
> 
> Trevor