Webserver: add access to special:// URLs #2443

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
9 participants
@wsnipex
Member

wsnipex commented Mar 15, 2013

This adds a new HTTP handler allowing for special:// vfs files.
Access works analog to VFS via http://xbmchost:port/special/special://path/to/file
It also adds a shortcut URL /log for quick and easy xbmc.log downloading.

.log and .xml files are passed through a User/Pass stripper before download:

  • username:pass in URIs
  • pass in XMLs

I added VS2010 project files, but did not test them.
Since Xcode stuff is voodoo to me, I kindly ask someone in the know to help there.

@Montellese

View changes

xbmc/network/httprequesthandler/Makefile
@@ -4,6 +4,7 @@ SRCS=HTTPImageHandler.cpp \
HTTPWebinterfaceAddonsHandler.cpp \
HTTPWebinterfaceHandler.cpp \
IHTTPRequestHandler.cpp \
+ HTTPSpecialHandler.cpp \

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

This list should be alphabetically ordered so please add your new entry where it belongs.

@Montellese

Montellese Mar 15, 2013

Member

This list should be alphabetically ordered so please add your new entry where it belongs.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ else
+ {
+ CLog::Log(LOGDEBUG, "CHTTPSpecialHandler::HandleHTTPRequest: bad request");
+ m_response = "400 Bad Request";

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

No need to set a response here as it won't be used anyway.

@Montellese

Montellese Mar 15, 2013

Member

No need to set a response here as it won't be used anyway.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ ext = ext.ToLower();
+ }
+ /* add a shortcut URL /log[?] pointing to xbmc.log */
+ else if (request.url.substr(0, 4) == "/log" && request.url.size() <= 5)

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

So "/log7" is a valid URL for accessing the xbmc log? ;-)

@Montellese

Montellese Mar 15, 2013

Member

So "/log7" is a valid URL for accessing the xbmc log? ;-)

This comment has been minimized.

@wsnipex

wsnipex Mar 15, 2013

Member

yes ;) it was easier then to test if its /log /logs or /log/

@wsnipex

wsnipex Mar 15, 2013

Member

yes ;) it was easier then to test if its /log /logs or /log/

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

Easier and not 100% correct ;-)

@Montellese

Montellese Mar 15, 2013

Member

Easier and not 100% correct ;-)

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.h
+ virtual int GetPriority() const { return 2; }
+
+private:
+ CStdString m_path;

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

Any reason why they have to be CStdString objects? Personally I prefer std::string wherever possible (will also save you the StdString.h include) and you're using std::string almost everywhere in the implementation anyways.

@Montellese

Montellese Mar 15, 2013

Member

Any reason why they have to be CStdString objects? Personally I prefer std::string wherever possible (will also save you the StdString.h include) and you're using std::string almost everywhere in the implementation anyways.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ {
+ m_response = "";
+ char m_buf[1024];
+ memset(m_buf,'\0',sizeof(m_buf));

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

cosmetics: spaces after the commas makes it easier to read.

@Montellese

Montellese Mar 15, 2013

Member

cosmetics: spaces after the commas makes it easier to read.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ */
+
+#include "HTTPSpecialHandler.h"
+#include "MediaSource.h"

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

I don't think you'll need this include right?

@Montellese

Montellese Mar 15, 2013

Member

I don't think you'll need this include right?

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+#include "URL.h"
+#include "filesystem/File.h"
+#include "network/WebServer.h"
+#include "settings/Settings.h"

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

You shouldn't need this one either.

@Montellese

Montellese Mar 15, 2013

Member

You shouldn't need this one either.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ return MHD_YES;
+ }
+
+ XFILE::CFile *file = new XFILE::CFile();

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

As I already told you I'd prefer if you changed the order of the logic.

  1. Check if the file exists (you don't need the CFile instance for that yet). If not, set the error to MHD_HTTP_NOT_FOUND and return.
  2. Check if you have a .log or a .xml file. If no, go to #3, otherwise go to #4.
  3. As you don't need to run the password removal stuff, save the path of the file and set HTTPFileDownload as the response type and return.
  4. As you need to run the password removal stuff, you can now create the CFile instance and open the file for reading and whatever other magic you perform.
@Montellese

Montellese Mar 15, 2013

Member

As I already told you I'd prefer if you changed the order of the logic.

  1. Check if the file exists (you don't need the CFile instance for that yet). If not, set the error to MHD_HTTP_NOT_FOUND and return.
  2. Check if you have a .log or a .xml file. If no, go to #3, otherwise go to #4.
  3. As you don't need to run the password removal stuff, save the path of the file and set HTTPFileDownload as the response type and return.
  4. As you need to run the password removal stuff, you can now create the CFile instance and open the file for reading and whatever other magic you perform.
@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ {
+ user = regex1.GetReplaceString("\\1");
+ pass = regex1.GetReplaceString("\\2");
+ replace = string("://") + string(user) + ":" + string(pass) + "@";

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

Do you really need the string() around "://" etc? concatenating const char* with string should be possible. Especially "string(user)" and "string(pass)" don't make sense as "user" and "pass" already are of type string.

@Montellese

Montellese Mar 15, 2013

Member

Do you really need the string() around "://" etc? concatenating const char* with string should be possible. Especially "string(user)" and "string(pass)" don't make sense as "user" and "pass" already are of type string.

This comment has been minimized.

@wsnipex

wsnipex Mar 15, 2013

Member

the string around "://" was a hint I got from someone on IRC, to make sure its a string. It might not be needed, but does it hurt? I will remove the other two, as they are certainly not needed and are a remnant of me chasing the cut off string.

@wsnipex

wsnipex Mar 15, 2013

Member

the string around "://" was a hint I got from someone on IRC, to make sure its a string. It might not be needed, but does it hurt? I will remove the other two, as they are certainly not needed and are a remnant of me chasing the cut off string.

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

Well it's certainly not wrong but IMO unnecessary and confused me for a second.

@Montellese

Montellese Mar 15, 2013

Member

Well it's certainly not wrong but IMO unnecessary and confused me for a second.

This comment has been minimized.

@ghost

ghost Mar 16, 2013

char* + string is well defined to you? for me, only string+char* is.

@ghost

ghost Mar 16, 2013

char* + string is well defined to you? for me, only string+char* is.

This comment has been minimized.

@Montellese

Montellese Mar 16, 2013

Member

As long as it compiles, it's well enough defined for me.

@Montellese

Montellese Mar 16, 2013

Member

As long as it compiles, it's well enough defined for me.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ StringUtils::Replace(line, replace, "xxx");
+ }
+ m_response += line;
+ memset(m_buf,'\0',sizeof(m_buf));

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

cosmetics: spaces after the commas.

@Montellese

Montellese Mar 15, 2013

Member

cosmetics: spaces after the commas.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+
+int CHTTPSpecialHandler::HandleHTTPRequest(const HTTPRequest &request)
+{
+ CStdString ext;

This comment has been minimized.

@Montellese

Montellese Mar 15, 2013

Member

Could be turned into a std::string by using StringUtils::ToLower() on line 49. Then you/we don't need CStdString in this implementation at all.

@Montellese

Montellese Mar 15, 2013

Member

Could be turned into a std::string by using StringUtils::ToLower() on line 49. Then you/we don't need CStdString in this implementation at all.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Mar 15, 2013

Member

The general implementation looks good and something a lot of people have asked for. Just a few things to improve.

Member

Montellese commented Mar 15, 2013

The general implementation looks good and something a lot of people have asked for. Just a few things to improve.

@MartijnKaijser

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
@@ -0,0 +1,133 @@
+/*
+ * Copyright (C) 2011-2013 Team XBMC

This comment has been minimized.

@MartijnKaijser

MartijnKaijser Mar 15, 2013

Member

cosmetics. since it's a new file i think 2011-2013 should just be 2013.

@MartijnKaijser

MartijnKaijser Mar 15, 2013

Member

cosmetics. since it's a new file i think 2011-2013 should just be 2013.

@t-nelson

This comment has been minimized.

Show comment
Hide comment
@t-nelson

t-nelson Mar 15, 2013

Contributor

Aside from easy log access, what do we need this feature for? While it does clean up credentials, there may be URLs/paths you just don't want people having access to (private stuff, porn, etc.). IMO there are still plenty of security and privacy issues at bay here.

Contributor

t-nelson commented Mar 15, 2013

Aside from easy log access, what do we need this feature for? While it does clean up credentials, there may be URLs/paths you just don't want people having access to (private stuff, porn, etc.). IMO there are still plenty of security and privacy issues at bay here.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Mar 16, 2013

Member

implemented the suggested changes. Also added filtering of username in xml tags

Member

wsnipex commented Mar 16, 2013

implemented the suggested changes. Also added filtering of username in xml tags

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ }
+ }
+ /* add a shortcut URL /log and /log/ pointing to xbmc.log */
+ else if ((request.url.substr(0, 4) == "/log" && request.url.size() == 4)

This comment has been minimized.

@Montellese

Montellese Mar 16, 2013

Member

Why don't you just do a string comparison against "/log" and "/log/"?

@Montellese

Montellese Mar 16, 2013

Member

Why don't you just do a string comparison against "/log" and "/log/"?

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+int CHTTPSpecialHandler::HandleHTTPRequest(const HTTPRequest &request)
+{
+ string ext;
+ CLog::Log(LOGDEBUG, "CHTTPSpecialHandler::HandleHTTPRequest: RequestUrl %s", request.url.c_str());

This comment has been minimized.

@Montellese

Montellese Mar 16, 2013

Member

We already have a log output for every incoming HTTP request so this one shouldn't be needed.

@Montellese

Montellese Mar 16, 2013

Member

We already have a log output for every incoming HTTP request so this one shouldn't be needed.

@Montellese

View changes

xbmc/network/httprequesthandler/HTTPSpecialHandler.cpp
+ ext = URIUtils::GetExtension(request.url);
+ StringUtils::ToLower(ext);
+
+ if (! XFILE::CFile::Exists(m_path))

This comment has been minimized.

@Montellese

Montellese Mar 16, 2013

Member

small cosmetic between ! and XFILE.

@Montellese

Montellese Mar 16, 2013

Member

small cosmetic between ! and XFILE.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Mar 16, 2013

Member

Looks much better. Just a few small things and then the code is good IMO.

I don't really have a strong opinion on what @t-nelson said because I have nothing to hide in my local network.

Member

Montellese commented Mar 16, 2013

Looks much better. Just a few small things and then the code is good IMO.

I don't really have a strong opinion on what @t-nelson said because I have nothing to hide in my local network.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Mar 16, 2013

Member

Thanks, code looks good now for me.

Member

Montellese commented Mar 16, 2013

Thanks, code looks good now for me.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Mar 27, 2013

Member

Looking over the commit order I'm a bit confused. Is it just github showing the commits in the wrong order or are they actually in the wrong order? Shouldn't it be

  1. [webserver] add HTTPSpecialHandler
  2. register CHTTPSpecialHandler
  3. add HTTPSpecialHandler to vcxproj

Actually 1 and 2 could be squashed down. As you'll have to rebase anyway, it would be nice if you could add a "[win32] in front of the "add HTTPSpecialHandler to vcxproj" commit message to make it more obvious.

This will also need some xcode monkeying when merged.

Member

Montellese commented Mar 27, 2013

Looking over the commit order I'm a bit confused. Is it just github showing the commits in the wrong order or are they actually in the wrong order? Shouldn't it be

  1. [webserver] add HTTPSpecialHandler
  2. register CHTTPSpecialHandler
  3. add HTTPSpecialHandler to vcxproj

Actually 1 and 2 could be squashed down. As you'll have to rebase anyway, it would be nice if you could add a "[win32] in front of the "add HTTPSpecialHandler to vcxproj" commit message to make it more obvious.

This will also need some xcode monkeying when merged.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Mar 27, 2013

Member

its very possible that I changed the order around while rebasing/squashing the last time. Would it really make a difference though?

I did squash those two down as requested and rebased again.

I'm aware that Xcode stuff is missing, thats why I asked for help with that in the PR description ;)

Member

wsnipex commented Mar 27, 2013

its very possible that I changed the order around while rebasing/squashing the last time. Would it really make a difference though?

I did squash those two down as requested and rebased again.

I'm aware that Xcode stuff is missing, thats why I asked for help with that in the PR description ;)

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Mar 27, 2013

Member

Thanks. Will if the commits are in the wrong order, the code won't compile for each commit. Usually the order wouldn't matter but if you e.g. bisect to find a bug and you end up between those commits you can't really check if that revision works because you can't compile it.

Member

Montellese commented Mar 27, 2013

Thanks. Will if the commits are in the wrong order, the code won't compile for each commit. Usually the order wouldn't matter but if you e.g. bisect to find a bug and you end up between those commits you can't really check if that revision works because you can't compile it.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Mar 27, 2013

Member
understood, didn't think about that.
Member

wsnipex commented Mar 27, 2013

understood, didn't think about that.
@NedScott

This comment has been minimized.

Show comment
Hide comment
@NedScott

NedScott Apr 2, 2013

Contributor

@t-nelson From what I can see, there are multiple troubleshooting situations where accessing files (and more than just xbmc.log) could be useful in this case. More and more, we have situations where it's not entirely clear where users can look at and check these files (android seems to have a few variations for the userdata folder, for example). In that sense, I would say this would be a troubleshooting "mode" for many. I love the idea of being able to do this, and can see it making a lot of situations easier to troubleshoot.

@wsnipex Forgive me, as I'm not a programmer and am not able to tell from the commits, but I assume there's a way to turn special protocol access on or off? One that defaults to off? I also know everyone cringes at GUI settings additions, but if there is a setting for this, I think it should be in the GUI (if you can't find xbmc.log, then you probably can't find advancedsettings.xml).

Contributor

NedScott commented Apr 2, 2013

@t-nelson From what I can see, there are multiple troubleshooting situations where accessing files (and more than just xbmc.log) could be useful in this case. More and more, we have situations where it's not entirely clear where users can look at and check these files (android seems to have a few variations for the userdata folder, for example). In that sense, I would say this would be a troubleshooting "mode" for many. I love the idea of being able to do this, and can see it making a lot of situations easier to troubleshoot.

@wsnipex Forgive me, as I'm not a programmer and am not able to tell from the commits, but I assume there's a way to turn special protocol access on or off? One that defaults to off? I also know everyone cringes at GUI settings additions, but if there is a setting for this, I think it should be in the GUI (if you can't find xbmc.log, then you probably can't find advancedsettings.xml).

@t-nelson

This comment has been minimized.

Show comment
Hide comment
@t-nelson

t-nelson Apr 3, 2013

Contributor

@NedScott clearly I'm aware of what it's useful for. My concern is whether it's useful enough to risk a gaping hole in security. My comment was mainly ensure this had been considered.

Contributor

t-nelson commented Apr 3, 2013

@NedScott clearly I'm aware of what it's useful for. My concern is whether it's useful enough to risk a gaping hole in security. My comment was mainly ensure this had been considered.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 3, 2013

Member

I don't see a gaping hole. You can always enable user/pass for the webserver. Also most users, me included don't have anything to hide on their own LAN

Member

wsnipex commented Apr 3, 2013

I don't see a gaping hole. You can always enable user/pass for the webserver. Also most users, me included don't have anything to hide on their own LAN

@arnova

This comment has been minimized.

Show comment
Hide comment
@arnova

arnova Apr 3, 2013

Member

Ah so potentionally rendering your router's firewall useless doesn't matter?! It's your first line of defense...

Member

arnova commented Apr 3, 2013

Ah so potentionally rendering your router's firewall useless doesn't matter?! It's your first line of defense...

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 3, 2013

Member

I don't follow. What has xbmcs webserver to do with your router?

Member

wsnipex commented Apr 3, 2013

I don't follow. What has xbmcs webserver to do with your router?

@NedScott

This comment has been minimized.

Show comment
Hide comment
@NedScott

NedScott Apr 3, 2013

Contributor

I believe the security concern is valid. This needs a switch, and it needs to be off by default.

Contributor

NedScott commented Apr 3, 2013

I believe the security concern is valid. This needs a switch, and it needs to be off by default.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 3, 2013

Member

which security issues are we talking about exactly? If there is more data to be sanitized, I'd be happy to do so, but I won't implement a gui option.

Member

wsnipex commented Apr 3, 2013

which security issues are we talking about exactly? If there is more data to be sanitized, I'd be happy to do so, but I won't implement a gui option.

@NedScott

This comment has been minimized.

Show comment
Hide comment
@NedScott

NedScott Apr 3, 2013

Contributor

Off the top of my head, addon_data. Some add-ons store plain text passwords for log-ins in various ways. The log file itself tells everyone on the network what you're watching and when, which is more a privacy concern than a computer security concern, but still an issue for some users.

I think this would be a bad idea if there was no way to turn it off.

Contributor

NedScott commented Apr 3, 2013

Off the top of my head, addon_data. Some add-ons store plain text passwords for log-ins in various ways. The log file itself tells everyone on the network what you're watching and when, which is more a privacy concern than a computer security concern, but still an issue for some users.

I think this would be a bad idea if there was no way to turn it off.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 3, 2013

Member

You can turn it off, turn off the webserver. I really do not understand the issue here. The webserver can be password protected....

edit: @NedScott plz provide concrete examples

Member

wsnipex commented Apr 3, 2013

You can turn it off, turn off the webserver. I really do not understand the issue here. The webserver can be password protected....

edit: @NedScott plz provide concrete examples

@NedScott

This comment has been minimized.

Show comment
Hide comment
@NedScott

NedScott Apr 3, 2013

Contributor

I could provide specific examples of add-ons storing passwords in different ways, but we can't predict them all. Password protecting the webserver is a valid point, but I'm still unsure why file access can't be disabled separately. I doubt most people would think to use a password on the webserver, or would be aware of what they are exposing on their local network. I'm certainly no network expert, so maybe it's nothing to be worried about.

Contributor

NedScott commented Apr 3, 2013

I could provide specific examples of add-ons storing passwords in different ways, but we can't predict them all. Password protecting the webserver is a valid point, but I'm still unsure why file access can't be disabled separately. I doubt most people would think to use a password on the webserver, or would be aware of what they are exposing on their local network. I'm certainly no network expert, so maybe it's nothing to be worried about.

@t-nelson

This comment has been minimized.

Show comment
Hide comment
@t-nelson

t-nelson Apr 3, 2013

Contributor

I'll more or less shut up if you enable this only when "enable debugging"
is ticked in GUI settings AND a password is set for HTTP access. A dialog
probably would not be a bad idea either. Though we should probably have one
stating the security/privacy implications of debug logging in the first
place.

Contributor

t-nelson commented Apr 3, 2013

I'll more or less shut up if you enable this only when "enable debugging"
is ticked in GUI settings AND a password is set for HTTP access. A dialog
probably would not be a bad idea either. Though we should probably have one
stating the security/privacy implications of debug logging in the first
place.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 7, 2013

Member

Since depending on "debugging enabled via Gui setting" would defy most of the usefulness, I added a gui setting to enable access to special:// urls

Member

wsnipex commented Apr 7, 2013

Since depending on "debugging enabled via Gui setting" would defy most of the usefulness, I added a gui setting to enable access to special:// urls

@t-nelson

This comment has been minimized.

Show comment
Hide comment
@t-nelson

t-nelson Apr 7, 2013

Contributor

How would it defy the usefulness? A non-debug log is useless, so enabling
debug is likely a step that will be required anyway.

Besides, we don't want more GUI settings.

Contributor

t-nelson commented Apr 7, 2013

How would it defy the usefulness? A non-debug log is useless, so enabling
debug is likely a step that will be required anyway.

Besides, we don't want more GUI settings.

@arnova

This comment has been minimized.

Show comment
Hide comment
@arnova

arnova Apr 7, 2013

Member

A GUI setting for this: no way...

Member

arnova commented Apr 7, 2013

A GUI setting for this: no way...

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 7, 2013

Member

I never enable debugging via Gui setting, only via advancedsettings, because I hate the overlay.
So either "if debug logging is enabled" or a Gui setting for this or always enabled.

You guys were lamenting about the "huge security risk", so a GUI setting is the correct solution.

Member

wsnipex commented Apr 7, 2013

I never enable debugging via Gui setting, only via advancedsettings, because I hate the overlay.
So either "if debug logging is enabled" or a Gui setting for this or always enabled.

You guys were lamenting about the "huge security risk", so a GUI setting is the correct solution.

@elupus

This comment has been minimized.

Show comment
Hide comment
@elupus

elupus Apr 7, 2013

Member

I'm for the enablement by the debug gui setting since it shows that you are running in a non standard mode. If we where to accept a GUI setting it would also need to indicate that it's a security risk doing so. User would have no idea that it was with the current title of this setting.

Member

elupus commented Apr 7, 2013

I'm for the enablement by the debug gui setting since it shows that you are running in a non standard mode. If we where to accept a GUI setting it would also need to indicate that it's a security risk doing so. User would have no idea that it was with the current title of this setting.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 7, 2013

Member

then what do you suggest for the settings text?
again the debug gui setting is useless to me and many users.

Member

wsnipex commented Apr 7, 2013

then what do you suggest for the settings text?
again the debug gui setting is useless to me and many users.

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Apr 7, 2013

Member

If the issue is just wanting to expose the log to the network, then why not just expose the log and leave it at that? Is it a security issue to expose only the (stripped) log?

Member

jmarshallnz commented Apr 7, 2013

If the issue is just wanting to expose the log to the network, then why not just expose the log and leave it at that? Is it a security issue to expose only the (stripped) log?

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 16, 2013

Member

thats the question here. I don't see a security issue in exposing the stripped log(or other files), @t-nelson and others obviously do.

If log only is agreeable, fine for me, but I do not want it dependent on having debugging enabled via GUI setting. This is my only requirement, as it makes my own usecase moot.

Member

wsnipex commented Apr 16, 2013

thats the question here. I don't see a security issue in exposing the stripped log(or other files), @t-nelson and others obviously do.

If log only is agreeable, fine for me, but I do not want it dependent on having debugging enabled via GUI setting. This is my only requirement, as it makes my own usecase moot.

@topfs2

This comment has been minimized.

Show comment
Hide comment
@topfs2

topfs2 Apr 17, 2013

Member

Would it make sense to add it to JSON-RPC instead? That way we can add it as a special permission?

Member

topfs2 commented Apr 17, 2013

Would it make sense to add it to JSON-RPC instead? That way we can add it as a special permission?

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 17, 2013

Member

@topfs2: Where would the security be through JSON-RPC? We have permissions but we don't do anything with them at the moment. Furthermore for an average joe user it's much easier to open a browser URL to retrieve the log file from a "remote" machine than having to execute a JSON-RPC request etc.

Member

Montellese commented Apr 17, 2013

@topfs2: Where would the security be through JSON-RPC? We have permissions but we don't do anything with them at the moment. Furthermore for an average joe user it's much easier to open a browser URL to retrieve the log file from a "remote" machine than having to execute a JSON-RPC request etc.

@topfs2

This comment has been minimized.

Show comment
Hide comment
@topfs2

topfs2 Apr 17, 2013

Member

Hehe, I meant that then we have it more under control, even if the permissions aren't used yet :)
And an url we could still create with a webinterface, e.g. http://IP:PORT/logs where that webinterface prettifies the log output from jsonrpc.

Member

topfs2 commented Apr 17, 2013

Hehe, I meant that then we have it more under control, even if the permissions aren't used yet :)
And an url we could still create with a webinterface, e.g. http://IP:PORT/logs where that webinterface prettifies the log output from jsonrpc.

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