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 a .gemspec file and fix dependency issues #45

Merged
merged 8 commits into from
Apr 27, 2022

Conversation

postmodern
Copy link
Contributor

  1. Added a .gemspec file based on the gem metadata of evil-win-3.3. Also added additional project metadata (changelog_uri and source_code_uri).
  2. Restrict dependency versions to ~> X.Y, >= X.Y.Z. This silences warnings from gem build evil-winrm.gemspec.
  3. Added the readline gem as a dependency. If Ruby is not built with libreadline support, the Readline module will actually point to the Reline module provided by the new reline gem. reline is a pure-Ruby alternative to GNU readline, but does not support all of Readline's methods (ex: quoting_detection_proc). This ensures that Readline always points to the actual Readline; although you should consider switching to reline to avoid any readline compilation issues.
  4. Added bundler as development dependency.
  5. Changed the Gemfile to pull dependencies in from the evil-winrm.gemspec file.
  6. Removed the Gemfile.lock file, as it's typically only committed to Git for web applications, not for CLI utils.

@postmodern postmodern force-pushed the gemspec branch 2 times, most recently from 50d01c3 to 2f95192 Compare April 4, 2022 04:10
@jaysonvirissimo
Copy link

jaysonvirissimo commented Apr 4, 2022

Only sorta related, but is it explained somewhere why we don't follow the conventional Ruby gem file structure (Gemfile, lib, spec, bin, etc... at the top level with app code living under lib)?

@postmodern
Copy link
Contributor Author

postmodern commented Apr 4, 2022

My guess is to make it easier to git clone and execute. You could still move evil-winrm.rb into bin/ and add a symlink to bin/evil-winrm (Git now supports symlinks). I would strongly recommend splitting evil-winrm.rb apart into a bin/ file that calls the CLI class, a CLI class, and other misc lib/ classes/modules. This would then make it easier to start writing tests for the code, which would help prevent bugs and breakages.

@OscarAkaElvis
Copy link
Member

First, thank you to take time to do a PR.

