[prev in list] [next in list] [prev in thread] [next in thread] 

List:       moderncrypto-noise
Subject:    Re: [noise] Query about the definition of CipherState.encryptWithAd
From:       Trevor Perrin <trevp () trevp ! net>
Date:       2020-05-02 10:01:55
Message-ID: CAGZ8ZG1QrkD-yzgV83j6JXQidsvwP5rdetwQosA1WTOf=mhqbQ () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Fri, May 1, 2020 at 8:35 AM Mike Hearn <mike@plan99.net> wrote:

> During a code review a colleague flagged an issue that I don't have a
> great answer for.
>
> The Noise spec requires that the EncryptWithAd operation might not
> actually encrypt, if it's called before the key is set. This seems
> surprising and potentially a source of subtle bugs. I'd have expected an
> error to be signalled if you attempt an encryption or decryption operation
> without a key.
>
> It appears it's defined this way to make WriteMessage simpler when
> processing an initial key in the first part of a handshake, before any DH
> operation has run. Everything being written out can be passed through
> EncryptAndHash without a special case for the position where no key is
> available. But translated directly to code this results in a rather odd
> exception inside the core encryption codepath which just looks all wrong.
>

Hi Mike,

Yeah, we did it this way to look simpler in ReadMessage / WriteMessage but
I agree it looks weird to have a function named "EncryptWithAd" that might
not encrypt.  I think this is unlikely to be a source of bugs because
handshake processing is so simple and rigid it should be easy to test
exhaustively with our test vectors.

But still, we could probably improve this.  If you (or your colleague) want
to try rewriting it I'm sure we'd be happy to take a look.

Trevor

[Attachment #5 (text/html)]

<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" \
class="gmail_attr">On Fri, May 1, 2020 at 8:35 AM Mike Hearn &lt;<a \
href="mailto:mike@plan99.net">mike@plan99.net</a>&gt; wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div><div><div><div \
style="display:none;border:0px;width:0px;height:0px;overflow:hidden"><img alt="" \
width="1" height="0" style="display: none; border: 0px; width: 0px; height: 0px; \
overflow: hidden;"></div><div><div>During a code review a colleague flagged an issue \
that I don&#39;t have a great answer for.<br></div><div><br></div><div> The Noise \
spec requires that the EncryptWithAd operation might not actually encrypt, if \
it&#39;s called before the key is set. This seems surprising and potentially a source \
of subtle bugs. I&#39;d have expected an error to be signalled if you attempt an \
encryption or decryption operation without a key.<br></div><div><br></div><div>It \
appears it&#39;s defined this way to make WriteMessage simpler when processing an \
initial key in the first part of a handshake, before any DH operation has run. \
Everything being written out can be passed through EncryptAndHash without a special \
case for the position where no key is available. But translated directly to code this \
results in a rather odd exception inside the core encryption codepath which just \
looks all wrong.</div></div></div></div></div></blockquote><div><br></div><div>Hi \
Mike,</div><div><br></div><div>Yeah, we did it this way to look simpler in \
ReadMessage / WriteMessage but I agree it looks weird to have a function named \
&quot;EncryptWithAd&quot; that might not encrypt.   I think this is unlikely to be a \
source of bugs because handshake processing is so simple and rigid it should be easy \
to test exhaustively with our test vectors.</div><div><br></div><div>But still, we \
could probably improve this.   If you (or your colleague) want to try rewriting it \
I&#39;m sure we&#39;d be happy to take a \
look.</div><div><br></div><div>Trevor</div><div><br></div></div></div>


[Attachment #6 (text/plain)]

_______________________________________________
Noise mailing list
Noise@moderncrypto.org
https://moderncrypto.org/mailman/listinfo/noise


[prev in list] [next in list] [prev in thread] [next in thread]