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

Add PSRP Support #162

Closed
wants to merge 35 commits into from
Closed

Add PSRP Support #162

wants to merge 35 commits into from

Conversation

jborean93
Copy link
Collaborator

@jborean93 jborean93 commented Mar 23, 2017

This is a pretty big PR so I am all open to suggestions to make it as smooth as possible. This PR brings in the following options/changes

  • Deprecated the protocol class, kept it there for compatibility with older version
  • Deprecated the plaintext and ssl auth options to make things simpler and less confusing
  • Added a client and their subclasses PsrpClient and WsmvClient which replaces the Protocol classes functions
  • Added support for sending Powershell commands over PSRP and various options like pipeline input and retrieving extra powershell streams
  • Tidied up the Session class to be a bit more straight forward and have error handling
  • Some other PRs I've seen like adding proxy and Kerberos service name support
  • Added lots more tests to cover what I changed and added
  • Updated the README with lots more info

Things I know I have broken and am open to how we deal with this

  • Session no longer return std_out, std_err and status_code but rather a custom class with
    • stdout
    • stderr
    • return_code
    • (PSRP Only) output, verbose, debug, warning, information and error streams as separate objects
  • Session now defaults to a https endpoint unless otherwise specified
  • Probably more that I am forgetting

@jborean93
Copy link
Collaborator Author

jborean93 commented Mar 23, 2017

@nitzmahone here is the big beast, appreciate you have limited time but I am happy to work with you to get this release ready and ensure it doesn't negatively impact downstream projects. Let me know your thoughts on all this when you can.

Edit: I'm not sure if you can do this but I was also wondering if you could add me as a maintainer of this repo so I am can go through issues and PRs once this is all done.

@coveralls
Copy link

Coverage Status

Coverage increased (+27.2%) to 95.102% when pulling b3a47fe on jborean93:psrp into 09a81d5 on diyan:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+27.9%) to 95.773% when pulling e4d5e27 on jborean93:psrp into 09a81d5 on diyan:master.

@nitzmahone
Copy link
Collaborator

@jborean93 Daaaang, you've been busy. :) I've blocked out a half day on April 3 (hopefully Ansible stuff will be down to a smolder by then) to just focus on your NTLM encryption PR and this PR. Looks great at first glance, but obviously with something this large, the devil's in the details...

@jborean93
Copy link
Collaborator Author

jborean93 commented Mar 23, 2017

A lot of banging my head against the wall due to really crappy logging on Microsoft's part and just unexplained occurrences of Shell's disappearing but I got there in the end :). I left the encryption stuff out of this PR as I wasn't sure what will eventually change when you get time to have a look through it and didn't want to add any more complexity to this than what is already there.

I'm also currently spiking out a new python library solely for transferring and receiving files so non-Ansible users can have that ability. So far it seems quick but haven't compared to the current Ansible timings and have come across a weird issue with files > 30MB due to MaximumReceivedDataSizePerCommandMB and no documents on how to override this.

@jborean93
Copy link
Collaborator Author

After a lot of time, I finally created another library without the shackles of backwards compatibility with pywinrm here https://github.com/jborean93/pypsrp. Feel free to have a closer look at that if you are interested.

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.

3 participants