Fix for session clixml encoding on Python 3

[jborean93]

The output from WinRM is a byte string and on Python 3 the Session class tries to clear up the clixml that is produced on the stderr. Currently it expects a text/unicode string which is fine on Python 2 but doesn't work on 3. This PR fixes that issue and ensure the stderr is cleaned and outputted properly as per before.

Supersedes:

• Issue #111 #112
• Decode std_err on python3, fixes #136 #137
• Fix encoding issue on Python3 #178
• decode stderr before _clean_error_msg #218
• Decode std_err on run_ps before cleaning up #220

[coveralls]

Coverage increased (+3.5%) to 72.875% when pulling 3bf4ab0 on jborean93:py3-stderr into ffec954 on diyan:master.

[nmaludy]

+1 Just ran into this as well. Excited to see it merged.

[lebonez]

This actually fails when len(nodes) == 0. It attempts to encode the original message but fails because it is still bytes. This was actually circumvented previously by _strip_namespace failing. Since pre python 3.6 can't do fancy type checking the best fix would be to do this and let the exception hit then warn (seems best for backwards compatibility and to prevent type incompatibility from hiding).
assert len(nodes) != 0, "nodes length 0 falling back to original error message"

change:
return msg.encode('utf-8')
back to
return msg

[Bouke]

I think I ran into the same encoding issue;

Traceback (most recent call last):
  File "(..)/bin/invoke", line 11, in <module>
    sys.exit(program.run())
  File "(..)/lib/python3.6/site-packages/invoke/program.py", line 332, in run
    self.execute()
  File "(..)/lib/python3.6/site-packages/invoke/program.py", line 480, in execute
    executor.execute(*self.tasks)
  File "(..)/lib/python3.6/site-packages/invoke/executor.py", line 133, in execute
    result = call.task(*args, **call.kwargs)
  File "(..)/lib/python3.6/site-packages/invoke/tasks.py", line 127, in __call__
    result = self.body(*args, **kwargs)
  File "(..)/tasks.py", line 49, in all
    network(ctx, host, username=username, password=password)
  File "(..)/lib/python3.6/site-packages/invoke/tasks.py", line 127, in __call__
    result = self.body(*args, **kwargs)
  File "(..)/tasks.py", line 58, in network
    domain = s.ps('(Get-ItemProperty "HKLM:\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\").Domain').strip()
  File "(..)/tasks.py", line 34, in ps
    r = self.run_ps(cmd)
  File "(..)/src/pywinrm/winrm/__init__.py", line 54, in run_ps
    rs.std_err = self._clean_error_msg(rs.std_err)
  File "(..)/src/pywinrm/winrm/__init__.py", line 89, in _clean_error_msg
    return msg.encode('utf-8')
AttributeError: 'bytes' object has no attribute 'encode'

[bharathanr]

I ran into this issue myself, and I was wondering why it hasn't been integrated yet. Is it because the build is now failing for Python2? I'm not very sure about how to interpret the test output, so I may be wrong there.

The try-except in _strip_namespace should be left in, correct? I'm interested in understanding why it was removed originally.

Thanks for your time.

[ltamaster]

+1

[crowdy]

👍

[jborean93]

@lebonez thanks for the heads up, just updated the commit to include cases where stderr does not contain clixml.

[farhank3389]

Anyone know when this will be merged?

[aduzsardi]

It's still not working for on python 3.6.6 (ubuntu 18.04)

Getting this error message

Traceback (most recent call last):
  File "./Get-ADUser", line 19, in <module>
    rp = s.run_ps(command)
  File "/home/user/stuff/winrm/lib/python3.6/site-packages/winrm/__init__.py", line 54, in run_ps
    rs.std_err = self._clean_error_msg(rs.std_err)
  File "/home/user/stuff/winrm/lib/python3.6/site-packages/winrm/__init__.py", line 89, in _clean_error_msg
    return msg.encode('utf-8')
AttributeError: 'bytes' object has no attribute 'encode'

Any ideas ?

[aduzsardi]

this should be return msg.decode('utf-8')

[jborean93]

Looks like it expects msg to be set to new_msg above this but in some cases that's not what happens. Needs a bit more logic and thanks for the pickup.

[lebonez]

The reason this is failing is when new_msg is an empty string it goes back to attempting to encode a bytes object as stated in the comment you can't decode a bytes object. But the comment will fail as well when msg is a string because it doesn't have a decode attribute. I would suggest the following be committed

