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

[MSSQL] Improvement #136

Merged
merged 27 commits into from
Mar 9, 2024
Merged

Conversation

XiaoliChan
Copy link
Contributor

@XiaoliChan XiaoliChan commented Dec 4, 2023

Change log:

  • No more SMB needed (also remove --no-smb)
  • Enhance the error message
  • Fix no-output in command execution
  • Improve the logic in mssqlexec.py
  • Add --mssql-timeout
  • Fix --use-kcache

References:

Screenshot:
2023-12-05_00-47

@NeffIsBack NeffIsBack added the enhancement New feature or request label Dec 4, 2023
@XiaoliChan XiaoliChan force-pushed the mssql-improv branch 2 times, most recently from 726b56a to d503b0b Compare December 5, 2023 06:46
@XiaoliChan
Copy link
Contributor Author

XiaoliChan commented Dec 5, 2023

CCache fix:

  • Before (it will use host IP address in kerberos authentication, not hostname)
    image

  • After
    image

More screenshot
image

@XiaoliChan
Copy link
Contributor Author

Confirm this PR can do review now

Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
@mpgn
Copy link
Collaborator

mpgn commented Feb 25, 2024

Got one error @XiaoliChan
image

@mpgn mpgn added this to the v1.2.0 milestone Feb 25, 2024
@XiaoliChan
Copy link
Contributor Author

Got one error @XiaoliChan image

I will test it later

@XiaoliChan
Copy link
Contributor Author

@mpgn Please test again, the bug should be fixed

Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
@mpgn
Copy link
Collaborator

mpgn commented Feb 28, 2024

The nanodump is fixed 🎉

I have an error with kerberos, can you check if you have the same ? maybe my lab is broken
image

@XiaoliChan
Copy link
Contributor Author

I have an error with kerberos, can you check if you have the same ? maybe my lab is broken image

I can confirm it work in my side
image

@XiaoliChan
Copy link
Contributor Author

XiaoliChan commented Feb 29, 2024

@mpgn I have made a PR #191 to improve the parse_challenge function, this PR only works after that PR is merged

Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
@XiaoliChan
Copy link
Contributor Author

@mpgn Please test again after #191 is merged

@mpgn
Copy link
Collaborator

mpgn commented Mar 3, 2024

  • For the timeout, use 60 seconds by default, 2 seconds is way to short i'm hitting timeout every two requests
  • The logic you implemented on the login function is really strange and the code is repeted twice if there is a wrong login. I would prefer this version as this is also how it's done on all other protocols
        try:
            res = self.conn.login(None, username, password, domain, None, not self.args.local_auth)
            if res is not True:
                raise
            self.check_if_admin()
            out = f"{self.domain}\\{self.username}:{process_secret(self.password)} {self.mark_pwned()}"
            self.logger.success(out)
            if not self.args.local_auth:
                add_user_bh(self.username, self.domain, self.logger, self.config)
            if self.admin_privs:
                add_user_bh(f"{self.hostname}$", self.domain, self.logger, self.config)
            return True
        except BrokenPipeError:
            self.logger.fail("Broken Pipe Error while attempting to login")
            return False
        except Exception as e:
            error_msg = self.handle_mssql_reply()
            self.logger.fail("{}\\{}:{} {}".format( self.domain, self.username, process_secret(self.password), error_msg if error_msg else ""))
            return False

@XiaoliChan
Copy link
Contributor Author

  • For the timeout, use 60 seconds by default, 2 seconds is way to short i'm hitting timeout every two requests

Maybe 5s is better?

@XiaoliChan
Copy link
Contributor Author

  • The logic you implemented on the login function is really strange and the code is repeted twice if there is a wrong login. I would prefer this version as this is also how it's done on all other protocols
        try:
            res = self.conn.login(None, username, password, domain, None, not self.args.local_auth)
            if res is not True:
                raise
            self.check_if_admin()
            out = f"{self.domain}\\{self.username}:{process_secret(self.password)} {self.mark_pwned()}"
            self.logger.success(out)
            if not self.args.local_auth:
                add_user_bh(self.username, self.domain, self.logger, self.config)
            if self.admin_privs:
                add_user_bh(f"{self.hostname}$", self.domain, self.logger, self.config)
            return True
        except BrokenPipeError:
            self.logger.fail("Broken Pipe Error while attempting to login")
            return False
        except Exception as e:
            error_msg = self.handle_mssql_reply()
            self.logger.fail("{}\\{}:{} {}".format( self.domain, self.username, process_secret(self.password), error_msg if error_msg else ""))
            return False

Yes, I just forgot it did raise and jump into the exception

Signed-off-by: XiaoliChan <30458572+XiaoliChan@users.noreply.github.com>
@mpgn mpgn added the tested label Mar 4, 2024
@mpgn
Copy link
Collaborator

mpgn commented Mar 4, 2024

All good for me, everything works on my lab, great improvement 🎉

@XiaoliChan XiaoliChan mentioned this pull request Mar 5, 2024
@Marshall-Hallenbeck Marshall-Hallenbeck merged commit d9e46db into Pennyw0rth:main Mar 9, 2024
1 check passed
@XiaoliChan XiaoliChan deleted the mssql-improv branch March 9, 2024 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants