-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
Update API to get details from postgres #391
Conversation
@mzfr Having 4 queries is ok, we just have to think how to do it in the most efficient way I also think, maybe we can redesign our API according to our data model |
9489260
to
599c1cd
Compare
This is done in case there are large number of sessions for a snare
Made changes to use sqlalchemy instead of running raw queries
Also make tables accesible for both the functions of a class
599c1cd
to
c49d7c7
Compare
tanner/api/api.py
Outdated
|
||
return result | ||
|
||
async def return_snare_info(self, uuid, count=-1): | ||
query_res = [] | ||
async def return_snare_info(self, uuid): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if instead of returning all the info for all the sessions we will return only the list of sessuin uuids and make the full info as a separate API request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for returning sessions UUID we already have the call i.e api/<snare-uuid>/sessions
. So in a way, we already have a separate API request for full information
Instead of using insert() function separately we used Tables functionality to insert
tanner/api/api.py
Outdated
async with self.pg_client.acquire() as conn: | ||
stmt = select([PATHS.c.attack_type]) | ||
rows = await (await conn.execute(stmt)).fetchall() | ||
result["total_sessions"] = len(rows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rows = await (await conn.execute(stmt)).fetchall() | ||
result["total_sessions"] = len(rows) | ||
for r in rows: | ||
attack_type = AttackType(r[0]).name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see prev comment
tanner/api/api.py
Outdated
sessions = await self.return_snare_info(snare_uuid) | ||
if sessions == "Invalid SNARE UUID": | ||
return result | ||
time_stmt = select([SESSIONS.c.start_time, SESSIONS.c.end_time]).where( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tanner/api/api.py
Outdated
query = await (await conn.execute(stmt)).fetchall() | ||
|
||
for row in query: | ||
session = loads(dumps(dict(row), default=alchemyencoder)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am just curious, what is the problem of just dict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns a dictionary with id
as UUID(<value>)
it's not decoded. So either we will have to separately decode the value to str or use this JSON dumps.
Both dumps and loads are used because dumps return a string but we need it like a dictionary.
tanner/api/api.py
Outdated
from tanner.utils.attack_type import AttackType | ||
|
||
|
||
def alchemyencoder(obj): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't belong to API class
""" | ||
% (uuid) | ||
stmt = select([SESSIONS]).where(SESSIONS.c.sensor_id == uuid) | ||
query = await (await conn.execute(stmt)).fetchall() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still sure that fetching all sessions is a bad idea
Also modified the apply_filter function.
Default for LIMIT is 1000 and OFFSET is 0
).where(SESSIONS.c.sensor_id == snare_uuid) | ||
|
||
times = await (await conn.execute(time_stmt)).fetchall() | ||
result["total_duration"] = str(times[0][0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested with my db and got this "total_duration": "-1 day, 23:56:37"
tanner/api/api.py
Outdated
result['attack_frequency'][attack] += 1 | ||
result["total_sessions"] = 0 | ||
result["total_duration"] = 0 | ||
result["attack_frequency"] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to return only positive numbers or all zeros as well? if we want to return all the range, it should be more attacks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah we can do that it returns all the attacks that are available, no matter what the type of attack is.
tanner/api/api.py
Outdated
all_paths = [] | ||
for p in paths: | ||
all_paths.append(dumps(dict(p), cls=AlchemyEncoder)) | ||
session["paths"] = all_cookies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paths
@afeena please test the changes that I pushed. I think there is still something wrong with the time. |
tanner/api/api.py
Outdated
tables += ", paths P" | ||
columns += ", P.session_id" | ||
where += " AND P.attack_type=%s"%(filters["attack_type"]) | ||
elif "owners" in filters: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure if/else is a best solution here?
>>> filters = {"attack_type":1, "owners":"user"}
>>> if "attack_type" in filters:
... tables += ", paths P"
... elif "owners" in filters:
... tables += ", owners O"
...
>>> tables
'sessions S, paths P'
@mzfr If you think something wrong with a time - test it with multiple cases. Please, test you code as much as you can before you push it. |
Instead of using if/elif we have to use if so each condition get checked everytime
useful while write tests
@mzfr the bug still exists and I can't write all the paths into the postgres
|
@afeena It's weird because I used nikto as well as zaproxy and I got equal number of accepted paths and counts in the |
tanner/dbutils.py
Outdated
@@ -142,13 +142,16 @@ def time_convertor(time): | |||
|
|||
for path in session["paths"]: | |||
timestamp = time_convertor(path["timestamp"]) | |||
attackType = AttackType[path["attack_type"]].value | |||
if not attackType: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how this solves KeyError? if path doesn't have attack_type you will get an exception anyway
File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/server.py", line 106, in on_shutdown
File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/sessions/session_manager.py", line 83, in delete_sessions_on_shutdown
File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/sessions/session_manager.py", line 98, in delete_session
File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/sessions/session_analyzer.py", line 32, in analyze
File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/sessions/session_analyzer.py", line 39, in save_session
File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/dbutils.py", line 146, in add_analyzed_data
KeyError: 'attack_type'
This should only happen if right after sending the request tanner shutdowns
API return wrong number of sessions (it return the number of paths instead):
Filters doesn't seem to work
Error 500 when using name of the attack
|
Also make attack_type filter work with names
|
…e in human readable format
If someone passes a valid UUID but there is no session of that UUID then API would crash so to prevent that I added try and except
@mzfr Works good now :) |
I have added the support for the API to work with Postgres. All the endpoints are working similarly to the way they were working with Redis. I haven't tested the
tannerweb
but I am assuming that if API is working then the web part would still be fine(I'll test tanner web and make sure nothing breaks in it).Now the implementation might look sloppy because I used 4 different queries to extract the information rather than execute single query with multiple joins on the table. The reason I didn't use a single query with joins is that the data returned was weird[1].
[1] It returned a very large list(let's call it L) that had multiple lists in them and each small list was actually a row. Now the problem was that say if we have 5 cookies(key and value) so the first 5 lists of L have the same values except 1 different value(the cookie).
Sorry if that explanation was confusing :)