* Matthew Toseland <toad at amphibian.dyndns.org> [2007-09-30 00:36:40]:
> The comment on processJFKMessage2 is inaccurate, or at least confusing.
> *This*
> function is being run by the initiator, and can therefore be expensive - it
> involves a signature verification. And calling it "Responder method" is also
> confusing.
>
Fixed in r15410
> More significantly, it would seem that the responder can choose any nonce for
> the initiator? Is this intended? JFK prevents us having to keep state on the
> responder - but we do need to keep some on the initiator, surely?
>
I haven't found anything in the paper related to that... but it sound
like a good idea.
Done in r15413 (r15411 is broken)
> Process message 3:
>
> try { prefix = "I".getBytes("UTF-8"); } catch (UnsupportedEncodingException
> e)
> {}
> - could be relatively expensive, should be kept in a static byte[]
>
Done in r15412
> Do we implement N'_I vs N_I ? This is a commit/reveal on N_I, which seems an
> important part of the protocol, and I don't see any obvious sign of it in the
> code. Clearly this requires state on the part of the initiator: it generates
> a nonce, commits to it, then after a round trip, it reveals it. It is
> interesting that this is not present in the IPsec draft version - which
> version is more up to date / more well-studied?
It doesn't exist in the draft I implemented ... N'_I is the hash of N_I
right ? what does it provide ahead of a fixed length nonce (something we
already have) ? Our nonces are 2^8, is that long enough ?
>
> The boot ID and possibly noderef or other data is "sa" in the spec IMHO.
> Hence
> it should be included in the signature, as well as included in the
> encryption.
ATM we encrypt it but we don't sign it... arguably it's already
authentified by the HMAC.
Done in r15413
>
> Process message 4:
>
> Javadoc header should include the format.
Well, some work has been done in previous commits; It's changing so fast
lately that I don't keep it up to date ^-^
>
> byte[] getBytes
> - this is way too heavy, we do not need serialization here, isn't whatever is
> put into the cache a byte[] anyway? Just cast it!
Agreed.
Done in r15414
>
> Send message 3:
>
> We send the bootID after the signature, we should send it before as it's part
> of sa. It doesn't matter though, certainly not with the outer HMAC. It is
> good to be as close to the spec as possible.
>
Hmmm, I'm not convinced that it's useful; unless you insist I won't
change it.
> We need to resend message 3 if we don't get a message 4 in some period.
Did we do that for StS ? where ? Don't we wait for an other handshaking
attempt to take place ? I don't see how to do it properly with the
current packet mangler ... but yeah it's a good idea.
> Send message 4:
>
> We should exchange noderefs both ways or not at all. Or, ideally,
> configurably. But not only in message 4 and not in any other message!
>
We do send the noderef in message3 and message4 (sa and sa') : it's a
two ways exchange. I don't understand what you mean.
> getLightDiffieHellmanContext(PeerNode):
>
> If currentDHContext is global to the FNPPacketMangler it should not depend on
> any single PeerNode. I'm not sure what the correct fix is - hardcoding the
> group in the NodeCrypto would work, but would suck...
>
Hence I didn't do it until you asked ... done in r15415
> Other than that, looks good.
>
:)
Did you pay attention to synchronization related matters ? I don't know
if things stored in PeerNode (nonceInitiator, Ke, Ka, Ks) require forced
sync. or not :$
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20070930/a7065884/attachment.pgp>
