-
Notifications
You must be signed in to change notification settings - Fork 8.4k
Security: Harden socket creation and validate error code input. #13765
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
Security: Harden socket creation and validate error code input. #13765
Conversation
✅ Deploy Preview for kubernetes-ingress-nginx canceled.
|
Welcome @oyiz-michael! |
Hi @oyiz-michael. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
0997bcb
to
7a7f134
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/triage accepted
/kind cleanup
/priority backlog
/cherry-pick release-1.13 |
/cherry-pick release-1.12 |
@Gacko: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/lgtm |
@Gacko: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Gacko, oyiz-michael The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/ok-to-test |
@Gacko: new pull request created: #13785 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
@Gacko: new pull request created: #13786 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/assign |
What this PR does / why we need it
This PR hardens ingress-nginx in maintenance mode by addressing security and reliability issues:
Security
0777
. Tightened to0660
to restrict access to owner/group.Reliability
os.MkdirAll
), preventing crashes in minimal container images.Types of changes
Which issue/s this PR fixes
How Has This Been Tested?
Socket Security Tests:
Assert final socket mode: stat.Mode() & os.ModePerm == 0o660
Startup when parent dir missing → succeeds and creates dir with 0o755
Parent path exists but is file → returns clear error
Input Validation Tests:
Error-code parsing: ["", " ", "abc", "599", "200", "200, 201"] → validated correctly
Whitespace trimming and empty value handling
Malformed input rejection with clear error messages
Unit Tests:
go test ./internal/ingress/metric/collectors -v - Socket collector tests pass
go test ./internal/ingress/annotations/customhttperrors -v - Input validation tests pass with new trimming logic
Security Validation:
Verified 0o660 permissions still allow NGINX master/worker processes (same group) to access socket
Confirmed directory creation uses secure 0o755 permissions via os.MkdirAll
Tested input validation handles empty strings, whitespace, and malformed error codes
Regression Testing:
All existing functionality preserved
No breaking changes to metrics collection or error handling
Socket communication works normally with hardened permissions
Checklist: