Digest Authentication in Webserver Library #3053

Lan-Hekary · 2017-03-16T08:21:37Z

Added and Example called HTTPAdvancedAuth to show the user how to use the auth method
the Normal Basic Auth wasn't affected as it's the default mode in the function call
the user can change the auth relm too
also added a feature to change the auth failure html content

codecov-io · 2017-06-17T23:56:00Z

Codecov Report

❗ No coverage uploaded for pull request base (master@3e9caf7). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master   #3053   +/-   ##
========================================
  Coverage          ?   27.6%           
========================================
  Files             ?      20           
  Lines             ?    3655           
  Branches          ?     678           
========================================
  Hits              ?    1009           
  Misses            ?    2468           
  Partials          ?     178

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e9caf7...35b7c0a. Read the comment docs.

Lan-Hekary · 2017-06-18T00:01:01Z

Hello @igrr , will this feature be added to the Master Branch ??
if it's added please close the Pull request :D

Lan-Hekary · 2017-09-07T23:29:51Z

@igrr any news ??
I've been waiting for a long time now ..
tell me if there is are problem or modification ..

igrr

Sorry, forgot about this MR again. Left some (mostly minor comments below).

One major request is to make sure new functionality does not get linked into the application if Basic auth is used. My suggestion is to add two functions, requestBasicAuthentication and requestDigestAuthentication, keep requestAuthentication for compatibility (making it call requestBasicAuthentication). This way the extra code only gets added if Digest auth is actually used in the sketch.

igrr · 2017-09-08T08:56:38Z

libraries/ESP8266WebServer/src/ESP8266WebServer.cpp

+  if(mode==BASIC_AUTH){
+    sendHeader("WWW-Authenticate", "Basic realm=\"" + _srealm + "\"");
+  }else{
+    _snonce=String(RANDOM_REG32,HEX)+String(RANDOM_REG32,HEX)+String(RANDOM_REG32,HEX)+String(RANDOM_REG32,HEX);


Does String(val, HEX) automatically pad the string to 8 hexadecimal places, or the string can end up being shorter?

Also, it might be a bit nicer to put this into a separate function, a la String getRandomHexString(size_t length).

String(val, HEX) in the original Arduino depends on itoa which does not support padding
But it's not a problem really
the nonce and opaque values are arbitrary in length anyway

but the idea of a function is much better as I need to call it twice ..
I will modify it right away ..

igrr · 2017-09-08T08:58:27Z

libraries/ESP8266WebServer/src/ESP8266WebServer.cpp

-void ESP8266WebServer::requestAuthentication(){
-  sendHeader("WWW-Authenticate", "Basic realm=\"Login Required\"");
-  send(401);
+void ESP8266WebServer::requestAuthentication(HTTPAuthMethod mode, const char* realm, const String& AuthFailMsg){


suggest authFailMsg instead of AuthFailMsg.

igrr · 2017-09-08T08:59:43Z

libraries/ESP8266WebServer/examples/HttpAdvancedAuth/HttpAdvancedAuth.ino

@@ -0,0 +1,45 @@
+#include <ESP8266WiFi.h>


Please add a public domain / CC0 license at the top of the sketch

I copied this sketch from the HttpBasicAuth folder and it does not have a license ..
I am not sure of what should I put here ..
can you give an Example ??

E.g. https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/Blink/Blink.ino#L1-L4

Sorry, i mean public domain license, as in Blink example. It has been mentioned before that using LGPL license on examples is too restrictive. We already have a few such examples, and it would be better to migrate all the examples to CC0/public domain license.

sorry I just copied the license from the next sketch in the folder .
fixed it now

igrr · 2017-09-08T09:01:04Z

libraries/ESP8266WebServer/examples/HttpAdvancedAuth/HttpAdvancedAuth.ino

+
+const char* www_username = "admin";
+const char* www_password = "esp8266";
+const char* www_realm = "Custom Realm";             // allows you to set the realm of authentication Default:"Login Required"


These long comments will not fit into default Arduino IDE window.

igrr · 2017-09-08T09:01:29Z

libraries/ESP8266WebServer/examples/HttpAdvancedAuth/HttpAdvancedAuth.ino

+const char* www_username = "admin";
+const char* www_password = "esp8266";
+const char* www_realm = "Custom Realm";             // allows you to set the realm of authentication Default:"Login Required"
+String AuthFailResponse = "Authentication Failed";    // the Content of the HTML response in case of Unautherized Access Default:empty


same here, variable names in Arduino usually start with lowercase letter

Lan-Hekary · 2017-09-08T18:34:05Z

thanks for the fast reply @igrr 🗡️

Concerning this :

One major request is to make sure new functionality does not get linked into the application if Basic auth is used. My suggestion is to add two functions, requestBasicAuthentication and requestDigestAuthentication, keep requestAuthentication for compatibility (making it call requestBasicAuthentication). This way the extra code only gets added if Digest auth is actually used in the sketch.

The function prototype in the header file is the following
void requestAuthentication(HTTPAuthMethod mode = BASIC_AUTH, const char* realm = NULL, const String& AuthFailMsg = String("") );

it defaults to use the Basic Auth if no parameters is passed to the function call
so the user won't feel the difference.

this code for example exactly works the same as if no modifications were added
if(!server.authenticate(www_username, www_password)) return server.requestAuthentication();

if you are worried about the extra code added .. the Bulk of the Code resides in the function authenticate and I have no Idea how to reduce it if the user prefers a Basic Auth ..
I guess it won't make a big difference in code size .. but it would simplify things a bit ..

please advise on how to proceed I will modify it as you say it if you say so :D

I am working on the other modifications in the meanwhile waiting for a reply to push the last modification

igrr · 2017-09-09T01:08:38Z

Ok, i understand. It's okay to keep the function as is (requestAuthentication with a parameter).

Lan-Hekary · 2017-09-09T08:53:37Z

I've made the modifications you asked for
I wrote a Function to get random hex but I made it a fixed length and it's padded now
String getRandomHexString();
to avoid the hassle of dynamic allocation and to avoid any additional overhead

…tion

Lan-Hekary · 2017-09-16T02:00:57Z

any news ?? @igrr
I think you forgot me again ..

* Add Digest Auth * Check for Opaque and Nonce * Remove Serial Debug and fix Indentation * Added example sketch with documentation,Fixed indentation and Defaults * Digest Authentication minor changes + new padded 32 digit random function * update license to public domain * renaming functions

Lan Hekary and others added 13 commits January 25, 2017 03:52

Add Digest Auth

a4020cc

Check for Opaque and Nonce

9d7397a

Remove Serial Debug and fix Indentation

e8c35d9

Merge remote-tracking branch 'MainESP/master' into DigestAuth

b2bf0f2

Added example sketch with documentation,Fixed indentation and Defaults

28f572b

Merge branch 'master' into DigestAuth

d7fda10

Merge branch 'master' into DigestAuth

12e0d10

Merge branch 'master' into DigestAuth

7338ec5

Merge branch 'master' into DigestAuth

858977b

Merge branch 'Master' into DigestAuth

dd839af

Merge branch 'master' into DigestAuth

4f6de29

Merge branch 'master' into DigestAuth

1ffc141

Merge branch 'master' into DigestAuth

36940cd

Merge branch 'master' into DigestAuth

f91b58c

igrr requested changes Sep 8, 2017

View reviewed changes

Lan-Hekary force-pushed the DigestAuth branch from 7c3fb22 to f91b58c Compare September 9, 2017 09:13

Lan-Hekary and others added 5 commits September 9, 2017 11:15

Digest Authentication minor changes + new padded 32 digit random func…

91d9cc1

…tion

Merge branch 'master' into DigestAuth

e0a9538

update license to public domain

2ea36fb

Merge branch 'master' into DigestAuth

282babb

Merge branch 'master' into DigestAuth

0b945f8

renaming functions

35b7c0a

igrr merged commit eebc5ec into esp8266:master Sep 18, 2017

Lan-Hekary deleted the DigestAuth branch September 19, 2017 06:42

Lan-Hekary restored the DigestAuth branch September 19, 2017 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Digest Authentication in Webserver Library #3053

Digest Authentication in Webserver Library #3053

Lan-Hekary commented Mar 16, 2017

codecov-io commented Jun 17, 2017 •

edited

Loading

Lan-Hekary commented Jun 18, 2017

Lan-Hekary commented Sep 7, 2017

igrr left a comment •

edited

Loading

igrr Sep 8, 2017

igrr Sep 8, 2017

Lan-Hekary Sep 8, 2017

igrr Sep 8, 2017

igrr Sep 8, 2017

Lan-Hekary Sep 8, 2017

igrr Sep 9, 2017

igrr Sep 12, 2017

Lan-Hekary Sep 12, 2017

igrr Sep 8, 2017

igrr Sep 8, 2017

Lan-Hekary commented Sep 8, 2017

igrr commented Sep 9, 2017

Lan-Hekary commented Sep 9, 2017

Lan-Hekary commented Sep 16, 2017

Digest Authentication in Webserver Library #3053

Digest Authentication in Webserver Library #3053

Conversation

Lan-Hekary commented Mar 16, 2017

codecov-io commented Jun 17, 2017 • edited Loading

Codecov Report

Lan-Hekary commented Jun 18, 2017

Lan-Hekary commented Sep 7, 2017

igrr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lan-Hekary commented Sep 8, 2017

igrr commented Sep 9, 2017

Lan-Hekary commented Sep 9, 2017

Lan-Hekary commented Sep 16, 2017

codecov-io commented Jun 17, 2017 •

edited

Loading

igrr left a comment •

edited

Loading