Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAC address getVendorName (OUI Lookup) functionality #904

Merged
merged 83 commits into from Feb 15, 2023

Conversation

egecetin
Copy link
Collaborator

No description provided.

@egecetin egecetin marked this pull request as ready for review October 18, 2022 16:11
@egecetin
Copy link
Collaborator Author

egecetin commented Oct 18, 2022

@seladb I think It is ready to review but I have two questions in my mind,

  • Maybe this feature should be mentioned somewhere in the documentation and/or examples after the merge (or maybe with this PR), but I'm not sure where is the best location for this. I think you can point somewhere more accurately
  • How should this json file (PCPP_OUIDataset.json) be distributed for compiled binary packages? I'm sure some people will miss the requirement of json file, but I can't say my Makefile skills are the best.

@egecetin egecetin marked this pull request as draft December 26, 2022 19:18
.gitignore Show resolved Hide resolved
@egecetin egecetin marked this pull request as ready for review February 5, 2023 14:09
@egecetin egecetin requested a review from seladb as a code owner February 5, 2023 14:09
Common++/src/OUILookup.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,113 @@
import json
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some python experience and have a few ideas on how to improve the code and make it simpler. Do you want me to take a shot?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, why not

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll write the code and let you review

Copy link
Owner

@seladb seladb Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egecetin I refactored the python script in this commit: e887c42 and created a new one: create_oui_data.py.
I also made a few updates to your script:

  • Fixed an encoding issue
  • Removed the line which removes double quotes (") - I don't think it's needed

If you download the latest manuf.dat and run both scripts the result will be similar except for one bug that I think I found in your script:

Some long mac addresses don't have a short address, for example:

1C:87:74:00:00:00/28	PhilipsP	Philips Personal Health Solutions
1C:87:74:10:00:00/28	Sigfox
1C:87:74:20:00:00/28	Nichigak	Nichigaku
1C:87:74:30:00:00/28	SiloraR&	Silora R&D
1C:87:74:40:00:00/28	WeberMar	Weber Marking Systems GmbH
1C:87:74:50:00:00/28	Xiaoxing	Xiaoxinge (Tangshan) Electronic Technology Co., Ltd.
1C:87:74:60:00:00/28	Schawbel	Schawbel Technologies LLC
1C:87:74:70:00:00/28	IngBuero	Ing Buero Ziegler
1C:87:74:80:00:00/28	SurtecIn	Surtec Industries, Inc
1C:87:74:90:00:00/28	WideWorl	Wide World Trade HK ltd.
1C:87:74:A0:00:00/28	Nebbiolo	Nebbiolo Technologies
1C:87:74:B0:00:00/28	HABEYUSA	HABEY USA Inc.
1C:87:74:C0:00:00/28	NewNordi	New Nordic Engineering
1C:87:74:D0:00:00/28	Claber	Claber Spa
1C:87:74:E0:00:00/28	QuestInt	Quest Integrity

In your script those records are put under ASUSTek COMPUTER INC. but it has a different short mac:

1C:87:2C	ASUSTekC	ASUSTek COMPUTER INC.

In my script they are put under a vendor with no name:

    "1869684": {
        "vendor": "",
        "maskedFilters": [
            {
                "mask": 28,
                "vendors": {
                    "31368092319744": "Philips Personal Health Solutions",
                    "31368093368320": "Sigfox",
                    "31368094416896": "Nichigaku",
                    "31368095465472": "Silora R&D",
                    "31368096514048": "Weber Marking Systems GmbH",
                    "31368097562624": "Xiaoxinge (Tangshan) Electronic Technology Co., Ltd.",
                    "31368098611200": "Schawbel Technologies LLC",
                    "31368099659776": "Ing Buero Ziegler",
                    "31368100708352": "Surtec Industries, Inc",
                    "31368101756928": "Wide World Trade HK ltd.",
                    "31368102805504": "Nebbiolo Technologies",
                    "31368103854080": "HABEY USA Inc.",
                    "31368104902656": "New Nordic Engineering",
                    "31368105951232": "Claber Spa",
                    "31368106999808": "Quest Integrity"
                }
            }
        ]
    },

Where 1869684 corresponds to 1C:87:74

Please review the script and let me know what you think

Copy link
Collaborator Author

@egecetin egecetin Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! A minor change might be adding "Unknown" to vendor field instead of empty for a case which short address matches but none of long addresses do not match. So getVendorName will return "Unknown" instead of empty string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also removed my python script since yours is better :)

3rdParty/OUIDataset/updateData.sh Outdated Show resolved Hide resolved
Common++/CMakeLists.txt Outdated Show resolved Hide resolved
Tests/Packet++Test/main.cpp Outdated Show resolved Hide resolved
Tests/Packet++Test/Tests/EthAndArpTests.cpp Outdated Show resolved Hide resolved
Tests/Packet++Test/Tests/EthAndArpTests.cpp Outdated Show resolved Hide resolved
@egecetin egecetin merged commit 0124e33 into seladb:dev Feb 15, 2023
@egecetin egecetin deleted the maclookup branch February 15, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants