patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9576, Adding authorization cookie...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9576: Adding authorization cookie management

Submitter:  Giuseppe Modugno <giusloq>
Submitted:  Tue 27 Feb 2018 09:38:58 AM UTC
   
 
Category:  apps Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Planned Release:  None

Jump to the original submission

Tue 13 Mar 2018 11:18:58 AM UTC, comment #13: 


> It appears to me that httpd_cgi_handler() is not called, because the query string is absent

Apparently, I am using 'old style CGI'. Looking at httpd code, I can see that the handler is called.

Mike Kleshov <kleshov>
Group Member
Tue 13 Mar 2018 11:03:54 AM UTC, comment #12: 

Mike, I'm trying to add an authorization layer as per your idea.

You suggested to send username/password in the query string, maybe encrypted with an algorithm that produce a different encrypted text.

Suppose the HTTP request is "GET /protected.html" without a query string. You should redirect the browser to "/login.html" or you should answer with "/login.html" file content (I suppose login.html is the page with the login form).

The current httpd server calls fs_open(file, "/protected.html") function that returns ERR_OK.
It appears to me that httpd_cgi_handler() is not called, because the query string is absent.

How do you avoid replying with the original content of protected.html without a valid authentication value?

Giuseppe Modugno <giusloq>
Fri 02 Mar 2018 07:27:06 PM UTC, comment #11: 

Both sides have valid arguments. Still I would like to make httpd open enough to allow people reading and adding cookies. I  wouldn't want to tightly couple this into its API though. Instead, I think the whole server might need some API cleanup. This won't be done for 2.1.0.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 02 Mar 2018 11:42:40 AM UTC, comment #10: 


> IMHO sending password continuously (encrypted or not) in query string is a bad thing. For example, query strings are saved in History or Bookmark from your browser. You see the query string if you print the page.

That's not true. Requests come from javascript (XMLHttpRequest), so no history, no bookmarks, no URI when printing. You'll have to use a sniffer or fire up browser's debugger, but in this case a cookie is just as visible.

> This is my opinion and I hope you don't think I'm trying to push to apply my patch.

Same here. I'm just trying to present arguments accurately.

Mike Kleshov <kleshov>
Group Member
Fri 02 Mar 2018 10:42:27 AM UTC, comment #9: 


> Come on. It's easy enough to obfuscate the password, mix it with current time or request counter. That's not an issue.


All can be done and I admit the current solution for security is HTTPS/TLS. With this technology, you can send password even in clear.
In situations where you don't have HTTPS/TLS you have a security flaw. Full stop.

Anyway you can try to increase the security as you can. IMHO sending password continuously (encrypted or not) in query string is a bad thing. For example, query strings are saved in History or Bookmark from your browser. You see the query string if you print the page.

I know you can search cookies too, but it seems to me they are a little more hidden then query strings. And they are less annoying. And I think cookies are not stored by the browser if they haven't an expiration date (or a maximum duration).

This is my opinion and I hope you don't think I'm trying to push to apply my patch.

Giuseppe Modugno <giusloq>
Thu 01 Mar 2018 12:02:48 PM UTC, comment #8: 


> One dummy hacker understand immediately that you are continuously sending password to the server and have more possibilities to crack the system.

Come on. It's easy enough to obfuscate the password, mix it with current time or request counter. That's not an issue.

Mike Kleshov <kleshov>
Group Member
Thu 01 Mar 2018 11:14:37 AM UTC, comment #7: 


> Create a single-page web application.


And this is a limitation.

> Protected data is pulled from the server using javascript, request URI must contain ?user=---&pass=---


I know cookies and base64 encoding aren't security technologies, but they are more "hidden" than a query string. One dummy hacker understand immediately that you are continuously sending password to the server and have more possibilities to crack the system.

> , so the server is able to check credentials and grant or deny the request. Username and password validity in login form can be checked in a similar manner. Credentials can be stored in browser's local storage.
> All of this can work with vanilla httpd.


Giuseppe Modugno <giusloq>
Thu 01 Mar 2018 11:08:39 AM UTC, comment #6: 


> However, I'd rather give the httpd the opportunity to add user-defined header lines using some callback. Like that, the change to httpd is minimal and better reusable. Authentication could then be added externally.

Simon, if you give me more details, I can try to change my patch.

Consider that authentication cookie could be needed in several parts of HTTP request processing.

  • At file opening. This is why I call an external callback named httpd_authorized_for_uri(). Here the application tells httpd to continue opening the file requested or another file (login form). Another solution could be to change fs_open() (and fs_custom_open() too) to add auth cookie among parameters. However I decided to not change many core parts of httpd code.
  • During CGI processing. Here I changed httpd_cgi_handler() to include auth cookie among parameters.
  • Maybe during SSI processing, but I'm not familiar with SSI implementation in httpd.


I don't think a simple handler to call when a user specified HTTP header is found in the request could solve the authentication issue. Mostly if you don't have the possibility to save a context in the handler and compare it in fs_open() and httpd_cgi_handler().

I think there are many data in the HTTP request that could be useful during file opening and CGI processing (external to httpd). As well as the auth cookie, there are some other headers such as User-Agent. I know struct http_state is an opaque structure that is better to keep private in httpd, but we could add a "public" structure (included in http_state) that can be passed as an argument.
This http_req structure should contain HTTP request info and all other data that external functions can be used to answer to the request:

  • interesting headers (Auth, User-Agent, ...) fine tuned with macros;
  • pointer to struct fs_file (that is already passed to fs_open/httpd_cgi_handler);
  • pcb, why not (I'm not sure if it's possible to deduce the original IP address of the request and this could be useful);
  • query string;
  • additional headers to add in the reply;
  • ...


IMHO this would simplify API and processing of generic requests. For example, with this "public" and "shared" http_req structure, you need to only enable detection of "Cookie: Auth" header by macro and fs_open_custom() could automatically points to "/login.html" instead of "/index.html" , if the auth cookie is absent or not valid.


int fs_open_custom(struct http_req *req, const char *filename) {
  const char *auth_cookie = req->req_headers[HTTPD_MYHEADER_AUTH_COOKIE];
  const struct fsdata_file *f;
  struct fs_file *file = req->file;

  if (auth_is_valid(auth_cookie)) {
    f = fs_search(filename);
  } else {
    f = fs_search("/login.html");
  }

  if (f != NULL) {
    req->file->data = (const char *)f->data;
    req->file->len = f->len;
    req->file->index = f->len;
    req->file->pextension = NULL;
    req->file->flags = f->flags;
#if HTTPD_PRECALCULATED_CHECKSUM
    req->file->chksum_count = f->chksum_count;
    req->file->chksum = f->chksum;
#endif /* HTTPD_PRECALCULATED_CHECKSUM */
#if LWIP_HTTPD_FILE_STATE
    req->file->state = fs_state_init(file, name);
#endif /* #if LWIP_HTTPD_FILE_STATE */
    return ERR_OK;
  } else {
    return ERR_VAL;  // File not found
  }
}



Giuseppe Modugno <giusloq>
Thu 01 Mar 2018 11:00:22 AM UTC, comment #5: 


> Please, please, suggest one authentication scheme that can be used without changing current httpd.

Create a single-page web application. Protected data is pulled from the server using javascript, request URI must contain ?user=---&pass=---, so the server is able to check credentials and grant or deny the request. Username and password validity in login form can be checked in a similar manner. Credentials can be stored in browser's local storage.
All of this can work with vanilla httpd.

Mike Kleshov <kleshov>
Group Member
Thu 01 Mar 2018 10:41:51 AM UTC, comment #4: 


> I'm not very familiar with various authentication schemes, but I think a number of them can be implemented without having to modify httpd.

Please, please, suggest one authentication scheme that can be used without changing current httpd.

> That's one reason not to apply this patch. The other one is the this: httpd is rather simple, please keep it this way.

Simple, but highly configurabile. My patch can be completely disabled by macros, as usual in lwip. So if you don't want, don't enable it.

Simple, but complete. IMHO a web server that isn't able to manage authentication and user types/sessions (with logout, expiration after some time, and so on) is useless in many situations.

I hope there's a simpler solution to this, but I have seen many users in the past that ask how to add authentication in httpd without a real solution.


Giuseppe Modugno <giusloq>
Thu 01 Mar 2018 10:39:10 AM UTC, comment #3: 


> I'm using cookie to add the session ID in every subsequent HTTP requests, so the server can check if it is valid or not.


Theorically, the authentication cookie should be added through "Set-Cookie" header in HTTP response from the server. In order to keep httpd simple, I use another solution. The session ID is returned in the body of a normal HTTP response (of course, a "generated virtual" file). It's the client that manually adds the cookie with the Javascript instruction:

document.cookie = "Auth=" + json.sid;


And the client could automatically goes to the home page with the correct cookie with the follow instruction:

window.location.replace("index.html");


Giuseppe Modugno <giusloq>
Wed 28 Feb 2018 03:21:44 PM UTC, comment #2: 

I haven't really had the time to look at this patch, yet, but reading the original submission, I tend to not include it.

I'm not opposed to adding support for auth mechanisms and I think I would do this with cookies. However, I'd rather give the httpd the opportunity to add user-defined header lines using some callback. Like that, the change to httpd is minimal and better reusable. Authentication could then be added externally.

I still want to look at the patch though.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 28 Feb 2018 01:07:17 PM UTC, comment #1: 

I'm not very familiar with various authentication schemes, but I think a number of them can be implemented without having to modify httpd. That's one reason not to apply this patch. The other one is the this: httpd is rather simple, please keep it this way.

Mike Kleshov <kleshov>
Group Member
Tue 27 Feb 2018 09:38:58 AM UTC, original submission:  

I tried to add authorization cookie management to httpd. You have to enable LWIP_HTTPD_SUPPORT_AUTH_COOKIE and define HTTP_AUTH_COOKIE_NAME (usually "Auth") and HTTP_LOGIN_FILE (the name of the file that is returned to every HTTP requests without a valid authorization cookie... it is usually the login.html web page to force entering username/password).

The user should supply the function httpd_authorized_for_uri(const char *auth_cookie, const char *uri). The function should return true if the uri can be retrieved with that cookie.

My application creates a new session ID (random number) when the user enters valid login data. The session ID is returned in a JSON "generated file" as the answer to login request. The Javascript running in the client can save the cookie, so it will be used in next requests.

The application could invalidate session IDs that aren't used for long times (httpd_authorized_for_uri can restart the timer).

Another small change is with CGI handler function. In my application I needed to check session ID in CGI handler, because the result changes depending on the type of session ID (normal user, admin, and so on). In my case, I encoded the type of the user inside the session ID.

Maybe the name of macros must be changed to be similar to the others in lwip. I don't know lwip too much to create good macro names. Moreover, I think those three macros must be added to httpd_opts.h with their default values.

Giuseppe Modugno <giusloq>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #43407:  patch.diff added by giusloq (5KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kleshov (Posted a comment)
  • -email is unavailable- added by giusloq (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-02-27 giusloq Attached File- Added patch.diff, #43407

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code