Introduce digest access authentication with SHA-256 #13236

arsenalzp · 2025-06-11T19:24:57Z

This is the draft PR related to #13127 which introduced SHA-256 algorithm for Digest access authentication.
In case this implementation is OK I will add related extension of Credential class and tests, of course.

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

Signed-off-by: Oleksandr Krutko <[email protected]> improve DigestAuthenticator algorithm for SHA-256 Signed-off-by: Oleksandr Krutko <[email protected]> Improve DigestAuthenticator class Signed-off-by: Oleksandr Krutko <[email protected]>

arsenalzp · 2025-06-16T12:05:09Z

Hello,
Are you colleagues satisfied by this solution?
If so I will proceed with tests and SHA256 credentials class

sbordet · 2025-06-16T12:19:16Z

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

@@ -58,7 +58,18 @@ public class DigestAuthenticator extends LoginAuthenticator
    private final ConcurrentMap<String, Nonce> _nonceMap = new ConcurrentHashMap<>();
    private long _maxNonceAgeMs = 60 * 1000;
    private int _maxNC = 1024;
+    private String algorithm = "MD5";


Fields of this class are prefixed with _, so change this to _algorithm.

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

sbordet · 2025-06-16T12:21:53Z

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

        @Override
        public boolean check(Object credentials)
-        {
+        {            


Remove these spaces.

sbordet · 2025-06-16T12:22:04Z

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

            if (credentials instanceof char[])
                credentials = new String((char[])credentials);
            String password = (credentials instanceof String) ? (String)credentials : credentials.toString();
-
+            


Remove spaces.

sbordet · 2025-06-16T12:22:14Z

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

-            }
-            catch (Exception e)
+                return stringEquals(TypeUtil.toString(md.digest(), 16).toLowerCase(), response == null ? null : response.toLowerCase());
+


Remove empty line.

sbordet · 2025-06-16T12:23:24Z

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

            {
                LOG.warn("Unable to process digest", e);
            }
-
-            return false;
+                return false;


Fix indentation.

sbordet · 2025-06-16T12:23:34Z

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

        }
-
+        


Remove spaces.

sbordet · 2025-06-16T12:24:10Z

@arsenalzp looks good, but please follow suggestions in my review.

arsenalzp · 2025-06-18T13:38:12Z

@arsenalzp looks good, but please follow suggestions in my review.

I'm in business trip right now, I will review and do all remaining parts on weekend.
Do we have time yet?

gregw · 2025-06-18T20:41:38Z

@arsenalzp We might make those changes for you so we can get this into a build this week. They are all just formatting and naming. Thanks for the contribution!

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

Applying review comments Co-authored-by: Simone Bordet <[email protected]>

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

more format and name fixes

gregw · 2025-06-18T20:51:12Z

@sbordet I applied your suggestions and fixed some others. Can you re-review if this builds OK?

gregw · 2025-06-18T20:57:01Z

@sbordet dang! Looks like I broke this build, I'll fix somehow

...ty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java

checkstyle

arsenalzp force-pushed the jetty-13127 branch from c92d073 to ef7d0c3 Compare June 11, 2025 19:45

joakime added this to Jetty 12.1.0 Jun 11, 2025

joakime moved this to 👀 In review in Jetty 12.1.0 Jun 11, 2025

joakime requested a review from sbordet June 11, 2025 21:34

joakime added the Enhancement label Jun 11, 2025

joakime requested a review from gregw June 11, 2025 21:35

sbordet requested changes Jun 12, 2025

View reviewed changes

sbordet self-requested a review June 13, 2025 19:54

arsenalzp force-pushed the jetty-13127 branch from b253e9d to 6b24431 Compare June 13, 2025 20:24

sbordet requested changes Jun 16, 2025

View reviewed changes