pypxe / PyPXE

Pure Python PXE (DHCP-(Proxy)/TFTP/HTTP/NBD) Server
MIT License
539 stars 125 forks source link

Vendor-Class filtering #62

Closed pbertera closed 9 years ago

pbertera commented 9 years ago

DHCP server accepts requests only from clients declaring a vendor class (option 60) "PXEClient". Would be useful to add an option to be more flexible. In this patch pbertera/PyPXE@6e45934c0a480be3ad56cca7218ee5a1473c1541 I added a regex filter using the -v or --vendor-class command line argument.

mmattioli commented 9 years ago

Is there a better way to handle this than regex? I feel regex is too clunky...

psychomario commented 9 years ago

IIRC, and this (page 24 Table 2-2 at the bottom) agrees, clients have to start with PXEClient. Are there some non-compliant systems or is this use-case outside the PXE booting stack?

mmattioli commented 9 years ago

Let's take a step back and realize the purpose of the built-in DHCP server isn't to be a standalone DHCP server; it's purpose is to send PXE information to clients via DHCP with the option of providing IP/NM/GW if needed. I feel going down the path of accommodating any/all DHCP clients that aren't meant to PXE boot is a slippery slope and one that's out of the scope of this project.

psychomario commented 9 years ago

I agree with @mmattioli. I'm all for extending the modules within the purview of PXE booting (e.g #27), but I don't think we should strive to be 100% independantly functional, as there are many better and more well maintained projects for e.g dhcp servers.

pbertera commented 9 years ago

I understand your reasons, but would be great to have the DHCP service compatibile with some non-standard devices, maybe we can leave the vendor-class filtering in the library without having the command line switch in pypxe-server.py

Or at least place the vendor-class filtering in a method, something like:

def isPxeClient(self, options):
    if not (60 in options and 'PXEClient' in options[60][0]):
        return False

So one can sublcass the DHCPD class and implement a custom filter. Obviously I'm speaking about using PyPXE as a library embedded in some projects

psychomario commented 9 years ago

This seems like an acceptable solution to me, it means it's not our job to create a clean way to create the rules, and we're not really going outside the 'minimal server' bounds.

If you can implement this tidily and add something to the documentation, I'll see about merging it in.

psychomario commented 9 years ago

Just a note, that snippet will need an else return True or it'll never be true.

pbertera commented 9 years ago

@psychomario yes, sure :) Against which branch should I send a PR still development ?

psychomario commented 9 years ago

yes, development. It'll be merged into master with your other PR and mine when we've finished the cleanup and testing.