We have many comments regarding this PR.

  1. We don't have included gemspec file in the repo. Let's suppose that is ok to have it here... in that case, version should be 3.4 (that's fine, can be changed later), we don't want to leave commented stuff in the source like the line with # spec.require_paths = ["lib"]. Ifi is not needed, then remove it. You removed post_install_message and that is unforgivable for us 😄 . I tested your .gemspec file locally, I built it and after installing it.... i tried to launch evil-winrm .... commands and they don't work. It seems your gemspec prepare it to run as evil-winrm.rb and we don't like that way, we prefer evil-winrm as it is now.
  2. We don't like the metadata added pointing to your forked repo. It should point to our repo or not being there.
  3. Regarding the "X.Y+ version family" in gemspec... what is the point of doing something like that? and after adding the bundler, now the way that the bundler is , is not aligned with how the other dependencies versions are... weird. Even more... after building your gemspec and installing it, i'm getting errors like these:

image

  1. Not sure if adding readline as dependency could be a problem. We developed the autocompletion feature as "if you have it, good, but if you don't have it, just a warning". If you add it as a hard dependency.... I think it will be problematic because in some Linux distributions like Kali Linux, the included ruby is missing readline by default.
  2. I can't see the point of removing Gemfile.lock file even not being a web project I think is useful to make it work with the right versions... what about if tomorrow a new version of some dependency is breaking evil-winrm ?

I checked your Github profile and I know that you are a "ruby guru", and we assumed that you probably know more about Ruby stuff than evil-winrm team... but anyway, we prefer to fit evil-winrm needs than fit Ruby standards or Ruby best practices. Bear that in mind.

Cheers.

@OscarAkaElvis
Copy link
Member

OscarAkaElvis commented Apr 4, 2022

I tested the gem building in another different OS. This time a parrot security instead of kali linux (both are full updated)... and this is the result of trying to install it after building (using your gemspec):

image

@postmodern
Copy link
Contributor Author

postmodern commented Apr 4, 2022

  1. I can redo the commits and remove the commented out lines.
    1.2. Having evil-winrm in the gem but evil-winrm.rb in the Git repo makes things slightly more difficult. I could add another bin/evil-winrm file that simply loads/runs ../evil-winrm.rb.
  2. Oops, looks like bundle gem added those. I will change them to point to https://github.com/Hackplayers/evil-winrm.
  3. >= X.Y.Z will match any version equal to or greater than X.Y.Z. This means if you require >= 1.2.3 that will also match 2.0.1, which may have a different API than the 1.*.* version family. ~> X.Y will match every version that starts with X. and is greater than X.Y.0, this ensures you always use the X.*.* version family starting at X.Y.0. Combine the two together and ~> X.Y, >= X.Y.Z will lock the version range to >= X.Y.Z, < X+1.0.0. RubyGems typically use Semantic Versioning, which means their APIs cannot change or remove things until a major version bump (1.10.99 -> 2.0.0). You probably want to lock down the major version number of all of your dependencies, otherwise when any of the dependencies releases a new major version that changes their API, it would probably break evil-winrm. Read more about rubygems versions/dependencies here: https://guides.rubygems.org/patterns/#semantic-versioning

I tested the gem building in another different OS. This time a parrot security instead of kali linux (both are full updated)... and this is the result of trying to install it after building (using your gemspec):

Can you run that gem install command with --verbose or --debug or possibly try another gem? Also what version of rubygems is Parrot Security using by default? This could be a rubygems bug, possibly already fixed in newer versions of rubygems.

@postmodern
Copy link
Contributor Author

Updated.

@OscarAkaElvis
Copy link
Member

I checked your changes. Great about the most of them... but, I tested the gem building and the installation in another different Parrot Security machine and got the same result. This time I used --debug and this was shown:

image

Do you remember what I told about setting readline as a hard dependency? I still think is not a good idea. What can we do? just remove it as dependency I guess...

@postmodern
Copy link
Contributor Author

postmodern commented Apr 5, 2022

I'm curious what gem list readline shows. I suspect readline isn't available.

Also try without the --local. Appears that readline isn't available so rubygems is trying to resolve the dependency, but --local is preventing it from doing so.

With readline:

$ ruby -v
ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
$ gem list readline

*** LOCAL GEMS ***

readline (default: 0.0.2)
readline-ext (default: 0.1.1)
$ gem build evil-winrm.gemspec 
  Successfully built RubyGem
  Name: evil-winrm
  Version: 3.4
  File: evil-winrm-3.4.gem
$ GEM_HOME=test-gems gem install --force --local evil-winrm-3.4.gem 
Happy hacking! :)
Successfully installed evil-winrm-3.4
Parsing documentation for evil-winrm-3.4
Done installing documentation for evil-winrm after 0 seconds
1 gem installed

@OscarAkaElvis
Copy link
Member

I did some tests... git cloned your fork on gemspec branch (where these changes are). I did what you said.... tried to install, then adding debug and surprisingly... another diferent error arised

image

It seems that the ~> is causing troubles... because fileutils moved forward fast on its versions and now it seems I have one 1.x installed.

I tested it just letting >=0.7.2 removing the ~> from the gemspec file for fileutils and it started to work.

image

As you can see, it got installed and the autocomplete worked!

I see the warning while building regarding the removal of ~> but we should find a way to make it work or live with the warning which I don't really care. If you have any suggestion about how to do this better is welcome... otherwise, maybe we should remove that ~> not only from fileutils. What do you think?

@OscarAkaElvis
Copy link
Member

Still have pending to test it in another vm using kali without readline...

@OscarAkaElvis
Copy link
Member

Tested in Kali which have a ruby compiled without the readline stuff. These are the results of the test after cloning your forked repo (gemspec branch).

image

Similar problems like in parrot but instead of arising in gem installing, it was later trying to use it.

image

As you can see, I have installed stringio 3.0.1 and the gemfile is restricting it to 0.x versions due to the ~>. After removing it from the gemspec and trying again... then happened the same with fileutils... it seems i have 1.5.0. So I removed the ~> as well for it in gemspec file... and this is the result:

image

The final problem arised. This kali has not readline... and it seems is not as simple as a "gem install" to get it. Is the reason we didn't add it before as a dependency. You know more ruby than us, for sure... and correct me if I'm wrong, but as far as I know, the readline feature is not a gem... is like a feature that needs to be added during ruby compilation. That is the reason why we recommend to the users which don't have it to read the notes about it in our Readme, here and follow any of the recommended methods to make it working (for me, the first shown there is always the easier).

