chef
Block auto-indexing of static assets more narrowly
In c870cc3, we added rules to 404 instead of serving a 403 or auto-indexing on static asset directories in automate-ui and dex respectively. The regexes used are overly broad in that they cause any URIs ending in "dex" or "assets" to 404^1 including unrelated API endpoints. For example, this is blocking us from publishing policyfiles named "index".
I'm not so familiar with these internals, but I believe it's sufficient to use regexes ^/dex/ and ^/assets/ etc instead of .dex and .assets etc. I also blocked auto-indexing of two other static asset directories under dex (fonts and font-awesome-*). I made a note that ideally dex itself should be handling this.
I'll note that the 404s served at the loadbalancer look different than the 404s of underlying servers, so an attacker can still know they "hit" something. For example:
$ curl -k -v -H 'Host: chef-automate' 'https://127.0.0.1/dex/foo/bar'
...
< HTTP/2 404
< date: Wed, 28 Aug 2024 17:35:03 GMT
< content-type: text/plain; charset=utf-8
< content-length: 19
< x-content-type-options: nosniff
< x-xss-protection: 1; mode=block
< x-content-type-options: nosniff
<
404 page not found
$ curl -k -v -H 'Host: chef-automate' 'https://127.0.0.1/dex/static'
< HTTP/2 404
< date: Wed, 28 Aug 2024 17:35:55 GMT
< content-type: text/html
< content-length: 146
< x-xss-protection: 1; mode=block
< strict-transport-security: max-age=63072000; includeSubDomains
< x-content-type-options: nosniff
<
<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx</center>
</body>
</html>
At least it doesn't auto-index, but I do question the integrity of this approach. As I stated above it should be handled by dex itself. I suspect there's a way to handle the automate-ui assets more elegantly as well.
:nut_and_bolt: Description: What code changed, and why?
Fix nginx 404 rules
:chains: Related Resources
- https://github.com/chef/automate/issues/4855
- https://github.com/chef/automate/pull/5872
- https://github.com/chef/automate/commit/c870cc374fd371efe7aaaeed4d2a02c1affc98d5
- https://community.progress.com/s/article/Paths-or-URIs-ending-with-dex-or-assets-Lead-to-a-404-in-Chef-Automate
:+1: Definition of Done
For us, the ability to publish policyfiles named "index"
:athletic_shoe: How to Build and Test the Change
curl assets and dex endpoints
:white_check_mark: Checklist
All PRs must tick these:
- [x] I have read the CONTRIBUTING document.
- [x] All commits signed-off for the Developer Certification of Origin.
With occasional exceptions, all PRs from Progress employees must tick these:
- [ ] Is the code clear? (complicated code or lots of comments--subdivide and use well-named methods, meaningful variable names, etc.)
- [ ] Consistency checked? (user notifications, user prompts, visual patterns, code patterns, variable names)
- [ ] Repeated code blocks eliminated? (adapt and reuse existing components, blocks, functions, etc.)
- [ ] Spelling, grammar, typos checked? (at a minimum use
make spellin any component directory) - [ ] Code well-formatted? (indents, line breaks, etc. improve rather than hinder readability)
All PRs from Progress employees should tick these if appropriate:
- [ ] Tests added/updated? (all new code needs new tests)
- [ ] Docs added/updated? (all customer-facing changes)
Please add a note next to any checkbox above if you are NOT ticking it.
:camera: Screenshots, if applicable
n/a
Deploy Preview for chef-automate processing.
| Name | Link |
|---|---|
| Latest commit | 490058705e618eaf950435fbf19618b3f64d7456 |
| Latest deploy log | https://app.netlify.com/sites/chef-automate/deploys/66cf62bd92f8d10008572a39 |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
I added this as a feature request as well. If anyone else comes across this PR, please feel free to upvote it too.
Hello @adsr
I tried with the policy name index or dex with these nginx changes and without these changes, but in both cases, the policyfile was not published. If I used any other name instead of index/dex, it worked (using the embedded chef server) in the development setup. I also attached the screenshots with this comment.
Steps I followed
cd automate
hab studio enter
export DEVPROXY_URL=localhost
start_all_services
start_chef_server
infra_service_load_chef_repo
converge_policyfile_chef_client
to reproduce this you can change the policy name in
the automate/.studio/chef-server-collection file lines no 85, 161, and 163 before run converge_policyfile_chef_client
##Screenshots
failed with names exampledex and index
success with name example1
Hi @vinay033 thanks for looking at this. I tried running your steps on a pristine Debian VM but I got an error on start_all_services (logs). Am I missing some setup?
$ uname -a
Linux debian 6.1.0-26-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.112-1 (2024-09-30) x86_64 GNU/Linux
$ cat /etc/*release
PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
NAME="Debian GNU/Linux"
VERSION_ID="12"
VERSION="12 (bookworm)"
VERSION_CODENAME=bookworm
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
$ curl 'https://raw.githubusercontent.com/habitat-sh/habitat/main/components/hab/install.sh' | sudo bash
$ git clone https://github.com/chef/automate
$ cd automate
$ hab studio enter
[1][default:/src:0]# export DEVPROXY_URL=localhost
[1][default:/src:0]# start_all_services
... (see gist link above)
Hello. Are there any further updates on this request please @vinay033 @tpowell-progress ?