qzind / tray

Browser plugin for sending documents and raw commands to a printer or attached device.
https://qz.io
Other
837 stars 272 forks source link

getNetworkInfo: Force Non-Blank MACs #103

Closed tresf closed 4 years ago

tresf commented 8 years ago

Some customers use the MAC Address returned from getNetworkInfo as a way to assist with their own internal product licensing.

Unfortunately, some VPN products return blank MAC addresses confusing the licensing software.

This is a placeholder to allow getNetworkInfo to take an optional parameter to force non-blank MACs and return a non-primary adapter (or perhaps a primary but non-VPN) adapter's MAC address.

first non-empty address mac, but with a order by mac to always return the same to the pc and return not at random that does not allow unique identification.

tresf commented 8 years ago

So it turns out that detecting virtual MACs may be a matter of algorithm.

http://serverfault.com/questions/40712/what-range-of-mac-addresses-can-i-safely-use-for-my-virtual-machines

This would be a much better approach than blindly matching against what we consider "blank". May be a good job for sift but we'd need a new option of listing ALL Mac addresses so that the client-side could do the filtering.

@bberenz thoughts?

tresf commented 8 years ago

@bberenz here's the rough draft of the parsing logic... feedback welcome.

//test data
var macs = clean([
    '0A-00-27-00-00-18', //virtualbox host
    '08-00-27-FB-9F-88', //virtualbox guest
    '7C-7A-91-95-B2-59', //physical wifi
    '7C-7A-91-95-B2-58', //physical ethernet
    '7C-7A-91-95-B2-5C', //physical bluetooth
    '00-00-00-00-00-00-00-E0', //microsoft tunnel adapter
]);

//well known vm guest prefixes
var vmGuest = clean([
    '00-50-56', //vmware
    '00-0C-29', //vmware
    '00-05-69', //vmware
    '00-03-FF', //microsoft
    '00-1C-42', //parallells
    '00-0F-4B', //xen
    '00-16-3E', //xen
    '08-00-27', //vbox
]);

//locally administered prefixes that break the rules
var notUniversal = clean([
    '00-00-00-00-00-00-00-E0' //ms tunnel
]);

//loop through test data and echo results
for (var i = 0; i < macs.length; i++) {
    log(macs[i] + ", Universal: " + isUniversal(macs[i]) + 
        ", VM Guest: " + isVmGuest(macs[i]));
}

//universally administered addresses (burned-in-address, unmodified physical address)
//TODO: determine if Java/ipconfig/ifconfig honor LSB or MSB ordering of bytes
function isUniversal(address) {
    for (var i = 0; i < notUniversal.length; i++) {
        if (address.indexOf(notUniversal[i]) == 0) {
            return false;
        }
        //second insignificant bit of first octet is zero per IEEE 802
        //warning: wireshark will have the addresses in reverse order
        return parseInt(address.substring(0, 2), 16).toString(2)
                .slice(-8).substring(6, 7) == '0';
    }
}

//address belongs to hypervisor guest machine
function isVmGuest(address) {
    for (var i = 0; i < vmGuest.length; i++) {
        if (address.indexOf(vmGuest[i]) == 0) return true;
    }
    return false;
}

//strip non-hex
function clean(data) {
    for (var i = 0; i < data.length; i++) {
        data[i] = data[i].replace(/[^A-Fa-f0-9]/g, "").toUpperCase();
    }
    return data;
}

//debug
function log(o) {
    var p = document.createElement('p');
    p.innerHTML = o;
    document.getElementById('body').appendChild(p);
    console.log(o);
}
tresf commented 8 years ago

@bberenz initial logic pushed to netinfo branch of sift library: https://github.com/qzind/sift/commit/be477f04db5a83418ddd512669780c5400ea5b5f. Opinion on the following appreciated:

  1. Utility functions (_isBurnedIn(...) and family) ... where should they live? Where's a better place?
  2. Do the properties burnedIn: true, ... and family look OK? Will these be easy to pass through keep/toss?
  3. I feel this has immediate value as a static class such as if (sift.Mac('7C-7A-91-95-B2-58').burnedIn) { ... }. Opinion?
  4. Is sift.Mac confusing (e.g. Macintosh?)