So I think definitely that readline should not be there as a dependency. Thoughts?

@postmodern
Copy link
Contributor Author

postmodern commented Apr 6, 2022

Let's bump the version requirements for fileutils and stringio to match what is available on rubygems.org.

It looks like you're still installing the gem with --local, which causes it to not download the readline gem from rubygems.org. Try installing it without --local.

@postmodern
Copy link
Contributor Author

postmodern commented Apr 6, 2022

Updated the version requires for fileutils and stringio.

Under ruby-2.6 (the oldest ruby I have installed), this is the complete output of building and installing the gem:

[postmodern@dev evil-winrm]$ chruby ruby-2.6
[postmodern@dev evil-winrm]$ gem build evil-winrm.gemspec 
  Successfully built RubyGem
  Name: evil-winrm
  Version: 3.4
  File: evil-winrm-3.4.gem
[postmodern@dev evil-winrm]$ gem install evil-winrm-3.4.gem 
Fetching io-console-0.5.11.gem
Fetching reline-0.3.1.gem
Fetching readline-0.0.3.gem
Fetching logger-1.5.1.gem
Fetching rubyntlm-0.6.3.gem
Fetching nori-2.6.0.gem
Fetching multi_json-1.15.0.gem
Fetching stringio-3.0.1.gem
Fetching little-plugger-1.1.4.gem
Fetching ffi-1.15.5.gem
Fetching logging-2.3.0.gem
Fetching httpclient-2.8.3.gem
Fetching builder-3.2.4.gem
Fetching gyoku-1.4.0.gem
Fetching gssapi-1.3.1.gem
Fetching erubi-1.10.0.gem
Fetching winrm-2.3.6.gem
Fetching rubyzip-2.3.2.gem
Fetching winrm-fs-1.3.5.gem
Building native extensions. This could take a while...
Successfully installed io-console-0.5.11
Successfully installed reline-0.3.1
+---------------------------------------------------------------------------+
| This is just a loader for "readline". If Ruby has the "readline-ext" gem  |
| that is a native extension, this gem will load it. If Ruby does not have  |
| the "readline-ext" gem this gem will load "reline", a library that is     |
| compatible with the "readline-ext" gem and implemented in pure Ruby.      |
|                                                                           |
| If you intend to use GNU Readline by `require 'readline'`, please install |
| the "readline-ext" gem.                                                   |
+---------------------------------------------------------------------------+
Successfully installed readline-0.0.3
Successfully installed logger-1.5.1
Building native extensions. This could take a while...
Successfully installed stringio-3.0.1
Successfully installed rubyntlm-0.6.3
Successfully installed nori-2.6.0
Successfully installed multi_json-1.15.0
Successfully installed little-plugger-1.1.4
Successfully installed logging-2.3.0
Successfully installed httpclient-2.8.3
Successfully installed builder-3.2.4
Successfully installed gyoku-1.4.0
Building native extensions. This could take a while...
Successfully installed ffi-1.15.5
Successfully installed gssapi-1.3.1
Successfully installed erubi-1.10.0
Successfully installed winrm-2.3.6
RubyZip 3.0 is coming!
**********************

The public API of some Rubyzip classes has been modernized to use named
parameters for optional arguments. Please check your usage of the
following classes:
  * `Zip::File`
  * `Zip::Entry`
  * `Zip::InputStream`
  * `Zip::OutputStream`

