Local address announces

[Guus Sliepen]

On Tue, Jul 20, 2010 at 11:35:28AM +0200, Daniel Schall wrote:

> please find attached a proposed feature implementation for tinc.

Great!

> I've implemented a multicast sender, which announces its own endpoint on
> every connected interface regularly.

I guess this is to get around the limitation of broadcast ports. I see you set
the multicast TTL to 1 to limit it to the local LAN, which is a good idea.

> All nodes receiving multicast packets send an answer to the sender with
> their own endpoint address.

Why the need for a response message? If all nodes announce periodically, they
will see each other and their local addresses just from the announcement
packets.

> To ensure authenticated packets, the multicast answer has to be signed using
> the HMAC of the sender.

The challenge field is also not necessary in my opinion, unless you meant
this as a cookie to prevent a HMAC verification from bogus nodes?

> I'd appreciate if you could take your time to look through my source code
> and give me feedback.

Ok, I have only skimmed the source, and I haven't tried it out yet, which I
will do in the coming days.

- On which version did you base your changes? I see some fixes from after the
  release of 1.0.13 in your tarball.

- As Timothy says, using git would have been better, so you can keep track of
  your own changes, and it would be easy for me to import them into my own git
  branches. However, I've now created a local branch and imported your sources,
  so I can at least get a diff against 1.0.13 easily.

- I see a lot of lines where only whitespace has changed, this clutters up the
  diff. In the future, try not to change whitespace unnecesarily. If it's your
  editor doing it automatically, try to turn that feature off.

- Try to use the same code style as the rest of the tinc source. I'm not
  religious about it, but it looks much better when things are uniform.

- It would be nice if send/receive_udp_packet() could be reused for the
  multicast packets. This way you also get encryption and replay protection,
  and you would not have to duplicate the HMAC part.

- You use strcmp() on the contents of a mcpacket_t without any input
  validation. If I send a multicast packet without any NUL bytes in it...

More later :)

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus at tinc-vpn.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://www.tinc-vpn.org/pipermail/tinc-devel/attachments/20100720/f0115fae/attachment.pgp>