questionable code in nmbd/nmbd_packets.c?

[Jeremy Allison]

I'm so far behind on my email it's not even funny :-(.

I'm sorry it's taken so long to get to this.

Cole, Timothy D. wrote:
> 
> I've been trying to nail down some strange behavior with Samba over the
> course of many months, and I've finally narrowed it down to the following
> code in nmbd/nmbd_packet.c, lines 1080-1088:
> 
>       /* This is one occasion where we change a subnet that is
>         given to us. If the packet was sent to WORKGROUP<1b> instead
>         of WORKGROUP<1d> then it was unicast to us a domain master
>         browser. Change subrec to unicast.
>       */
>       if(dgram->dest_name.name_type == 0x1b)
>         subrec = unicast_subnet;
> 
>       process_get_backup_list_request(subrec, p, buf+1);
> 
> Why is this done?
> 
> Since unicast_subnet->myip is the IP of the WINS server, it results in the
> source ip in the reply datagram being that of the WINS server (which is
> wrong), and bogus data (servername<00> == WINS server IP) ends up in the
> NetBIOS cache on the client as a result (also wrong).  This happens because
> the subrec argument is subsequently passed to send_backup_list_response(),
> where it is used to determine the source IP in the reply packet.
> 
> The subnet the original request was made on needs to be preserved.
> 
> IMO, process_get_backup_list_request() should take two subnet_record *
> arguments, one being the subnet the original request was made on (which
> would then be passed to send_backup_list_response()), the second being the
> subnet for which the backup list is returned.
>
> Alternatively, this check could be moved inside of
> process_get_backup_list_request(), where the original subnet record can be
> saved and passed on to send_backup_list_response().  I think that is the
> cleaner approach.  If nobody has any objections, I'll have a patch for this
> prepared shortly.

Either of these look like good fixes. The reason the subnet
record is changed in this particular (1b) case is that we
are being asked as a domain master browser for a list of
all possible backup browsers in the workgroup. In this case
we should search the unicast subnet (which contains the
collated lists for the workgroup) rather than the subnet
the request came in on.

But as you correctly pointed out, we shouldn't reply from
that subnet address.

Luke is incorrect however that doing this at all is a bug,
as it is only done on packet *receipt* - not on request.
This is why it cannot lose a reply record.

I'll also post my patch for this once I've fixed it for
you to test.

Thanks for spotting this,

	Jeremy Allison,
	Samba Team.




-- 
--------------------------------------------------------
Buying an operating system without source is like buying
a self-assembly Space Shuttle with no instructions.
--------------------------------------------------------