Please ensure that your Gemfiles and .gemspecs are suitably restrictive
to avoid an unexpected breakage when 3.0 is released (e.g. ~> 2.3.0).
See https://github.com/rubyzip/rubyzip for details. The Changelog also
lists other enhancements and bugfixes that have been implemented since
version 2.3.0.
Successfully installed rubyzip-2.3.2
Successfully installed winrm-fs-1.3.5
Happy hacking! :)
Successfully installed evil-winrm-3.4
Parsing documentation for io-console-0.5.11
Installing ri documentation for io-console-0.5.11
Parsing documentation for reline-0.3.1
Installing ri documentation for reline-0.3.1
Parsing documentation for readline-0.0.3
Installing ri documentation for readline-0.0.3
Parsing documentation for logger-1.5.1
Installing ri documentation for logger-1.5.1
Parsing documentation for stringio-3.0.1
Installing ri documentation for stringio-3.0.1
Parsing documentation for rubyntlm-0.6.3
Installing ri documentation for rubyntlm-0.6.3
Parsing documentation for nori-2.6.0
Installing ri documentation for nori-2.6.0
Parsing documentation for multi_json-1.15.0
Installing ri documentation for multi_json-1.15.0
Parsing documentation for little-plugger-1.1.4
Installing ri documentation for little-plugger-1.1.4
Parsing documentation for logging-2.3.0
Installing ri documentation for logging-2.3.0
Parsing documentation for httpclient-2.8.3
Installing ri documentation for httpclient-2.8.3
Parsing documentation for builder-3.2.4
Installing ri documentation for builder-3.2.4
Parsing documentation for gyoku-1.4.0
Installing ri documentation for gyoku-1.4.0
Parsing documentation for ffi-1.15.5
Installing ri documentation for ffi-1.15.5
Parsing documentation for gssapi-1.3.1
Installing ri documentation for gssapi-1.3.1
Parsing documentation for erubi-1.10.0
Installing ri documentation for erubi-1.10.0
Parsing documentation for winrm-2.3.6
Installing ri documentation for winrm-2.3.6
Parsing documentation for rubyzip-2.3.2
Installing ri documentation for rubyzip-2.3.2
Parsing documentation for winrm-fs-1.3.5
Installing ri documentation for winrm-fs-1.3.5
Parsing documentation for evil-winrm-3.4
Installing ri documentation for evil-winrm-3.4
Done installing documentation for io-console, reline, readline, logger, stringio, rubyntlm, nori, multi_json, little-plugger, logging, httpclient, builder, gyoku, ffi, gssapi, erubi, winrm, rubyzip, winrm-fs, evil-winrm after 22 seconds
20 gems installed
[postmodern@dev evil-winrm]$ evil-winrm --help

Evil-WinRM shell v3.4

Usage: evil-winrm -i IP -u USER [-s SCRIPTS_PATH] [-e EXES_PATH] [-P PORT] [-p PASS] [-H HASH] [-U URL] [-S] [-c PUBLIC_KEY_PATH ] [-k PRIVATE_KEY_PATH ] [-r REALM] [--spn SPN_PREFIX] [-l]
    -S, --ssl                        Enable ssl
    -c, --pub-key PUBLIC_KEY_PATH    Local path to public key certificate
    -k, --priv-key PRIVATE_KEY_PATH  Local path to private key certificate
    -r, --realm DOMAIN               Kerberos auth, it has to be set also in /etc/krb5.conf file using this format -> CONTOSO.COM = { kdc = fooserver.contoso.com }
    -s, --scripts PS_SCRIPTS_PATH    Powershell scripts local path
        --spn SPN_PREFIX             SPN prefix for Kerberos auth (default HTTP)
    -e, --executables EXES_PATH      C# executables local path
    -i, --ip IP                      Remote host IP or hostname. FQDN for Kerberos auth (required)
    -U, --url URL                    Remote url endpoint (default /wsman)
    -u, --user USER                  Username (required if not using kerberos)
    -p, --password PASS              Password
    -H, --hash HASH                  NTHash
    -P, --port PORT                  Remote host port (default 5985)
    -V, --version                    Show version
    -n, --no-colors                  Disable colors
    -N, --no-rpath-completion        Disable remote path completion
    -l, --log                        Log the WinRM session
    -h, --help                       Display this help message

and here is the same output under ruby-3.1.1 (the newest ruby I have installed):

[postmodern@dev evil-winrm]$ chruby ruby-3.1.1
[postmodern@dev evil-winrm]$ gem build evil-winrm.gemspec 
  Successfully built RubyGem
  Name: evil-winrm
  Version: 3.4
  File: evil-winrm-3.4.gem
[postmodern@dev evil-winrm]$ gem install evil-winrm-3.4.gem 
Fetching rubyntlm-0.6.3.gem
Fetching nori-2.6.0.gem
Fetching multi_json-1.15.0.gem
Fetching little-plugger-1.1.4.gem
Fetching logging-2.3.0.gem
Fetching httpclient-2.8.3.gem
Fetching builder-3.2.4.gem
Fetching gyoku-1.4.0.gem
Fetching ffi-1.15.5.gem
Fetching gssapi-1.3.1.gem
Fetching erubi-1.10.0.gem
Fetching winrm-2.3.6.gem
Fetching rubyzip-2.3.2.gem
Fetching winrm-fs-1.3.5.gem
Successfully installed rubyntlm-0.6.3
Successfully installed nori-2.6.0
Successfully installed multi_json-1.15.0
Successfully installed little-plugger-1.1.4
Successfully installed logging-2.3.0
Successfully installed httpclient-2.8.3
Successfully installed builder-3.2.4
Successfully installed gyoku-1.4.0
Building native extensions. This could take a while...
Successfully installed ffi-1.15.5
Successfully installed gssapi-1.3.1
Successfully installed erubi-1.10.0
Successfully installed winrm-2.3.6
RubyZip 3.0 is coming!
**********************

The public API of some Rubyzip classes has been modernized to use named
parameters for optional arguments. Please check your usage of the
following classes:
  * `Zip::File`
  * `Zip::Entry`
  * `Zip::InputStream`
  * `Zip::OutputStream`

Please ensure that your Gemfiles and .gemspecs are suitably restrictive
to avoid an unexpected breakage when 3.0 is released (e.g. ~> 2.3.0).
See https://github.com/rubyzip/rubyzip for details. The Changelog also
lists other enhancements and bugfixes that have been implemented since
version 2.3.0.
Successfully installed rubyzip-2.3.2
Successfully installed winrm-fs-1.3.5
Happy hacking! :)
Successfully installed evil-winrm-3.4
Parsing documentation for rubyntlm-0.6.3
Installing ri documentation for rubyntlm-0.6.3
Parsing documentation for nori-2.6.0
Installing ri documentation for nori-2.6.0
Parsing documentation for multi_json-1.15.0
Installing ri documentation for multi_json-1.15.0
Parsing documentation for little-plugger-1.1.4
Installing ri documentation for little-plugger-1.1.4
Parsing documentation for logging-2.3.0
Installing ri documentation for logging-2.3.0
Parsing documentation for httpclient-2.8.3
Installing ri documentation for httpclient-2.8.3
Parsing documentation for builder-3.2.4
Installing ri documentation for builder-3.2.4
Parsing documentation for gyoku-1.4.0
Installing ri documentation for gyoku-1.4.0
Parsing documentation for ffi-1.15.5
Installing ri documentation for ffi-1.15.5
Parsing documentation for gssapi-1.3.1
Installing ri documentation for gssapi-1.3.1
Parsing documentation for erubi-1.10.0
Installing ri documentation for erubi-1.10.0
Parsing documentation for winrm-2.3.6
Installing ri documentation for winrm-2.3.6
Parsing documentation for rubyzip-2.3.2
Installing ri documentation for rubyzip-2.3.2
Parsing documentation for winrm-fs-1.3.5
Installing ri documentation for winrm-fs-1.3.5
Parsing documentation for evil-winrm-3.4
Installing ri documentation for evil-winrm-3.4
Done installing documentation for rubyntlm, nori, multi_json, little-plugger, logging, httpclient, builder, gyoku, ffi, gssapi, erubi, winrm, rubyzip, winrm-fs, evil-winrm after 8 seconds
15 gems installed
[postmodern@dev evil-winrm]$ evil-winrm --help

Evil-WinRM shell v3.4

