Add Seal/Sign Support for NTLM

[JoeyShapiro]

I added encryption support for NTLM so requests can be made to servers with sign/seal enabled.
I made the changes required to Azure/go-ntlmssp and am now putting them here.
This works on a windows 2016 vm with the default quick config, which requires encryption.

I removed encryption.go and the bodgit/ntlmssp library, as they seem unused and never wired up. Let me know if you need me to add them back for backwards compatibility. But I figured it made more sense to use the one from Azure that is already in and used in other places.

I really want this change so I can use this library to interact with all kinds of winrm servers. So let me know if there are any changes that need to be made for a merge. I am more than happy to make changes.

[masterzen]

Hi!

Thanks for your contributions. Unfortunately, it removes encryption.go on which #188 and #191 are based. I think there may be a path for having all three, but I need a few confirmation before:

1. is the work in this PR genuinely a new implementation of MS-NLMP, or have you reused part of bodgit/ntlmssp ?
2. It seems that sequence-number validation is weaker than what was in bodgit/ntlmssp: the bodgit version provides anti-replay by tracking incomingSeqNum and asserts the peer's signature encodes that same expected value. Yours only pulls the sequence number straight out of the received signature and just checks the checksum computed from it.

[JoeyShapiro]

ok. im happy to work something out. so I made some changes. I decided to remove Azure/go-ntlmssp in favor of only using bodgit/ntlmssp. Then there aren't 2 libraries doing the same thing.

However, I think I noticed a bug in bodgit/ntlmssp. the Wrap function sets OriginalContent to len(length+sig+sealed). The spec (MS-WSMV 2.2.9.1.1.2.1) claims that claims that OriginalContent length must be the length of the original message. I tested this on a 2016 Server, and my way works.

I worked around it here, but I think ideally, I would like to make a PR to that library and fix the problem

[JoeyShapiro]

I just made a PR to the ntlmssp library requesting the change: bodgit/ntlmssp#75

[masterzen]

Hi @JoeyShapiro,

Thanks for your work, I worked this week on a branch that would merge #188, #191 and your (original PR) removing bodgit/ntlmssp and standardizing on azure/go-ntlmssp.
It's almost ready to be shared to you and the two other PRs contributors for testing. The idea was to merge untouched all PRs and work on an adaptation with further commits.

The rationale is that I've never been thrilled using bodgit's version because it will never be as maintained as a microsoft owned version, even if bodgit/ntlmssp quality is good. Unfortunately, azure/go-ntlmssp is missing MIC support (which bodgit has), which forced me into some ugly workarounds.

If you don't mind, I'd prefer to keep your new version on-hold until I push that branch and have you (and the contributors of #188, #191) test it (I have a very limited test environment nowadays). If you all feel that could work and testing is OK, we'll merge that version, otherwise we'll continue with bodgit version (that's independent of the fix you're making to bodgit/ntlmssp which whatever solution we choose should still be pushed there).
Does that sound OK for you?

[JoeyShapiro]

yeah. im good with whatever. ill await your change and see how it works. no rush. I just want this stuff working in a nice way.
On my PR to bodgit, I also noticed some oddities. That is why my original change was using Azure. it seemed more official and supported.
I am fine with us testing a few ideas and taking our time. as long as it all works properly

[masterzen]

Hi @JoeyShapiro,

The branch merging all three PRs, including this one (the original Azure based version) and migrating all to azure/go-ntlmssp is ready for testing at #194.

Please checkout this branch and test against your setup, report any success and failures.

The only downside of azure/go-ntlmssp is lack of MIC support, which I had to work around in WinRM. I may instead implement it upstream, if we go this route.

Thank you for your help!

[JoeyShapiro]

Closing this, as it is solved by #194.