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

PoC of python nas heuristic #85

Merged
merged 5 commits into from
Dec 17, 2024
Merged

PoC of python nas heuristic #85

merged 5 commits into from
Dec 17, 2024

Conversation

cooperq
Copy link
Collaborator

@cooperq cooperq commented Dec 13, 2024

No description provided.

Comment on lines 49 to 51
res = heur_ue_imsi_sent(msg)
if(res[0]):
print(res[1])
Copy link
Collaborator

@wgreenberg wgreenberg Dec 13, 2024

Choose a reason for hiding this comment

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

Suggested change
res = heur_ue_imsi_sent(msg)
if(res[0]):
print(res[1])
triggered, message = heur_ue_imsi_sent(msg)
if triggered:
print(message)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good suggestion

parsed = NASLTE.parse_NASLTE_MT(bin)
return parsed[0]

def heur_ue_imsi_sent(msg):
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of returning (bool, string | None) why not just return string | None?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just feel like returning a bool is cleaner especially with the above rewrite

Comment on lines 36 to 37
if msg['EPSAttachType']['V'].to_int() == 2:
return (True, output)
Copy link
Collaborator

Choose a reason for hiding this comment

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

a comment explaining the significance of this would be great

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

+1

Comment on lines 24 to 25
def test_non_nas_msg(self):
self.assertEqual(self.run_heur(self.non_nas_msg), False, "non_nas_msg should not trigger heuristic")
Copy link
Collaborator

Choose a reason for hiding this comment

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

what do you think about throwing an error if we fail to parse a NAS message from the input?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we could yes

exit(1)

buffer = sys.argv[1]
msg = parse_nas_message(buffer)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we're not handling the case of an invalid NAS message here, in which case i think msg == None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is handled in line 30 by the heuristic, but we could also handle it here instead, never call the heuristic, and also throw an error.

tools/nasparse.py Outdated Show resolved Hide resolved
print('Opening {}...'.format(pcap_path))

count = 0
for (pkt_data, pkt_metadata,) in RawPcapNgReader(pcap_path):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
for (pkt_data, pkt_metadata,) in RawPcapNgReader(pcap_path):
for pkt_data, pkt_metadata in RawPcapNgReader(pcap_path):

uplink = (gsmtap_hdr[4] & 0b01000000) >> 6
buffer = pkt_data[header_end:]
msg = nasparse.parse_nas_message(buffer, uplink)
(triggered, message)= nasparse.heur_ue_imsi_sent(msg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
(triggered, message)= nasparse.heur_ue_imsi_sent(msg)
triggered, message = nasparse.heur_ue_imsi_sent(msg)

Comment on lines 17 to 19
gsmtap_end_idx = (len(pkt_data) - header_end) * -1

gsmtap_hdr = pkt_data[UDP_LEN:gsmtap_end_idx]
Copy link
Collaborator

Choose a reason for hiding this comment

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

for some reason this took me forever to wrap my head around lol. would gsmtap_hdr = pkt_data[UDP_LEN:header_end] work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah good call, I for some reason forgot you could do a fixed end index for a slice.

@wgreenberg wgreenberg merged commit 32149c3 into main Dec 17, 2024
1 check passed
@wgreenberg wgreenberg deleted the pycrate-parser branch December 17, 2024 22:46
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.

2 participants