apaprocki / node-dhcpjs

dhcpjs provides native DHCP support in Node.js
MIT License
61 stars 21 forks source link

Can I use this module to send a `DHCPINFORM` query? #13

Closed TooTallNate closed 7 years ago

TooTallNate commented 8 years ago

According to https://en.wikipedia.org/wiki/Web_Proxy_Autodiscovery_Protocol, I need to issue a DHCPINFORM query to the DHCP server.

What would a request like that look like?

Thanks in advance!

apaprocki commented 8 years ago

Yes, the enumeration was just missing in the protocol file. I pushed it and added an example file showing basic use of how you would make the request in 53372aa. If I run that example with sudo (due to the use of port 68) I get back DHCPACK messages from my router. I presume if your DHCP server has WPAD information, you'd see it contained in the DHCPACK. Good for your needs?

apaprocki commented 8 years ago

@TooTallNate need any help with this?

pollyshaw commented 7 years ago

I tried running this and didn't get a response back from the server (a home Sagemcom F@ST 2704N branded with PlusNet) until I included the ciaddr in the DHCPInform Request.

Client.prototype.createDiscoverPacket = function(user) {
    var pkt = {
        op:     0x01,
        htype:  0x01,
        hlen:   0x06,
        hops:   0x00,
        xid:    0x00000000,
        secs:   0x0000,
        flags:  0x0000,
        ciaddr: '0.0.0.0',
        yiaddr: '0.0.0.0',
        siaddr: '0.0.0.0',
        giaddr: '0.0.0.0',
    };
    if ('xid' in user) pkt.xid = user.xid;
    if ('chaddr' in user) pkt.chaddr = user.chaddr;
/* This line added */
    if ('ciaddr' in user) pkt.ciaddr = user.ciaddr;
    if ('options' in user) pkt.options = user.options;
    return Client.prototype.createPacket(pkt);
}

I might send a PR but not sure if editing the createDiscoverPacket method is really the right thing to do seeing as you wouldn't usually know the ciaddr when sending a discover message...

apaprocki commented 7 years ago

@pollyshaw That seems unrelated to this pull-request... but according to the RFC:

4.4.1 Initialization and allocation of network address

   The client begins in INIT state and forms a DHCPDISCOVER message.
   The client SHOULD wait a random time between one and ten seconds to
   desynchronize the use of DHCP at startup.  The client sets 'ciaddr'
   to 0x00000000. ...

There should not be an address assigned in an initial DHCPDISCOVER. I'm not sure what is up with that product, but it doesn't appear to be working as it should if it requires it to be non-zero.

pollyshaw commented 7 years ago

Thanks for getting back so quickly! I think the confusion is that the DHCPInform example uses the createDiscoverPacket method. You've convinced me that it would be very wrong to change that method to include the ciaddr, so instead I've created a PR (#18) which just changes the example-dhcpinform to use the createPacket method rather than the createDiscoverPacket method, and that now works perfectly for me.

TooTallNate commented 7 years ago

With https://github.com/apaprocki/node-dhcpjs/commit/53372aa9acb1d5a553d71709f8054f2b01dd080d, this issue can be closed in my book.

pollyshaw commented 7 years ago

I agree that the original issue can be closed, as yes, the module can be used to send a DHCPInform query. However, the example as it stands does not demonstrate this, as it doesn't send the ciaddr in in the packet, and therefore does not work, at least against my DHCP server. (The table in section 4.4.1 in RFC 2131 shows that the ciaddr field should be filled in with the current network address for DHCPInform requests.)

The example-dhcpinform.js calls the following code to create and send the packet:

            var discover = client.createDiscoverPacket(pkt);
            client.broadcastPacket(discover, undefined, function() {
                console.log('dhcpInform ['+interface+': '+pkt.ciaddr+']: sent');
            });

And createDiscoverPacket sets the ciaddr to 0.0.0.0:


Client.prototype.createDiscoverPacket = function(user) {
    var pkt = {
        op:     0x01,
        htype:  0x01,
        hlen:   0x06,
        hops:   0x00,
        xid:    0x00000000,
        secs:   0x0000,
        flags:  0x0000,
        ciaddr: '0.0.0.0',
        yiaddr: '0.0.0.0',
        siaddr: '0.0.0.0',
        giaddr: '0.0.0.0',
    };
    if ('xid' in user) pkt.xid = user.xid;
    if ('chaddr' in user) pkt.chaddr = user.chaddr;
    if ('options' in user) pkt.options = user.options;
    return Client.prototype.createPacket(pkt);
}

Hence my PR #18 which changes the example code to use the createPacket method directly, because createPacket uses the ciaddr provided in the parameter rather than setting it to 0.0.0.0.

Should I create a new issue?

BTW @TooTallNate interested if the example worked for you?

TooTallNate commented 7 years ago

Yes, I would recommend creating a new issue.

On Fri, Oct 28, 2016 at 3:02 PM, pollyshaw notifications@github.com wrote:

I agree that the original issue can be closed, as yes, the module can be used to send a DHCPInform query. However, the example as it stands does not demonstrate this, as it doesn't send the ciaddr in in the packet, and therefore does not work, at least against my DHCP server. (The table in section 4.4.1 in RFC 2131 https://www.ietf.org/rfc/rfc2131.txt shows that the ciaddr field should be filled in with the current network address for DHCPInform requests.)

The example-dhcpinform.js calls the following code to create and send the packet:

        var discover = client.createDiscoverPacket(pkt);
        client.broadcastPacket(discover, undefined, function() {
            console.log('dhcpInform ['+interface+': '+pkt.ciaddr+']: sent');
        });

And createDiscoverPacket sets the ciaddr to 0.0.0.0:

Client.prototype.createDiscoverPacket = function(user) { var pkt = { op: 0x01, htype: 0x01, hlen: 0x06, hops: 0x00, xid: 0x00000000, secs: 0x0000, flags: 0x0000, ciaddr: '0.0.0.0', yiaddr: '0.0.0.0', siaddr: '0.0.0.0', giaddr: '0.0.0.0', }; if ('xid' in user) pkt.xid = user.xid; if ('chaddr' in user) pkt.chaddr = user.chaddr; if ('options' in user) pkt.options = user.options; return Client.prototype.createPacket(pkt); }

Hence my PR #18 https://github.com/apaprocki/node-dhcpjs/pull/18 which changes the example code to use the createPacket method directly, because createPacket uses the ciaddr provided in the parameter rather than setting it to 0.0.0.0.

Should I create a new issue?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/apaprocki/node-dhcpjs/issues/13#issuecomment-257039635, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEWWD5RxEGF25geDYFt2gvnrjG3DNyWks5q4nDmgaJpZM4HrSf0 .