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

minified/obfuscated resources in "pages/static"; privacy breach #879

Open
onlyjob opened this issue Sep 10, 2015 · 11 comments
Open

minified/obfuscated resources in "pages/static"; privacy breach #879

onlyjob opened this issue Sep 10, 2015 · 11 comments

Comments

@onlyjob
Copy link

onlyjob commented Sep 10, 2015

Files in pages/static contain embedded JavaScript and CSS resources in minified obfuscated form. This is a bad practice just like committing pre-build binaries.
Those files should be composed on build time.
Minified resources are same as pre-compiled binaries -- un-readable, non-modifiable and non-distributable in Debian.

Lately it's been demonstrated how minified JavaScript can become an attack vector (i.e. security breach).

It is hard to check whether minified resources were tampered with.

Finally there is a privacy breach as resources bundled to pages/static have references to external web sites such as google.com and ajax.googleapis.com.

Please replace minified resources with original uncompressed ones (please make sure that there are no external references), compose static .go resources on build-time and avoid committing minified junk to source tree.

Thanks.

@onlyjob
Copy link
Author

onlyjob commented Oct 13, 2015

Ping?
There are more issues with bundled files. There are no sources included for Bootstrap JS/CSS and jQuery (but at least they can be found in web). However I could not find origins of containers_css.go and gcharts_js.go. Also contents of google_jsapi_js.go seems to be copied from https://www.google.com/jsapi but where is the source?? Seems that #304 did it all wrong. :(
Please advise.

@vishh
Copy link
Contributor

vishh commented Oct 13, 2015

cc @vmarmol.

On Mon, Oct 12, 2015 at 5:00 PM, Dmitry Smirnov [email protected]
wrote:

Ping?
There are more issues with bundled files. There are no sources included
for Bootstrap JS/CSS and jQuery (but at least they can be found in web).
However I could not find origins of containers_css.go and gcharts_js.go.
Also contents of google_jsapi_js.go seems to be copied from
https://www.google.com/jsapi but where is the source?? Seems that #304
#304 did it all wrong. :(
Please advise.


Reply to this email directly or view it on GitHub
#879 (comment).

@onlyjob
Copy link
Author

onlyjob commented Oct 13, 2015

Another pitfall of embedding is that it makes build fragile. For instance, Bootstrap v3.3.5 contains

throw new Error('`selector` option must be specified when initializing ' + this.type + ' on the window.document object!')

which breaks compilation as follows if Bootstrap is upgraded in pages/static/bootstrap_min_js.go (due to '`' characters):

src/github.com/google/cadvisor/pages/static/bootstrap_min_js.go:1309: syntax error: unexpected name, expecting semicolon or newline

@jimmidyson
Copy link
Collaborator

Totally agree it's weird to embed the js/CSS in the go files manually but embedding the static content in the final binary is something we would want to keep to make distribution easy. go bindata or rice would be much better ways to do this. Do you fancy submitting a PR to fix this?

@onlyjob
Copy link
Author

onlyjob commented Oct 13, 2015

"Making distribution easy" is not worth it. Why not leave distribution to GNU/Linux distros?
Anyway I'm illiterate in Golang and I'm against bundling resources like this (see #304 (comment)) so I can't prepare a pull request.
Besides there are unidentified undocumented source-less resources so even if I could take care of technical issues I wouldn't know where to get missing sources from...

@jimmidyson
Copy link
Collaborator

Embedding the files in the binary means you don't have to worry about packaging debs/rpms/etc. You can just ship the binary & that is portable including static resources. This is a pretty common thing to do in the go world.

Totally agree that the way cadvisor currently does this needs improvement. I'll see if I get time to take a look.

@onlyjob
Copy link
Author

onlyjob commented Oct 13, 2015

... This is a pretty common thing to do in the go world.

Frankly it is a terrible practice -- too bad if it is "common". We do need proper packages and personally I won't install anything that is not packaged. I find it uncomfortable to have humongous "everything-included" executables with no security support... :(
By the way this problem was discovered during packaging...

Totally agree that the way cadvisor currently does this needs improvement. I'll see if I get time to take a look.

Thank you.

@jimmidyson
Copy link
Collaborator

While taking a quick look at this (go rice looks promising btw) I noticed that locally hosting the JS for the charting library breaks Google's ToS: see both https://developers.google.com/chart/interactive/faq#offline & https://developers.google.com/chart/interactive/faq#localdownload.

@vishh Thoughts? Do we need to remove the local copies of gcharts & jsapi so as not to break Google's (your employer's) ToS in one of their own OSS projects?

@vishh
Copy link
Contributor

vishh commented Oct 13, 2015

:) I'm fine with removing it from the binary. I'm not sure why it got added
in the first place.

On Tue, Oct 13, 2015 at 1:49 AM, Jimmi Dyson [email protected]
wrote:

While taking a quick look at this (go rice
https://github.com/GeertJohan/go.rice looks promising btw) I noticed
that locally hosting the JS for the charting library breaks Google's ToS:
see both https://developers.google.com/chart/interactive/faq#offline &
https://developers.google.com/chart/interactive/faq#localdownload.

@vishh https://github.com/vishh Thoughts? Do we need to remove the
local copies of gcharts & jsapi so as not to break Google's (your
employer's) ToS in one of their own OSS projects?


Reply to this email directly or view it on GitHub
#879 (comment).

@onlyjob
Copy link
Author

onlyjob commented Nov 10, 2015

This is still an outstanding issue... :(

@muellert
Copy link

muellert commented Jul 29, 2022

This is still an outstanding issue... :(

Seconded. In particular, it seems to make it impossible to use the prometheus-nginx-exporter on Debian, as it forces the Debian package of cadvisor to have the web ui disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants