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

feature request: ability to receive the original unresolved target address #19

Open
GlenDC opened this issue May 5, 2022 · 5 comments · May be fixed by #24
Open

feature request: ability to receive the original unresolved target address #19

GlenDC opened this issue May 5, 2022 · 5 comments · May be fixed by #24

Comments

@GlenDC
Copy link
Contributor

GlenDC commented May 5, 2022

For logging purposes I would like to have access to the original unresolved target address.

There is a flag in the config to disable dns resolve but in that case an upgrade to socket5 fails:

 socket handle error = upgrade incoming socket to socks5: i/o error: Domain name has to be explicitly resolved, please use TargetAddr::resolve_dns().: Domain name has to be explicitly resolved, please use TargetAddr::resolve_dns().

and without the upgrade to socket5 we do not have access to any target address..

I know my use case is not typical, but would it be possible to also store the original target address separate,
such that one can also get access to the original one? Because once the socket is upgraded to socks5
its target address is already DNS resolved...

@GlenDC
Copy link
Contributor Author

GlenDC commented May 7, 2022

As part of the split effort that might need to happen to solve #15 (comment) properly, this might also be solved in one go.

@GlenDC GlenDC linked a pull request May 8, 2022 that will close this issue
@GlenDC
Copy link
Contributor Author

GlenDC commented May 8, 2022

Your DNS resolve disable config was already working fine, just wasn't able to use it.
#24 is now there,
so this issue can be closed.

@GlenDC GlenDC closed this as completed May 8, 2022
@GlenDC GlenDC reopened this May 10, 2022
@GlenDC
Copy link
Contributor Author

GlenDC commented May 10, 2022

Never mind, in the current flow it is impossible to get the real unresolved address it seems.

Would it be allowed in your scope to have something for the server such as:

pub fn unresolved_target_addr(&self) -> Option<&TargetAddr> {
        self.unresolved_target_addr.as_ref()
    }

Which would basically contain the original read target address,
and will never change.

That would help me out,
while it would not interfere with the flow of any other use case.

@GlenDC
Copy link
Contributor Author

GlenDC commented May 10, 2022

Related to this it would be cool if one can do the socket upgrade, and get the auth done and receive header data (e.g. target address). But do the actual command execution in a separate request.

Currently one can already disable command execution, but once that's done how can one than explicitly execute the command, as that function is not publicly exposed for now.

It's related as it would allow me to get the target addr,
store the unresolved DNS, resolve DNS, store that one, optionally block the incoming socket if my custom firewall doesn't approve the target address, and execute command.

@GlenDC
Copy link
Contributor Author

GlenDC commented May 17, 2022

Could be as simple as:

diff --git a/src/server.rs b/src/server.rs
index c49bb07..9a7c307 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -194,6 +194,7 @@ pub struct Socks5Socket<T: AsyncRead + AsyncWrite + Unpin> {
     config: Arc<Config>,
     auth: AuthenticationMethod,
     target_addr: Option<TargetAddr>,
+    original_target_addr: Option<TargetAddr>,
     cmd: Option<Socks5Command>,
     /// Socket address which will be used in the reply message.
     reply_ip: Option<IpAddr>,
@@ -206,6 +207,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin> Socks5Socket<T> {
             config,
             auth: AuthenticationMethod::None,
             target_addr: None,
+            original_target_addr: None,
             cmd: None,
             reply_ip: None,
         }
@@ -508,6 +510,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin> Socks5Socket<T> {
                 ReplyError::AddressTypeNotSupported
             })?;
 
+        self.original_target_addr = Some(target_addr.clone());
         self.target_addr = Some(target_addr);
 
         debug!("Request target is {}", self.target_addr.as_ref().unwrap());
@@ -628,6 +631,10 @@ impl<T: AsyncRead + AsyncWrite + Unpin> Socks5Socket<T> {
         self.target_addr.as_ref()
     }
 
+    pub fn original_target_addr(&self) -> Option<&TargetAddr> {
+        self.original_target_addr.as_ref()
+    }
+
     pub fn auth(&self) -> &AuthenticationMethod {
         &self.auth
     }

The other option I see is to somehow expose more of the API (with or without a refactor) to allow for the primitive blocks to be used separately and as such be able to get the original target addr prior to resolving it.

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 a pull request may close this issue.

1 participant