Usage: evil-winrm -i IP -u USER [-s SCRIPTS_PATH] [-e EXES_PATH] [-P PORT] [-p PASS] [-H HASH] [-U URL] [-S] [-c PUBLIC_KEY_PATH ] [-k PRIVATE_KEY_PATH ] [-r REALM] [--spn SPN_PREFIX] [-l]
    -S, --ssl                        Enable ssl
    -c, --pub-key PUBLIC_KEY_PATH    Local path to public key certificate
    -k, --priv-key PRIVATE_KEY_PATH  Local path to private key certificate
    -r, --realm DOMAIN               Kerberos auth, it has to be set also in /etc/krb5.conf file using this format -> CONTOSO.COM = { kdc = fooserver.contoso.com }
    -s, --scripts PS_SCRIPTS_PATH    Powershell scripts local path
        --spn SPN_PREFIX             SPN prefix for Kerberos auth (default HTTP)
    -e, --executables EXES_PATH      C# executables local path
    -i, --ip IP                      Remote host IP or hostname. FQDN for Kerberos auth (required)
    -U, --url URL                    Remote url endpoint (default /wsman)
    -u, --user USER                  Username (required if not using kerberos)
    -p, --password PASS              Password
    -H, --hash HASH                  NTHash
    -P, --port PORT                  Remote host port (default 5985)
    -V, --version                    Show version
    -n, --no-colors                  Disable colors
    -N, --no-rpath-completion        Disable remote path completion
    -l, --log                        Log the WinRM session
    -h, --help                       Display this help message

@OscarAkaElvis
Copy link
Member

But doing that on versions is "bread for today hunger for tomorrow"... tomorrow if a new stringio major version appears... it will fail again. I insist, is not good idea to use ~> and we are not going to accept the PR if we are going to be maintaining it about this. For us is enough the >= and it will never need to be maintained again (at least about possible version problems). If I need to choose between seeing a warning on gem building (nobody is doing the gem building but me... I build it and then I push it to rubygems.org and the people just do gem install) or if the other option is the possibility of having problems because of the version and then need to do a commit.... I choose to see the warning. Sorry but I can't see the advantage of use ~>.

@postmodern
Copy link
Contributor Author

postmodern commented Apr 7, 2022

tomorrow if a new stringio major version appears... it will fail again.

The problem is that you used the --local option which prevents rubygems from downloading the version from the required version range, if the required version of stringio is not already available on the system. Do you understand? Please do not use the --local option when installing the built gem during testing.

insist, is not good idea to use ~> and we are not going to accept the PR

~> is an accepted practice among almost every rubygem on rubygems.org and is documented on rubygems.org. The problem is because you keep using --local when installing the built gem.

For us is enough the >= and it will never need to be maintained again (at least about possible version problems)

>= is unconsidered bad practice because if a new major version is released that changes the API of Readline, Logger, or StringIO than evil-winrm will break. Since Issues are disabled for this repo, there's no way for users to report this. The solution to this problem is to lock the version ranges down using ~>.

@OscarAkaElvis
Copy link
Member

Ok, let me some time to test this... now I'm busy as hell! but I'll do the tests without the --local and will be back to you. Thank you.

@postmodern
Copy link
Contributor Author

Thank you! I was confused why --local was being giving; like is evil-winrm not installed by default but the .gem file is included with Parrot Linux and you need to install it offline vs. just installing it from rubygems.org. I can wait additional time for this PR.

@OscarAkaElvis
Copy link
Member

Hmmm.... interesting... it worked on parrot

image

Now on kali without the readline. I mean with a ruby not including readline support... let's see:

image

Building ok... now using it to check if autocomplete feature is working...

image

It seems autocomplete is not working. Warning is shown and pressing "tab" is not completing...

So my question is... if adding it as dependency is not making it to work.... why to add it? As I said, I saw readline more like a feature compiled on ruby that as a gem that you can install. The message while building the gem is clear. And this is the proof. Now what?

@OscarAkaElvis
Copy link
Member

Hmmm not sure if my last test on Kali was using your last version... so I git pulled from your fork and then tried again. Now there is no weird message on building but same result

image

@postmodern
Copy link
Contributor Author

Hmm, looks like the ruby-core developers hollowed out the readline gem and moved on the libreadline specific code into readline-ext. We would have to also add readline-ext as a dependency to force it to use the true Readline, which supports quoting_detection_proc. I'll make that change and test again.

Long-term, I would highly recommend replacing the Readline code with the new Reline library. Unlike, readline, reline is pure-Ruby and does not require libreadline be installed.

@postmodern
Copy link
Contributor Author

It installs, but I don't have a Windows system with Remote Management enabled to test it.

$ gem build evil-winrm.gemspec 
  Successfully built RubyGem
  Name: evil-winrm
  Version: 3.4
  File: evil-winrm-3.4.gem
