Move all the Meterpreter source to this repo
OJ opened this issue · 33 comments
At the moment we have Meterpreter-related source scattered through a few repos:
- https://github.com/rapid7/meterpreter/ - for Windows and POSIX (and OSX at some point).
- https://github.com/rapid7/metasploit-javapayload/ - for Java and Android.
- https://github.com/rapid7/metasploit-framework/ - for PHP, Python and [whatever else].
I think it'd be fantabulous to have all the source in the same repo for a few reasons:
- Having all the source to the same app in the same repo makes a lot of sense.
- Features are often added to one version of meterpreter and not others, partly because it's so easy to forget that the others exist, especially for new contributors (I am particularly guilty of this).
- Meterpreter's history should live in one place regardless of the version.
- Issues related to Meterpreter can be tracked in one location.
- I'm running out of ideas but wanted this list to look bigger.
What do you all think? Is this a worthy goal? I'm sure it'll come with some pain with regards to builds and somesuch, but I think long term it's a worthy goal.
Thoughts appreciated.
For me it would make sense if meterpreter repo had submodules for each different lang/arch?
The Msf/Rex side could use some refactoring too into a separate gem...
And stdapi really needs to be refactored into things that are possible on all arch, e.g. file commands, command execution etc. There's a lot of windows stuff in there for a 'standard API' :) Moving these to extapi would make sense, but not sure how Python handles loading additional modules yet?
You got some free time on your hands? :)
The problems with the subrepo idea are:
- people can clone the source from the subrepos directly instead of getting the lot :) I like the idea of forcing people to have all the source, and force them to consider all the versions prior to submitting a PR.
- You need to submit N PRs for related changes rather than just 1.
*. You need to do the submodule dance with regards to updating, etc.
Personally not a fan of the idea, I still think the source belongs in the one spot.
As far as the stdapi refactor goes, that's worth discussing in another issue, do you agree? There'd be a lot of work involved in any refactor, and it might consume this thread a bit.
Free time? Of course! You have free time, I have free time.. FREE TIME FOR EVERYBODY!
But seriously.. I do have some time that I can throw into this. More than happy to do so. Before embarking, I'm keen to get everyone on the same page and have a clear plan in place.
I'm all for it! Though rather than submodules, I'd just rather we put everything in root here into a subdirectory called 'meterpreter-c', and then move python / php / java into their own meterpreter-* directories as well. It would be great to move some of the TLV/API handling code from metasploit-framework into here as well, so you could make ABI changes and have all of the code in one place.
If we want to move without completely whacking all change history, I've used git-filter-branch for some amazing results in the past.
To get even more radical, I'm not excited about the number of manual steps it takes to publish a gem from meterpreter_bins, then updated on the metasploit-framework side. It feels disruptive. Would it make sense if the actual bins got checked into meterpreter_bins by CI (so anyone could actually checkout that repo and get the latest build from here), and then, rather than publishing to rubygems, we just added a ref or something to the Metasploit-framework gemspec file like so to pull it directly:
gem "meterpreter_bins", :git => 'https://github.com/rapid7/meterpreter_bins.git', :ref => 'd9005ffc627aa2b7f74f25c1a08b64f3d5983ae7'
I can't imaging why any random ruby project other than metasploit-framework would actually care about meterpreter_bins, so publishing it to rubygems seems like overkill. Though maybe I'm making a weird assumption that doesn't apply to metasploit pro or something.
One thing that was clear was that @todb-r7 (and others) don't want to see compiled binaries in source repos. To be honest, I don't think they belong there either. Source control is for source, not the result of compiling that source. Same goes for anything that's generated.
I believe the purpose of hosting the gem on rubygems isn't about making meterpreter available as a gem to other applications, it's about having an externally hosted, publicly-accessible hosting point when updates can be released and people can get access to them without issue. It also means that R7 doesn't have to care about hosting them either. I think the current lay of the gem land makes sense in this regard, and I don't see it as overkill. Everyone already uses and has access to rubygems, so why not reuse it?
As far as the manual steps to publish the gem, I am not totally aware of them as I thought that @cdoughty-r7 et al had made that an automated thing from inside R7. If it's not a simple case of "push button, receive bacon" then I'm clearly in the dark here.
+1 to the rest though ;)
tbh the obvious place for me for binaries is in github under 'Releases' but I'm just old fashioned that way ;)
That makes it a bit harder to automate the process of pushing things out to people. The gem approach is already used by MSF to handle other stuff, so to me it still seems like a logical choice here.
@bcook-r7 I believe putting bins in a gem was a way to facilitate getting the same version in lockstep for both framework and pro. Since gem ... git: ... doesn't give you a hierarchical this-is-greater-than-that version, I don't know if it would work for that purpose.
@Meatballs1 I agree in principle, but 'Releases' do not make it easy to grab the binaries from an install, nor is there a way to upload them in an automated way by CI, unfortunately.
I sorta like the idea of having both client and server code in one place for the reason @bcook-r7 pointed out: changing the ABI can happen all at once without worrying about mismatched protocol versions. On the other hand, it makes builds a lot more complicated because you have to have a way to get that client code into the framework somehow. We could pull the client out into a new gem, but that has other problems.
Unfortunately this problem is close to impossible to solve without one of the following:
- Putting the client and server back in the same repo.
- Putting all the comms code in a submodule/separate repo/ somewhere else.
Both of which don't make me soil myself with glee :/
I think we're going to be stuck with this problem.
N.b. off topic but you could use the Github API with CI to generate releases:
https://developer.github.com/v3/repos/releases/#create-a-release
Only difficulty then is wgetting it down instead of bundle install ;)
@OJ You can never cleanly get the comms code between both server/client repos. However, you could generate the comms data as meta-data (XML TLVs?) and provide some scripts which will generate the Msf/C side code? This means there is a single place for the TLV definitions and removes burden/prevents human mistakes generating the C/Ruby/Python code etc?
The resulting Ruby code probably best pushed to a GEM and then MSF Pull request is just a gem version number change? And Or Meterpreter_Bins Gem could depend on Meterpreter_Client gem, or vice-versa so that there is never a mismatch between compiled binaries and the ruby code?
That'd be a nice to have. Sounds like quite a bit of work! But ultimately if we could get to a point where a process constructs those things from a set of known metadata then it would be nice.
However, the consumers at each still rely on identifiers. If those change for some reason then stuff breaks. It breaks silently in MSF and at compile time in the C stuff. I know we have this problem now, but it's less volatile.
I dunno. I have no answer ¯\_(ツ)_/¯
I think the answer is to have Meterpreter_Client Gem specify a specific Meterpreter_Binary Gem as a dependency. Then if you specify a Msf_Client version you never get a mismatch?
In my humble opinion things were easier when everything was just in one repository. The more repositories there are the harder it is to keep them in sync. I'd be in favour of merging the javapayload repo into this repo.
Additionally the javapayload binaries shouldn't really be in the metasploit-framework repository, let me know if there is anything I can do to help put them into a gem.
I share your view there @timwr, though I admit to not being around when Meterpreter was in the main MSF repo.
The churn that's in the MSF repo does drown out the Meterpreter stuff, and so despite the pain it probably does make sense for Meterpreter to be out on its own. However, the fact that php and python are still in MSF doesn't make sense. If it's to be out on its own, then do it in full! Merging all the Metepreter repos into one makes the most sense, as it it's effectively a single project, just with a few extra implementations.
Happy to work with you on merging things mate!
Just chiming in here (a little late) -- we have lots of other Metasploit repos as well. This is the way Metasploit is going, and I think it's a good thing. Examples via search
This seems to make testing and maintaining much easier, but at the cost of visibility. I don't have a good way today to track multiple repos in one view (@wvu-r7? Sounds like a fun job), and it's easy to forget things.
That said, I know @bcook-r7 was intent on tackling this problem. AFAIK there's no technical reason to not yank the poorly-named metasploit-javapayload into here, along with pretty much everything under /data/meterpreter. Unvendoring, basically.
Cool. So, I'm going to propose creating 'yet another repo' as a test merge of all the meterpreters. I'd like to be careful to preserve the history as well from each one, and even backport some missing history from when this repo was created, which would make me happy. Then, if we like how it turns out, rebase this repo on the new merged-together one. It will have this layout:
meterpreter-c
meterpreter-python
meterpreter-php
meterpreter-java
Also, I want to merge the meterpreter_bins repo in here too, so you can create the gem updated and built from here.
And, I want to get started directly after cutting one more meterpreter_bins release with the latest C meterpreter fixes (maybe if we could get #120 fixed?), and after whittling down the outstanding PRs for the java meterpreter so they don't get orphaned in that repo too.
Hmm, track them how?
@bcook-r7 Yeah merging the bins gem makes sense, esp when all the meterpreters are part of that lib. I'm happy to help with it, so let me know if there's anything I can do. I'll leave the history retention to you 😩 👏
I thought the bins gem was deliberately separate? To stop the bins being stuck in with the source code, inflating the size etc?
Doesn't bother me much, just may make it longer to download on an aussie tin can and twine connection.
Maybe, I'll play with it. The intent isn't to start including the source in the gem, for sure. Surely there's a way to make it not do that. IIRC there was some other gem that was shipping with MBs of build logs accidentally as well.
How does this look for layout?
https://github.com/bcook-r7/metasploit-payloads
The idea is that we may have future payloads called !meterpreter, so I setup the structure with that in mind. This also both preserves the history from the current repos, but also splices in history from older repos as well.
As a fun note, it appears that github has parsed the commits that I imported and sent notification emails. I apologize for that - didn't expect it to trigger notifications like that!
Updated: https://github.com/bcook-r7/metasploit-payloads/tree/master/gem
It now merges all of the meterpreter repos and hooks them all so the results end up in a single gem. I'm still not done fixing all of the places in framework that need updating to use it, but am getting there.
Looks good to me :)
@bcook-r7 did you figure out how to recover the history that I fucked up when I moved to this repo?
Yes, I have a giant merge script that splices it all together. Want to review it?
Edit: I just pushed to a repo instead - I think it needs a little more adjustment, but since its scripted, I can pull the trigger at any time. https://github.com/bcook-r7/mmerge
Update on this, the new unified repo is now accessible under a rapid7 account, and I believe it now has permissions the same as this one for committers: https://github.com/rapid7/metasploit-payloads
I also have a completed a docker image with all of the build dependencies, and have done some unified builds with jenkins. There is still some migration work on the MSF side that will need to happen both to use the new gem and to use the modified API it presents (since it can have package files outside of the 'meterpreter' directory), as well as work to do to automate testing.
The current plan is that once all of the build automation, testing and other things are complete, we'll cut over from these repos and move to the unified one. The total size on disk of the unified repo with full history is 85 MB - I hope it does not bust anyone's download budget. The C meterpreter external source code dependencies will be slowly moving to rapid7/meterpreter-deps, so that we don't make this repo much larger than it needs to be in the future.
Since I can refresh the unified repo as many times as needed, don't let this slow down meterpreter PRs here or elsewhere. In fact, this is a good time, if you have any outstanding PRs, to get them merged. The goal is to get this completely done before May, but I think we can get it finished much sooner.
For moving the issues, I just started looking around; it appears to be possible using the API, or a tool like this: https://github.com/google/github-issue-mover
https://github.com/rapid7/metasploit-payloads is now live! New PRs for C and Java meterpreter at least should should go there. Python and PHP I can keep in sync for now, and I can still easily merge any remaining PRs someone might as well.
👍
I'll miss this repo... it's like leaving an old friend.