akberenz commented 8 years ago

Your design pattern here goes against how the Driver and Vendor objects have been set up. Those objects hold only predetermined assigned variables, and then there is an external list that defines what those values would be for a given input. Yours tries to hold and compute all of this internally.

A better setup then, would be to have the Mac object take parameters of the macAddress, burnedInState, and vmGuestState, and only handle defining the properties. Then have an array of Mac objects that define the state values based on the mac (and only of the macs we would care to filter, everything else would get an inclusive default). So something like:

var internal = {
    ...
    macAddresses: [
        new Mac('UNKNOWN', true, false), // <- default for everything not in this list
        new Mac('00000000000000E0', false, false),
        new Mac('00-50-56', true, true),
        new Mac('00-50-56', true, true),
        ...
    ]
    ...

Then those looping functions you have will be taken care of in the actual filtering logic of sift, rather than in the object creation.

And of course, our "magic" would take care of determining if it was a list of macs (or possibly even a single mac?) for filtering.

tresf commented 8 years ago

Thanks. I thought of this, but

If we were to use the Driver and Vendor design, it seems it would make more sense to do it like this:

new Mac('UNKNOWN', Bia.UNKNOWN, Format.EMPTY, VmGuest.FALSE), // <- default for everything not in this list
new Mac('00000000000000E0', Bia.FALSE, Format.FULL, VmGuest.FALSE),
new Mac('00-50-56', Bia.FALSE, Format.PARTIAL, VmGuest.TRUE),

But I'm still unsure if this makes sense over some simple static functions since Bia.TRUE would never naturally occur in the listing; it hasn't been calculated yet. It seems like we're trying to fit a design which assumes a laundry list of well-known items to work for a list of unknowns (patterns, calculation and rules). I just don't know if it fits the use-case.

akberenz commented 7 years ago

It seems like we're trying to fit a design which assumes a laundry list of well-known items to work for a list of unknowns (patterns, calculation and rules). I just don't know if it fits the use-case.

The filtering can do calculations, each list gets passed to its own internal filter method. But in this case, if there will never be another non-burned-in value to run a check against, we could drop the field from the object. Use it instead as just a list of VM addresses, since those should be static. Then move the calculated field to its own call on the sift object, similar to the parse method.

laundry: [
    new Mac('UNKNOWN', true), //<- any non-vm device
    new Mac('00-50-56'), //params (partialAddress, isReal)
    //and so on
];

sift = { 
...
    checkBurnedIn: function(addr) {
        //code
    }
...
}

Then we'd just need an internal filter method to handle checking the addresses against the partial VM ones we know, and they can be used the same as any other filter.

tresf commented 7 years ago

Yeah, that's much easier to read. Something about LSB in the comments will help explain the burned-in status.

tresf commented 7 years ago

Note, VM is not a he same as not-burned in. Although one can assume VM is never burned in, the opposite can not be assumed. Just because it's not burned-in does not mean it belongs to a VM.

tresf commented 7 years ago

we'd need a new option of listing ALL Mac addresses so that the client-side could do the filtering.

Started support for all Mac addresses via https://github.com/qzind/tray/compare/2.1...netinfo

image

@bberenz feedback appreciated.

tresf commented 7 years ago

New updates: @bberenz: https://github.com/qzind/tray/commit/8f21a87f0a1f975d5adec1585cb7b090443b265e @tresf: https://github.com/qzind/tray/commit/ab27762cfc73a48572c046341c515efefe7fd384

image

tresf commented 7 years ago

tray/netinfo branch merged via 5ac87bf, now it's a matter of planning out the sift library.

Instead of ['0A-00-27-00-00-18', '08-00-27-FB-9F-88', ...] we'll also need to support [ Object, Object ...]. This suggests we should honor the input array format and pop elements appropriately.

The originally proposed clean(...) method is probably better suited as a redundant line in some utility function such as macStartsWith(...), macEquals(...), etc. or even perhaps new Mac(...).startsWith(...) and new Mac(...).equals(...).

tresf commented 7 years ago

Your design pattern here goes against how the Driver and Vendor objects have been set up.

How about this:

 var internal = {

        networkAdapters: [
            // FullAddress|PartialAddress, MacType
            new Mac('00-50-56', MacTypes.VM_GUEST), //vmware
            new Mac('00-0C-29', MacTypes.VM_GUEST),
            new Mac('00-05-69', MacTypes.VM_GUEST),
            new Mac('00-03-FF', MacTypes.VM_GUEST), //microsoft
            new Mac('00-1C-42', MacTypes.VM_GUEST), //parallells
            new Mac('00-0F-4B', MacTypes.VM_GUEST), //xen,vbox
            new Mac('00-16-3E', MacTypes.VM_GUEST),
            new Mac('08-00-27', MacTypes.VM_GUEST),
            new Mac('00000000000000E0', MacTypes.NOT_BURNED_IN), //ms loopback
        ],
        ...

And then a constructor like this:

    //mac address
    function Mac(address, mask) {
        //properties will be visible, but uneditable
        function _prop(val) { return { value: val, enumerable: true }; }

        var plain = plainMac(address);
        mask = setBurnedFlag(plain, mask);

        Object.defineProperties(this, {
            "value": _prop(plain),
            "burnedIn": _prop(mask & MacTypes.NOT_BURNED_IN != MacTypes.NOT_BURNED_IN),
            "vmGuest": _prop(mask & MacTypes.VM_GUEST == MacTypes.VM_GUEST),
            "formatted": _prop(prettyMac(plain)),
            "flags": _prop(mask);
        });
    }

Bitwise flags:

    var MacTypes = {};
    Object.defineProperties(MacTypes, {
        "NOT_BURNED_IN": { value: 1 },
        "VM_GUEST": { value: 2 },
    });

and them some helper functions...

    ///// UTILITY FUNCTIONS /////

    function plainMac(formatted) {
        return formatted.replace(/[^A-Fa-f0-9]/g, "").toUpperCase();
    }

    function prettyMac(raw) {
        return raw.match( /.{1,2}/g ).join(':');
    }

    function setBurnedFlag(mac, mask) {
        if (mask & MacTypes.NOT_BURNED_IN == MacTypes.NOT_BURNED_IN ||
               parseInt(mac.substring(0, 2), 16).toString(2).slice(-8).substring(6, 7) == '0' ) {
            return mask;
        }
        return mask | MacTypes.NOT_BURNED_IN;
    }

.. then the logic to actually perform the sifting, which would require at least the plainMac utility.

tresf commented 7 years ago

Mac address parsing added via https://github.com/qzind/sift/pull/3.

@klabarge can you shim this against QZ Tray 2.1 and perform some testing?

  1. Test on a VM to see if QZ Tray can detect that it's physical network adapter belongs to a VM Guest.
  2. Test on a Windows PC that adds the Microsoft Tunnel adapter, make sure it can be stripped out.
  3. Test against other physical adapters, make sure burned in status is listing properly for each card.

Once that's done, we can document and close. From a documentation perspective, I'd like to illustrate how to get the primary adapter with 2.1 and then how to use sift.js to filter out virtual adapters to get the first physical (burned in) fallback adapter.

tresf commented 7 years ago

@klabarge bump for testing, documentation.

klabarge commented 7 years ago
  1. Test on a VM to see if QZ Tray can detect that it's physical network adapter belongs to a VM Guest.

I started testing on an Ubuntu 14.04 64-bit VirutalBox VM but then discovered this bug https://github.com/qzind/sift/issues/7

  1. Test on a Windows PC that adds the Microsoft Tunnel adapter, make sure it can be stripped out.

I believe this is working in Chrome. data = sift.keep(data, {burnedIn: false}); returns only these adapters, so sift.toss will strip these adapters. However, Firefox is behaving differently (same bug as above)

  1. Test against other physical adapters, make sure burned in status is listing properly for each card.

I will wait for a response on the above bug.

Once that's done, we can document and close. From a documentation perspective, I'd like to illustrate how to get the primary adapter with 2.1 and then how to use sift.js to filter out virtual adapters to get the first physical (burned in) fallback adapter.

Initial documentation here. Adapter filtering documentation has been started, but I have not fulfilled this request yet.