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

Sign seal #83

Merged
merged 16 commits into from
Nov 16, 2023
Merged

Sign seal #83

merged 16 commits into from
Nov 16, 2023

Conversation

ramijebara
Copy link
Contributor

@ramijebara ramijebara commented Nov 15, 2023

What

Added sign, seal to the bindstr variable, to samba/lib/com/dcom/main.c. Based on my experience, communication with newer versions of windows is much more reliable. queries that fail with the un-patched code work with this change.

Why

I was trying to communicate with fully patched Windows Server 2016+ machines and the default code was failing communication. After much research and testing of community suggestions I found that this change makes the communication much more reliable.

It has been tested for around 6 months.

References

This change was initially proposed in PR#81

jira issue reference: SC-953 // this line was added by ArnoStiefvater

@ramijebara ramijebara requested a review from a team as a code owner November 15, 2023 16:12
@nichtsfrei nichtsfrei merged commit c6147e5 into greenbone:main Nov 16, 2023
6 checks passed
jjnicola added a commit that referenced this pull request Nov 20, 2023
@jjnicola jjnicola mentioned this pull request Nov 20, 2023
1 task
ArnoStiefvater added a commit that referenced this pull request Nov 20, 2023
## What
Revert #83, which added connection options in a hardcoded way.
<!--
  Describe what changes are being made, e.g. which feature/bug is being
  developed/fixed in this PR? How did you verify the changes in this PR?
-->

## Why
With greenbone/openvas-scanner#1355 this options can be added when
calling wmic or wmi via the library, and currently defaults to [sign].
This hardcoded options collide with the default or provided one.

For testing:
```
wmic -d 7 -U domain/user%pass //192.168.0.1[sign,seal] "SELECT name FROM Win32_ComputerSystem"
```
or 
```
``
<!-- Describe why are these changes necessary? -->

## References

<!-- Add identifier for issue tickets, links to other PRs, etc. -->

## Checklist

<!-- Remove this section if not applicable to your changes -->

- [ ] Tests
@jjnicola
Copy link
Member

Hello @ramijebara,

Thanks a lot for your contribution. Unfortunately, I had to revert this PR, since it generate conflict with the existing way of adding options.
As you can see in the PR #84, I explained the reason and how to use it. Since you are not using other Openvas components (from your comment in PR #82), I suggest that, if possible, you pass your [sing,seal] options directly concatenated with the server variable when you open the connection.
E.g. we use wmi_connect (int argc, char **argv). One of the arguments is the host and it already comes with [sign] concatenated (sign is default but it can be optionally change). This is why it conflicts if the options are also hardcoded with your PR.
Best regards,

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.

None yet

4 participants