search

rapid7 / rex-powershell

Rex library for dealing with Powershell Scripts
Other
53 stars 35 forks source link

Import SVIT deltas to rex/powershell - 20160628 #1

Closed sempervictus closed 8 years ago

sempervictus commented 8 years ago

With lib/rex/powershell being moved to a separate gem, differences between r7 and svit versions of the namespace can be quickly and easily reconciled to reduce portability issues downstream.

Added functionality includes a parameter to execute PSH in-place, meaning without wrappers in a native or pass-through PowerShell context (meterpreter plugin, other channels). Additional methods pushed upstream include decompression and decoding methods for PowerShell scripts built by Rex, which can be used for debugging or defensive measures.

Testing: None so far, this commit is in preparation for merging upstream changes to the lib/rex structure into the SVIT fork.

Notes: Consumers and developers should test all included functionality as the SVIT fork has some differences from the official upstream Framework, and may need to submit pull requests to other Rex gems (such as Text) in order to retain consistency and functionality with upstream.

sempervictus commented 8 years ago

@dmaloney-r7: Could i pester you to test out the PR in upstream/pro to check that i've not broken anything here? I'll be submitting some other deltas to split-out pieces of Rex over the coming weeks/months. Since i feel PSH to be somewhat my baby i'm starting here, and may therefore be missing changes from other components on which this depends. Thanks

thelightcosine commented 8 years ago

@sempervictus just a couple small things, mostly transcription errors from the diff merge. everything else looks fine here. if you could just patch these up real quick that'd be great, apologies for the delay on reviewing this, the past week has been hectic

sempervictus commented 8 years ago

No worries boss, thanks for looking into this. I'll fix up the comments you made, but I think they point to deeper issues in our internal fork. May take a day, please pardon my lag.

Just for the record, I did ask @bcook-r7 about getting the commit history back on this code, I'm not included in the MSF mailmap (for any number of good reasons, mainly that I work in lib 90% of the time) and I do reference this work for professional engagements. So if there's any chance we can hold merging the dependency till we can have full attribution (@meatballs too, he bridged the gap between our orgs like a major champ), it would be much appreciated. On Jul 5, 2016 11:42, "dmaloney-r7" notifications@github.com wrote:

@sempervictus https://github.com/sempervictus just a couple small things, mostly transcription errors from the diff merge. everything else looks fine here. if you could just patch these up real quick that'd be great, apologies for the delay on reviewing this, the past week has been hectic

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rapid7/rex-powershell/pull/1#issuecomment-230516590, or mute the thread https://github.com/notifications/unsubscribe/ABRPjOjjSZ09pbhJv21u39RRC43E4hx0ks5qSns6gaJpZM4JAoxI .

bcook-r7 commented 8 years ago

Yes, I talked to @dmaloney-r7 about restoring the history in these repos. I think we'll probably need to rebase the repo and do a force push, but it should be doable like we did for rapid7/metasploit-payloads

thelightcosine commented 8 years ago

@sempervictus well, i recovered the history the best that i can, you can see the commits, but the history on the individual files seems to still be jacked =/

sempervictus commented 8 years ago

Anyone bothering to check references for the kind of work we do is likely able to peek at commit history, so i think that'll work just fine for my selfish purposes :). Thank you very much.

On Wed, Jul 6, 2016 at 3:32 PM, dmaloney-r7 notifications@github.com wrote:

@sempervictus https://github.com/sempervictus well, i recovered the history the best that i can, you can see the commits, but the history on the individual files seems to still be jacked =/

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rapid7/rex-powershell/pull/1#issuecomment-230881780, or mute the thread https://github.com/notifications/unsubscribe/ABRPjDU16HoEIoNMWrip1-hrzIdLCltbks5qTALagaJpZM4JAoxI .

thelightcosine commented 8 years ago

@sempervictus I stand corrected, you need to do git log --follow but then it will track the history throguh the change, so it should be all good

thelightcosine commented 8 years ago

@sempervictus if you can fix up those couple of code comments i left, i can get this landed ASAP, cheers

sempervictus commented 8 years ago

Addressed comments, kinda curious what the deal with ternary is over there. Egypt hates 'em too, i use them in every language where it makes sense for concise logic.

Went through our GH history and the initial PR for this is https://github.com/rapid7/metasploit-framework/pull/124, back in 2012 when we had that short window of wondering if maybe the defenders heuristics and signature engines were going to beat us over our open-source nature. Chances are i had this code running before we moved to GH. IIRC you were working the same problem from other angles at the time :).

This repo doesnt seem to carry all of the relevant git history, i suppose i can still use PRs for reference, but commits are probably a bit better given git's cryptographic internals.

bcook-r7 commented 8 years ago

Wow, quite some history! Thanks for keeping at it so long.

It has puzzled me as well, though I've also untangled some gnarly uses, mostly to do with nesting. If there is ever a collective demon to rail against, I'd love if we all agreed that 'unless' causes more bugs than it helps. At any rate, the community ruby style guide says to prefer them where it make sense, if I might invoke a fallacious call to authority: https://github.com/bbatsov/ruby-style-guide

thelightcosine commented 8 years ago

Here's my thinking on ternary operators:

  1. They provide no material benefit. There is no advantage to using them over a standard conditional statement.
  2. They are significantly more difficult to read, forcing the person reading the code to slow down to decipher exactly what's going on
  3. They contain all of the worst aspects of any oneliner in ruby, which is that they are internally opaque to any standard debugging.

All of these things mean that, by my estimation, ternary operators are strictly worse than just using a standard conditional with no inherent advantages. As a general rule we always desire code that favours readability and ease of maintenance over code that is 'clever'. So in this case I have to disagree with the bbatsov style guide.

bcook-r7 commented 8 years ago

ternary aside, would love if we could move this PR to completion, so we could test the powershell compiler PR in framework again

sempervictus commented 8 years ago

Hmm, never thought of them as hard to read, but coding is as personal as driving so I won't argue the point. Thanks for getting this synced. On Jul 15, 2016 7:51 AM, "dmaloney-r7" notifications@github.com wrote:

Merged #1 https://github.com/rapid7/rex-powershell/pull/1.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rapid7/rex-powershell/pull/1#event-724463839, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRPjBCvxkL1gJqYQCi6j8cWHMGvlM6Zks5qV55hgaJpZM4JAoxI .