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

X-Forwarded-For (client_ip->source) with unix: #642

Closed
echolimazulu opened this issue Feb 12, 2022 · 10 comments · Fixed by #655
Closed

X-Forwarded-For (client_ip->source) with unix: #642

echolimazulu opened this issue Feb 12, 2022 · 10 comments · Fixed by #655
Assignees
Labels
z-enhancement ⬆️ Product Enhancement

Comments

@echolimazulu
Copy link

echolimazulu commented Feb 12, 2022


Who is affected?

- All users of the unix socket as a listener


Hello Nginx Team and Contributors,

Commit with function: ca373aa

It is necessary to extend the function of replacing a remote client address by introducing support for specifying the appropriate source when using a unix socket.

In case you are using a unix: socket in your configuration, address replacing is not possible based on the CIDR source address format.

The format mask: 0.0.0.0/0 is not applicable (not operational) using the unix listener.

@echolimazulu
Copy link
Author

@ocanty Thanks for the work you've done. Is it possible to extend support for this feature to unix socket users as a listener?

@VBart VBart added the z-enhancement ⬆️ Product Enhancement label Feb 16, 2022
@VBart VBart moved this from To Do to Scheduled Next in NGINX Unit roadmap-tracker Feb 16, 2022
@alejandro-colomar
Copy link
Contributor

Hi @echolimazulu,

I have some initial draft of a patch set to add this feature. Do you have some configuration with which I can test it?

Thanks,
Alex

@echolimazulu
Copy link
Author

Good morning @alejandro-colomar,

Thank you for your reaction and work on this issue!

I apologize for the long answer.

Within 1 hour, I will provide you with the Unit configuration you requested.

@echolimazulu
Copy link
Author

echolimazulu commented Mar 2, 2022

@alejandro-colomar,
I am glad to help.

UPDATE:
Sorry, was fixed.

UPDATE2:
I remind you that this issue was discovered by me and is related to: #641
Earlier I tried to use a mask for source other than CIDR addresses, for example: unix.

/var/lib/unit/conf.json

{
    "listeners": {
        "unix:///var/run/falcon.sock": {
            "pass": "applications/falcon",
            "client_ip": {
                "header": "X-Forwarded-For",
                "source": "0.0.0.0/0"
            }
        },
    },
    "applications": {
        "falcon": {
            "type": "python 3.10",
            "path": "/opt/falcon/",
            "module": "things_asgi",
            "callable": "app"
        }
    }
}

/opt/falcon/things_asgi.py

import falcon
import falcon.asgi


class ThingsResource:
    async def on_get(self, req, resp):
        resp.status = falcon.HTTP_200
        resp.content_type = falcon.MEDIA_TEXT
        resp.text = ('Hello World!\n')


app = falcon.asgi.App()

things = ThingsResource()

app.add_route('/things', things)

/bin/bash:

# curl --header "X-Forwarded-For: 10.10.10.10" --no-buffer -XGET --unix-socket /var/run/falcon.sock http://localhost/things_asgi

@alejandro-colomar
Copy link
Contributor

Thanks!

Could you please also write all of the commands you used to configure, compile, install, run, run-time config, etc.?

@alejandro-colomar alejandro-colomar linked a pull request Mar 2, 2022 that will close this issue
@alejandro-colomar
Copy link
Contributor

Could you please also write all of the commands you used to configure, compile, install, run, run-time config, etc.?

@echolimazulu

Ping :)

I'm having trouble trying that. Please share all of the commands you used so that I can fully reproduce it.

@echolimazulu
Copy link
Author

echolimazulu commented Mar 15, 2022

Hello @alejandro-colomar!

I apologize for the long responses on my part.
I'm hard at work on a patch for unix socket support (open/close - issue #643) which I'll provide via PR soon.

Unfortunately, the process of building Unit and its modules in our CI/CD system is covered by confidentiality requirements that I cannot violate.

I can only say that the assembly practically does not differ from the standard one for debugging with ASGI support.

I will try to test the improvement you suggested in our build system and let you know about all the nuances that arose during use. Please give me a few days to sort this out.

Thank you for the work you have done!
For me personally, the patch you proposed is of great importance and is of great benefit to the entire community!

@alejandro-colomar
Copy link
Contributor

alejandro-colomar commented Mar 15, 2022

Hello @alejandro-colomar!

I apologize for the long responses on my part. I'm hard at work on a patch for unix socket support (open/close - issue #643) which I'll provide via PR soon.

No worries :)

Unfortunately, the process of building Unit and its modules in our CI/CD system is covered by confidentiality requirements that I cannot violate.

I can only say that the assembly practically does not differ from the standard one for debugging with ASGI support.

Okay. Anyway, if you can simplify them by removing extra stuff and leave a minimum working set of commands that contain no sensitive information it would be of great help. If you can't, I'll understand.

My problem is that I've never successfully built and used unit modules; I always get a failure for one or other reason.
So having a set of commands that I know that should work would help a lot.

I will try to test the improvement you suggested in our build system and let you know about all the nuances that arose during use. Please give me a few days to sort this out.

Thanks, that would be of great help too!
Please take into account that my code will probably require changes in your config file,
since it only accepts the exact string "unix" for simplicity.

Thank you for the work you have done! For me personally, the patch you proposed is of great importance and is of great benefit to the entire community!

:-}

@echolimazulu
Copy link
Author

echolimazulu commented Mar 25, 2022

@alejandro-colomar, I have tested your solution. In my case, it works great!

All necessary information in the access_log is also received and displayed.
You perfectly solved this problem, I will learn from your example of brevity and purity of code in programming. Thank you!

This issue can be closed.

@VBart I would like to mark this PR: #655 as a solution to the mentioned problem.

@alejandro-colomar, on the issue: #641 I will try to form the necessary configuration for you to reproduce the specified issue.

@alejandro-colomar
Copy link
Contributor

@alejandro-colomar, I have tested your solution. In my case, it works great!

All necessary information in the access_log is also received and displayed. You perfectly solved this problem, I will learn from your example of brevity and purity of code in programming. Thank you!

This issue can be closed.

@VBart I would like to mark this PR: #655 as a solution to the mentioned problem.

@alejandro-colomar, on the issue: #641 I will try to form the necessary configuration for you to reproduce the specified issue.

I'm very happy to know that it works great for you!
Will try to fix the other issue.
And also happy that you like my code :-}
Thanks for helping with the testing.

Cheers,
Alex

@ghost ghost moved this from Scheduled Next to Released in NGINX Unit roadmap-tracker Sep 7, 2022
@tippexs tippexs moved this from 🚀 Released to 🚀 Released1 in NGINX Unit roadmap-tracker Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-enhancement ⬆️ Product Enhancement
Projects
Status: 🚀 Released
NGINX Unit roadmap-tracker
  
🚀 Released
Development

Successfully merging a pull request may close this issue.

3 participants