$ gem install evil-winrm-3.4.gem 
Happy hacking! :)
Successfully installed evil-winrm-3.4
Parsing documentation for evil-winrm-3.4
Done installing documentation for evil-winrm after 0 seconds
1 gem installed
$ evil-winrm --version
v3.4

I also noticed that evil-winrm.rb only uses Readline.quoting_detection_proc to check if completion is supported. Maybe there's another way to check for completion supported that would work with both Readline and Reline?

@OscarAkaElvis
Copy link
Member

I'll take time to answer due to vacation period

I don't know if the completion could be done in other way. If you want to research ...

I'll get back to you with the results probably tomorrow. Now answering on phone. I need my laptop to start my windoze VM with WinRM enabled .

@OscarAkaElvis
Copy link
Member

Tested... same result.

image

So adding readline-ext as dependency didn't help.

I think that if readline and readline-ext are not helping we should remove them from the gemspec file in order to accept the PR to add the gemspec file to the repo.

@postmodern
Copy link
Contributor Author

OK I can remove readline and readline-ext commits.

It occurred to me that all of the other Readline methods (line_buffer, completion_proc, completion_append_character, completion_case_fold, completer_quote_characters) are all supported by Reline and should work just like Readline; unless I'm missing something. I could submit a separate PR after this one that removes the Readline.quoting_detection_proc check, replace Readline. with Reline., and add reline as a dependency. Then evil-winrm would not have to deal with libreadline issues on different platforms.

@postmodern
Copy link
Contributor Author

postmodern commented Apr 19, 2022

Double checking the error, readline and readline-ext are being loaded, it's just that libreadline or the readline C extensions do not support Readline.quoting_detection_proc for some reason. Might want to try testing the dev version of evil-winrm on Parrot OS as a control. Removing the readline dependencies probably won't solve the Readline.quoting_detection_proc on Parrot OS.

@OscarAkaElvis
Copy link
Member

Not sure what you mean... my Parrot is not a default parrot, I did what evil-winrm's Readme file say about remote path completion: https://github.com/Hackplayers/evil-winrm#remote-path-completion

So in all my tests the remote path completion for that parrot are working... even before your PR. So the right test is to try it in a clean Parrot or in a Kali. Same result for both, it is not working because ruby has not readline support.

I know that removing the dependencies you added will not fix the problem, but at least we are not adding unneeded dependencies. The point is to add the scrictly necessary dependencies.

@postmodern
Copy link
Contributor Author

postmodern commented Apr 19, 2022

readline and readline-ext are kind of needed dependencies, due to the fact that only readline-ext defines Readline.quoting_detection_proc. If Ruby uses reline instead of readline-ext underneath, then calling Readline.quoting_detection_proc would raise NoMethodError instead of NotImplementedError.

Alternative workarounds are:

A. Depend on readline and readline-ext explicitly.
B. Add a rescue NoMethodError to handle that edge-case when Readline really points to Reline and there is no Readline.quoting_detection_proc method available.
C. Switch from readline to using reline, which should just work (tm) as it is a pure-Ruby implementation of libreadline. Correction, it appears that reline has some issues with tab-completing quoted words, so will have to hold off on reline support.

Your call.

@OscarAkaElvis
Copy link
Member

To be honest not sure about how to do A and I think B makes no sense if finally Reline has problems with completion.

We have here some options now about this PR.

  1. You remove unneeded dependencies and we accept the PR just to have gemspec in the repo.
  2. You do some researching and modify evil-winrm to allow remote path completion based on any of the dependencies you said and if that works, we remove the stuff we have from the readme in order to complile Ruby with readline support
  3. We close the PR without accepting it.

* Metadata was copied from the evil-winrm 3.3 release:
  https://rubygems.org/gems/evil-winrm/versions/3.3
* Added `bin/evil-winrm` which simply loads/executes `evil-winrm.rb`.
* This file is typically only committed to Git for web applications that
  must be deployed to a server, to ensure that the deployed app has the
  exact same dependency versions as the developer's copy of the app.
@postmodern
Copy link
Contributor Author

I have removed the two readline/readline-ext commits and force pushed. See if that works.

@OscarAkaElvis
Copy link
Member

Ok, I've checked it and tested it and I think everything is ok. We'll accept it soon.

@OscarAkaElvis OscarAkaElvis merged commit 7b21baa into Hackplayers:dev Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants