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

http authentication in esp_http_server (IDFGH-2757) #4823

Closed
wants to merge 12 commits into from

Conversation

jaghatei
Copy link

@jaghatei jaghatei commented Feb 23, 2020

This pull provides http authentication to the esp_http_server.
Function for basic auth is included. Other auth functions can be provided by user.
Normal URI handler will only be called in case authentication is disabled or a valid user id is found and authorized.
Authentication is disabled if: AuthType HTTP_AUTH_NONE or UserPassFn == NULL.
AuthType HTTP_AUTH_BASIC uses internal auth function.
AuthType HTTP_AUTH_USER uses user provided function if reference is provided in field UserAuthFn.

Configuration is handled via uri config:
static const httpd_uri_t favicon = {
.uri = "/favicon.ico",
.method = HTTP_GET,
.handler = favicon_get_handler,
.user_ctx = NULL,
.UserPassFn = myPassFn,
.UserAuthFn = NULL,
.AuthType = HTTP_AUTH_BASIC,
.auth_realm = "yourhttprealm"
};

handler function can check result of authentication in req->auth_user_id.
-1: auth disabled
values > = 0: id of authenticated user
Results -2 ( no auth provided by client) and -3 (no user authenticated) of internal basic auth will close request and not call handler function.
Your own UserAuthFn may react different here but needs to provide return values >=-1 then.

@github-actions github-actions bot changed the title http authentication in esp_http_server http authentication in esp_http_server (IDFGH-2757) Feb 23, 2020
@Alvin1Zhang
Copy link
Collaborator

@jaghatei Thanks for your contribution.

@CLAassistant
Copy link

CLAassistant commented Mar 13, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@xkevin190
Copy link

is this ready? What is missing to implement?

@jaghatei
Copy link
Author

jaghatei commented Jun 13, 2020

From my point this is ready for merge. But this needs to be done by "authorized" users...

@MihirShahLNM
Copy link

MihirShahLNM commented Jul 29, 2020

I get the error: implicit declaration of function 'mbedtls_base64_decode'

C:/esp-idf/components/esp_http_server/src/httpd_uri.c: In function 'httpd_basic_auth':
C:/esp-idf/components/esp_http_server/src/httpd_uri.c:305:3: error: implicit declaration of function 'mbedtls_base64_decode' [-Werror=implicit-function-declaration]
   mbedtls_base64_decode((unsigned char*)&userpass,sizeof(userpass),(size_t*)&r,(const unsigned char*)&hdr +6,strlen(hdr)-6);
   ^~~~~~~~~~~~~~~~~~~~~

You must have ......

#include "mbedtls/base64.h"

in "httpd_uri.c"

@MihirShahLNM
Copy link

Further, if you do not specify following in the configuration & mcu panics & restarts !

.UserPassFn = myPassFn,
.UserAuthFn = NULL,
.AuthType = HTTP_AUTH_BASIC,
.auth_realm = "yourhttprealm"

@shahpiyushv
Copy link
Collaborator

@jaghatei thanks for the contribution and sorry for the late reply. Can you have a look at our contribution guidelines and make appropriate changes. A couple of key points are:

  • The nomenclature should be inline with what is already used (Eg. user_pass_fn instead of UserPassFn).
  • An example to show how this should be used will be useful.
  • Better to keep this feature disabled by default.

I will provide other comments on the actual code separately.

@jaghatei
Copy link
Author

jaghatei commented Aug 2, 2020

@shahpiyushv: will look into these points
@MihirShahLNM:
will check for embedtls include.
But for the mcu panic:
Does this happen when HTTPD_AUTH_SUPPORT is Y or N on Kconfig?
It should not when set to N.

@Lisa999
Copy link

Lisa999 commented Oct 30, 2020

@jaghatei : Any progress yet?
Would be nice to have this...

@lhespress
Copy link
Collaborator

@jaghatei

I don't think we need modify both httpd_uri.c and esp_http_server.h file for the function, it's better to add it by user.

Please reference the code of #5646.

@AxelLin
Copy link
Contributor

AxelLin commented Aug 12, 2021

@jaghatei

I don't think we need modify both httpd_uri.c and esp_http_server.h file for the function, it's better to add it by user.

Please reference the code of #5646.

@lhespress

The http_auth_basic in components/esp_http_client/lib/include/http_auth.h
is in PRIV_INCLUDE_DIRS of esp_http_client component.
So the application code cannot re-use the http_auth_basic() function.
e.g. In #5646 (comment)
it implements http_auth_basic again.

Suggest to move http_auth_basic out of PRIV_INCLUDE_DIRS.

@lhespress
Copy link
Collaborator

@AxelLin You can configure the information as follows when call esp_http_client_init

const char                  *username;           /*!< Using for Http authentication */
const char                  *password;           /*!< Using for Http authentication */
esp_http_client_auth_type_t auth_type;           /*!< Http authentication type, see `esp_http_client_auth_type_t` */

or change it by

esp_http_client_set_username
esp_http_client_set_password
esp_http_client_set_authtype

@AxelLin
Copy link
Contributor

AxelLin commented Aug 12, 2021

@AxelLin You can configure the information as follows when call esp_http_client_init

The thing is to use http_auth_basic() for httpd application, not for http client.

@lhespress
Copy link
Collaborator

@AxelLin I still suggest you reference the code #5646 (comment) for httpd application.

@AxelLin
Copy link
Contributor

AxelLin commented Sep 13, 2021

@AxelLin I still suggest you reference the code #5646 (comment) for httpd application.

I have no problem to implement it by myself.
I just point out the code http_auth_basic() can be re-used by both http-client and http-server.

@lhespress
Copy link
Collaborator

@AxelLin Thanks for your understanding, I suggest it because of if we make HTTP client API public then "HTTP server application" will create additional dependency on "HTTP client" component, which does not look very good. Of course, we will do it if we have a component which share some common utilities between HTTP client and server applications.

@baldhead69
Copy link

baldhead69 commented Sep 15, 2021

Hi,

The "suggestion code" from @jaghatei seems very interesting to me.

Why hasn't it been implemented yet ?
This seems to me a basic requirement for a server.

There are some problems too.
Register users dynamically, password reset(in a local network) if the user forgets the password for example, etc.

Don't forget to consider authentication for websocket(WS) and websocket secure(WSS) too.

I opened a feature request here:
#7566
#7431

Thank's.

@baldhead69
Copy link

Hi,

Could someone suggest the best way to store users and passwords in flash memory ?
I would like to save this data to flash memory encrypted if it's not too complicated.
Before encrypted this data:
user: plain text
password: password hash + salt, (i still don't know how i will do it).

Fat Fs ?
Fat Fs with wear leveling ?
SPIFFS ?
SPIFFS with wear leveling ?
Other ?

Thank's.

@lhespress
Copy link
Collaborator

@jaghatei Thanks for your contribution, but I suggest handle it by application itself.

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally labels Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.