if new_msg:
    return new_msg.strip()
else:
    # maybe add .strip() here not sure the point.
    return msg.decode('utf-8')

I forgot about this after I overwrote the classes method to just return error msg. It might also be smart to add an except block for the else that way it catches an attribute error to just return the msg because it probably wasn't a bytes object as below. Just remove the else and do the following which is better in my opinion.

            else:
                # if new_msg was populated, that's our error message
                # otherwise the original error message will be used
                if new_msg:
                    # remove leading and trailing whitespace while we are here
                    return new_msg.strip().encode('utf-8')
        try:
            return msg.decode('utf-8').strip()
        except AttributeError:
            return msg

[jborean93]

@aduzsardi @lebonez the latest ocmmit should fix up the latest issue

[aduzsardi]

Hi, thx for the update
The error is gone now , but with ActiveDirectory module commands i don't get any output
For example

#!/usr/bin/env python3

import winrm
from getpass import getpass

username = input("Enter your AD username: ")
password = getpass("Enter your AD password: ")
ad_domain = "mydomain.lan"
upn = username + '@' + ad_domain

command = 'Get-ADUser ' + username

s = winrm.Session('admember.mydomain.lan',auth=(username,password),transport='ntlm')
pwsh = s.run_ps(command)
print(pwsh.std_out.decode('utf-8'))

Am i doing something wrong ?
But it does work with something like
command = 'Get-Content C:/test.txt'

[aduzsardi]

I printed the std_err , and is looks like you can't run activedirectory commands on a domain member server (that's not a domain controller) when you are connected remotely on it (winrm) even if the powershell module is installed on the server.

Unable to contact the server. This may be because this server does not exist, it is currently down, or it does not have the Active Directory Web Services running.
    + CategoryInfo          : ResourceUnavailable: (wsus:ADComputer) [Get-ADComputer], ADServerDownException
    + FullyQualifiedErrorId : ActiveDirectoryServer:0,Microsoft.ActiveDirectory.Management.Commands.GetADComputer

[jborean93]

@aduzsardi you can but you either need to use CredSSP, Kerberos with delegation, or explicitly define the credential in your powershell script.

[aduzsardi]

yes that's true... but
i'll say just that microsoft needs to find a better way to do remote connections with a bastion host in a domain environment , and it should be ready out of the box , messing around with kerberos delegations and group policies to set up credssp it's not very appealing to a mainly linux/*nix systems administrator when you're used to the simplicity of ssh ... that's my humble opinion

back to the code , thank you for the patches 👍

[jborean93]

@aduzsardi Microsoft have documented numerous ways that you can overcome the double hop issue https://docs.microsoft.com/en-us/powershell/scripting/setup/ps-remoting-second-hop?view=powershell-6.

Ultimately the biggest problem is that people expect network authentication like WinRM to act the same way as authentication through a local console or on RDP. Unless you use CredSSP or Kerberos with credential delegation, the new network login that is created does not have access to the user's credentials or a special token that can be used to authenticate with any further servers. Whereas a local or RDP login (uses CredSSP), means the login has access to the user's credentials which can be used to authenticate with further servers.

With pywinrm you have the ability to do the following to get credential delegation working;

• Use CredSSP, arguably the easiest but the delegation is unconstrained which can be a potential security issue
• Use Kerberos with kerberos_delegation=True to created a Kerb token that can be delegated, this can then be configured with AD to create a constrained delegation scenario for added security
• Pass your username and password in the PowerShell script you wish to run and use the -Credential parameter to explicitly set your credentials.

There's another option but unfortunately you cannot use pywinrm for this. This option is to create a custom PSSession Configuration endpoint that runs the script as a specific user. This can be combined with JEA to limit what cmdlets scripts can call for added security. If you are interested in this option, have a look at https://github.com/jborean93/pypsrp which is a Python library I've worked on that runs on the PowerShell Remoting layer rather than just WinRM.

[pratikpotdar]

Hey @badcure
I can see that this has been merged and added to 0.3.1 milestone. And also that the milestone is 100% complete. Was wondering when will this be pushed to PyPI? I'm using this library and facing this exact issue. Wanted to use the updated library.

[badcure]

Pinging @nitzmahone here. I remember him mentioning that he wanted to do some testing due to recent changes. With that said, let me know if you are comfortable with me releasing a new version.