Open bcoles opened 5 years ago
Likely, sounds similar to an issue @wvu-r7 found interacting with sessions after deleting the entire workspace.
Yes, this is ~similar to~ probably the same as #11531, which I've been seeing for a while on master
, and likely extends to many parts of database interaction.
To help inform what the proper fix might be, what is the purpose of dropping information related to running sessions while framework is actively handling them? Is the high-level goal just to make sure the database is cleaned up before shutting down framework?
To help inform what the proper fix might be, what is the purpose of dropping information related to running sessions while framework is actively handling them? Is the high-level goal just to make sure the database is cleaned up before shutting down framework?
Not sure if you're asking me?
The high level goal should be to avoid the bloat-and-die paradigm.
The database has always been a second-class-citizen, a nice-to-have, and entirely different from sessions. Information stored in the database is restricted to workspaces, where as sessions are workspace agnostic.
In short, I'll delete whatever I want, whenever I want, as often as I want. It should have no effect on active sessions.
Session state is also mirrored in the database if it is available, which might be the root problem. I ask because a possible solution may be to remove session tracking entirely from the database, since it is problematic to keep consistent if data can disappear at any time, even more so when multiple entities are sharing a database.
MSF5 does do a better job reporting and maintaining database inconsistency than 4 did. But the consequence is now it shows consistency bugs rather than swallowing them.
A minimal change could be to simply teach update_session to have no expectation of success, modulo a more correct rescue here:
diff --git a/lib/metasploit/framework/data_service/proxy/session_data_proxy.rb b/lib/metasploit/framework/data_service/proxy/session_data_proxy.rb
index 27fe49a05d..20795be963 100644
--- a/lib/metasploit/framework/data_service/proxy/session_data_proxy.rb
+++ b/lib/metasploit/framework/data_service/proxy/session_data_proxy.rb
@@ -51,7 +51,8 @@ module SessionDataProxy
mdm_session
end
rescue => e
- self.log_error(e, "Problem updating session")
+ # Sessions are ephemeral, so expect that it may not be possible to update a session.
+ # self.log_error(e, "Problem updating session")
end
end
Another direction is to make the database recreate the missing records on update as well:
--- a/lib/metasploit/framework/data_service/proxy/session_data_proxy.rb
+++ b/lib/metasploit/framework/data_service/proxy/session_data_proxy.rb
@@ -51,7 +51,8 @@ module SessionDataProxy
mdm_session
end
rescue => e
- self.log_error(e, "Problem updating session")
+ # The session may have disappeared, try recreating it.
+ report_session(opts)
end
end
Which would have this effect:
msf5 exploit(multi/handler) > hosts -d
Hosts
=====
address mac name os_name os_flavor os_sp purpose info comments
------- --- ---- ------- --------- ----- ------- ---- --------
192.168.86.31
[*] Deleted 1 hosts
msf5 exploit(multi/handler) > sessions -K
[*] Killing all sessions...
[*] 192.168.86.31 - Command shell session 1 closed.
msf5 exploit(multi/handler) > hosts
Hosts
=====
address mac name os_name os_flavor os_sp purpose info comments
------- --- ---- ------- --------- ----- ------- ---- --------
192.168.86.31
I'd like to propose a possible solution for this category of issues. Any time we delete database records that are tied to sessions we simply access the Msf::SessionManager
, process the Msf::Session
's assigning nil
to the db_record
attribute. I think this would prevent subsequent database operations associated with the given Msf::Session
.
Session state is also mirrored in the database if it is available, which might be the root problem. I ask because a possible solution may be to remove session tracking entirely from the database, since it is problematic to keep consistent if data can disappear at any time, even more so when multiple entities are sharing a database.
I admit I haven't looked into the recent database changes and web service at all, so take my opinion with a grain of salt.
In my ignorance, I find the idea of background changes to the session objects to be disconcerting.
Sessions are alive and valuable. Comparatively, with few exceptions, information stored in the database such as hosts
, loot
and creds
, can usually be re-retrieved via sessions if necessary. loot
is "safely" stored on the filesystem outside of the database magic.
If I ever lose a session because the database tried to do something clever in the backgound, and failed, I'll be very disappointed.
Yeah, it could maybe be risky if there's concurrent access (Metasploit plays fast and loose with multithreading) so we'd need to make sure that we're not in the middle of a database transaction updating a session state while we're also deleting database references from the sessions. If we didn't have to deal with concurrency, it might be a simpler solution.
@adamgalway-r7 would you be able to take a look at this? There are a few solution proposals in the comments above that may be useful.
db sessions objects should not delete, only update. If the session was killed or died it should simply update which makes it interesting that we even see theses errors.
Swallowing the failed update may be best path. While I agree that if the session is missing
from the database creating a new object with the in memory details seems like a good step toward repairing the loss. In the scenario reported here that would restore data intentionally removed by the user making this a less that optimal decision.
This leads me to believe there should be guard logic on objects a session depends on that limit deletion of objects if there is a live session (as detected in the DB or in console instance), in the described case I would have expected a warning similar to that offered when attempting to exit a console with sessions still connected.
msf5> hosts -d
[*] You have active sessions for X hosts, to delete anyway type "hosts -d -f". Any sessions for hosts deleted will exit.
Hi!
This issue has been left open with no activity for a while now.
We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 30 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.
Reconfirmed as of 20/11/2020:
msf6 exploit(windows/smb/ms17_010_eternalblue) > hosts -d
Hosts
=====
address mac name os_name os_flavor os_sp purpose info comments
------- --- ---- ------- --------- ----- ------- ---- --------
192.168.220.133 Windows 7 Professional
192.168.220.142 WIN-VE2H4ACMNAS Windows 8.1 client
[*] Deleted 2 hosts
msf6 exploit(windows/smb/ms17_010_eternalblue) > sessions -K
[*] Killing all sessions...
[-] Session manipulation failed: Couldn't find Mdm::Session with 'id'=3
Latest version from
master
.I issued a
hosts -d
prior tosessions -K
which may or may not have something to do with it.