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.

Sign up for GitHub

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

Jump to bottom

[connection.py] Improvement #63

Merged
merged 19 commits into from
Nov 3, 2023
Merged

[connection.py] Improvement #63

merged 19 commits into from
Nov 3, 2023

Conversation

XiaoliChan
Copy link
Contributor

@XiaoliChan XiaoliChan commented Oct 8, 2023 •
edited
Loading

Update log:

  • connection.py: Add missing self.port in connection.py, in order to use connection.port when writing module.
  • connection.py and protocol: Redirect self.args.port to self.port
  • connection.py: improve ipv6 support, now add is_ipv6 is_link_local_ipv6 variables
  • connection.py: rewrite gethost_addinfo function, don't need try to detect ipv6 anymore, just use AF_UNSPEC instead AF_INET6, AF_INET
  • connection.py: IPv4 preferred when target is dual stack

@XiaoliChan XiaoliChan changed the title [connection] add 'self.port' & [winrm] better logic [connection & winrm]: Improvement Oct 8, 2023
@XiaoliChan
Copy link
Contributor Author

port in connection is very important, in order to write module, it is needed
image

@NeffIsBack NeffIsBack added the enhancement New feature or request label Oct 8, 2023
@XiaoliChan XiaoliChan mentioned this pull request Oct 10, 2023
Closed
@NeffIsBack NeffIsBack added the bug-fix This Pull Request fixes a bug label Oct 10, 2023
@Marshall-Hallenbeck Marshall-Hallenbeck changed the base branch from main to develop October 12, 2023 20:34
@Marshall-Hallenbeck
Copy link
Collaborator

Changed to merge into develop

@XiaoliChan XiaoliChan changed the title [connection & winrm]: Improvement [connection.py]: Improvement Oct 13, 2023
@XiaoliChan
Copy link
Contributor Author

split the commit between connection.py and winrm

@XiaoliChan XiaoliChan changed the title [connection.py]: Improvement [connection.py] Improvement Oct 13, 2023
@XiaoliChan XiaoliChan mentioned this pull request Oct 13, 2023
Merged
mpgn
mpgn reviewed Oct 17, 2023
View reviewed changes
nxc/connection.py Outdated Show resolved Hide resolved
NeffIsBack
NeffIsBack requested changes Oct 18, 2023
View reviewed changes
nxc/protocols/ftp.py Outdated Show resolved Hide resolved
XiaoliChan and others added 9 commits October 19, 2023 10:25
@XiaoliChan
[connection.py] less line
96ba933 
Signed-off-by: XiaoliChan <[email protected]>
@XiaoliChan
Copy link
Contributor Author

Confirm this PR can do review @NeffIsBack

@Marshall-Hallenbeck
Copy link
Collaborator

@XiaoliChan I tried this against an ipv6 target and it doesn't connect, even though I can ping it. I'm attempting to do it with smb - is there something I need to set on the server side?

@XiaoliChan
Copy link
Contributor Author

XiaoliChan commented Nov 1, 2023 •
edited
Loading

@XiaoliChan I tried this against an ipv6 target and it doesn't connect, even though I can ping it. I'm attempting to do it with smb - is there something I need to set on the server side?

Wired, maybe some firewall policy in your side? I didn't set any firewall policy

@Marshall-Hallenbeck
Copy link
Collaborator

I wasn't using the link-local ipv6 address (defining %eth0/%eth1). Running tests now!

@mpgn
Copy link
Collaborator

mpgn commented Nov 2, 2023

Tested against HTP APT machine (only ipv6)

image

@mpgn mpgn added the tested label Nov 2, 2023
@mpgn mpgn added this to the v1.1.0 milestone Nov 2, 2023
@NeffIsBack NeffIsBack added the reviewed code Label for when a static code review was done label Nov 2, 2023
@Marshall-Hallenbeck
Copy link
Collaborator

There are errors with the winrm protocol with ipv6 - it looks like it's an error in Requests trying to parse the ipv6 URL. I don't think that's a killer of this PR, but @XiaoliChan could you look into if Requests can handle ipv6 correctly?

@XiaoliChan
Copy link
Contributor Author

There are errors with the winrm protocol with ipv6 - it looks like it's an error in Requests trying to parse the ipv6 URL. I don't think that's a killer of this PR, but @XiaoliChan could you look into if Requests can handle ipv6 correctly?

Two PR, #72 work after this PR merged

@mpgn
Copy link
Collaborator

mpgn commented Nov 2, 2023

Indeed @Marshall-Hallenbeck with ipv6 we got a nice stacktrace but it's a separate issue fixed on #72

Marshall-Hallenbeck
Marshall-Hallenbeck approved these changes Nov 2, 2023
View reviewed changes
Copy link
Collaborator

@Marshall-Hallenbeck Marshall-Hallenbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this LGTM then!

NeffIsBack
NeffIsBack approved these changes Nov 3, 2023
View reviewed changes
Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Marshall-Hallenbeck Marshall-Hallenbeck merged commit 9fc67da into Pennyw0rth:develop Nov 3, 2023
1 check passed
@XiaoliChan XiaoliChan deleted the connection-miss-port branch November 4, 2023 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NeffIsBack NeffIsBack NeffIsBack approved these changes

@mpgn mpgn mpgn approved these changes

@Marshall-Hallenbeck Marshall-Hallenbeck Marshall-Hallenbeck approved these changes

@zblurx zblurx Awaiting requested review from zblurx

Assignees
No one assigned
Labels
bug-fix This Pull Request fixes a bug enhancement New feature or request reviewed code Label for when a static code review was done tested
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@XiaoliChan @Marshall-Hallenbeck @mpgn @NeffIsBack