Digest auth new #2098

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.

Sign up for GitHub

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

Jump to bottom

Open

pratt4 wants to merge 8 commits into AsyncHttpClient:main from pratt4:digest-auth-new

Contributor

pratt4 commented Jun 10, 2025 •

edited

Loading

This is build on top of #2089

and still some changes are required around new testcases and failing testcases

closes #2068

hyperxpro and others added 2 commits

May 11, 2025 00:57


          Implement HTTP Digest Access Authentication

6222d4b


          RFC 7616: honour charset=UTF-8 in Digest auth AsyncHttpClient#2068

867f50e

pratt4 marked this pull request as draft

June 10, 2025 14:42


          Merge branch 'main' into digest-auth-new

Member

hyperxpro commented Jun 11, 2025

I'll have a look on the weekend. :)

pratt4 force-pushed the digest-auth-new branch 2 times, most recently from db10c8e to 735ab1c Compare

June 15, 2025 07:12


          RFC 7616: fixed failing testcases in pipelines

126404c

pratt4 force-pushed the digest-auth-new branch from 735ab1c to 126404c Compare

June 15, 2025 07:30

Member

hyperxpro commented Jun 16, 2025

I think only the auth-int implementation is not complete.

Contributor Author

pratt4 commented Jun 18, 2025

Thanks @hyperxpro for pointing that out! I’ve started implementing auth-int
I will push phase 1 in 1-2 days

pratt4 and others added 3 commits

July 18, 2025 00:46


          Merge branch 'main' into digest-auth-new

3536f37


          RFC 7616: added support to auth-int (phase-1)AsyncHttpClient#2068

d3a6fb5


          Merge branch 'main' into digest-auth-new

2815a12

pratt4 mentioned this pull request

any plan for RFC7616? #2068

Open

Contributor Author

pratt4 commented Jul 20, 2025

things to do

Support unlimited size request bodies using streaming: implement DigestingBodyGenerator that hashes while streaming (no upfront buffer), lifting the 10 MB limit
Adding a repeatability contract (e.g... isRepeatable()) to BodyGenerator so client auto-selects buffering or streaming modes
Optimization of FileBodyGenerator hashing by using zero-copy techniques (FileChannel + ByteBuffer or mmmap slices).
seamless auth-int support for ReactiveStreams and multipart body generators, preserving async back-pressure.
Negotiation of strongest hash algorithm automatically when server advertises multiple options (SHA-512-256 first, then SHA-256, then MD5).
Implementation of the RFC 7616 userhash=true parameter and handle UTF-8 encoded usernames properly.
Performance improvements on hex-encoding method with lookup-table-based implementation and consider mmap-based hashing.
Integration tests

Contributor Author

pratt4 commented Jul 20, 2025 •

edited

Loading

@hyperxpro i have fews questions it would be helpfull if you answer them

1)in Proxy auth is auth-int relevant? because as far as i know proxies happen before getting body
2) currently i dont think the code increments nonce infact it uses DEFAULT_NC=00000001
that means every request that uses Digest today sends nc="00000001". Most servers tolerate that because they don’t enforce nonce-count, but a strict implementation (especially when it also requires auth-int) will expect the counter to rise.
shall we increment it or it is kept default for some reason?

Thanks!

pratt4 commented

View reviewed changes

client/src/main/java/org/asynchttpclient/util/MessageDigestUtils.java

+                   * @return 2×length lower-case hex string
+                   * @throws IllegalArgumentException if {@code bytes} is null
+                   */
+                  public static String bytesToHex(byte[] bytes) {

Contributor Author

pratt4 Jul 20, 2025

need to optimize this

Member

hyperxpro commented Jul 25, 2025

Thanks. I will try to review it ASAP.

pratt4 marked this pull request as ready for review

August 9, 2025 18:25

Contributor Author

pratt4 commented Aug 15, 2025

Hi @hyperxpro
can you please review this whenever you get time..
so that i can quickly wrap this up as soon as possible

Thanks!

Member

hyperxpro commented Aug 16, 2025

@pratt4 Sorry, I was behind schedule. Let me take this up now.

hyperxpro requested a review from Copilot

August 16, 2025 18:38

This comment was marked as outdated.

Sign in to view

hyperxpro requested a review from Copilot

August 16, 2025 18:46

This comment was marked as outdated.

Sign in to view

hyperxpro reviewed

View reviewed changes

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                      MessageDigest md = MessageDigestUtils.pooledMessageDigest(hashAlgorithm);
+                      try {
+                          md.update(responseInput.getBytes(StandardCharsets.ISO_8859_1));

Member

hyperxpro Aug 16, 2025

We can reference wireCharset.

hyperxpro requested changes

View reviewed changes

Member

hyperxpro left a comment

Few comments

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                          md.update(responseInput.getBytes(StandardCharsets.ISO_8859_1));
+                          return MessageDigestUtils.bytesToHex(md.digest());
+                      } finally {
+                          md.reset();

Member

hyperxpro Aug 16, 2025

We don't need to reset MessageDigest since we will return the hash and it won't be reused anymore.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                              // For -sess: HA1 = H(H(username:realm:password):nonce:cnonce)
+                              String sessInput = ha1 + ":" + realm.getNonce() + ":" + realm.getCnonce();
+                              md.reset();
+                              md.update(sessInput.getBytes(StandardCharsets.ISO_8859_1));

Member

hyperxpro Aug 16, 2025

Use wireCs.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                          return ha1;
+                      } finally {
+                          md.reset();

Member

hyperxpro Aug 16, 2025

Reset is not required.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                              md.update(bb);
+                              return MessageDigestUtils.bytesToHex(md.digest());
+                          } finally {
+                              md.reset();

Member

hyperxpro Aug 16, 2025

Reset is not required.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                              md.update(request.getStringData().getBytes(charset));
+                              return MessageDigestUtils.bytesToHex(md.digest());
+                          } finally {
+                              md.reset();

Member

hyperxpro Aug 16, 2025

Reset is not required.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                              return MessageDigestUtils.bytesToHex(md.digest());
+                          } finally {
+                              md.reset();

Member

hyperxpro Aug 16, 2025

Reset is not required.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                              md.update(request.getByteData());
+                              return MessageDigestUtils.bytesToHex(md.digest());
+                          } finally {
+                              md.reset();

Member

hyperxpro Aug 16, 2025

Reset is not required.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                          throw new RuntimeException("Failed to hash request body", ioe);
+                      } finally {
+                          try {
+                              md.reset();

Member

hyperxpro Aug 16, 2025

Reset is not required.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                          return MessageDigestUtils.bytesToHex(md.digest());
+                      } catch (IOException ioe) {
+                          throw new RuntimeException("Failed to hash request body", ioe);

Member

hyperxpro Aug 16, 2025

This should be IOException not RuntimeException, because it is caused by an IO operation.

client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java

+                              md.update(bytes);
+                              return MessageDigestUtils.bytesToHex(md.digest());
+                          } catch (IOException ioe) {
+                              throw new RuntimeException("Failed to read file for auth-int hash", ioe);

Member

hyperxpro Aug 16, 2025

This should be IOException not RuntimeException, because it is caused by an IO operation.

Member

hyperxpro commented Aug 16, 2025

@pratt4 I will merge this into another branch where I will test it with the actual server implementation and see how it behaves before finally pushing to the main branch. It also requires some code formatting.

Overall, great work! Thanks a lot :)


          Merge branch 'main' into digest-auth-new

2c8a0f5

Contributor Author

pratt4 commented Aug 18, 2025

@hyperxpro thanks for the review!

I’m happy to fix the formatting issues myself if you can point them out along with the suggested changes

and about testing in another branch.... i was just curious,
would that mean merging this PR into a staging/test branch, or pulling the code manually?
I only ask to make sure it doesn’t end up as a “zombie PR”(if code is pulled manually) that will be harder for me to track later in future.

Also, the improvements which I mentioned earlier in the previous comments for that i will create a separate PR,
so this one stays focused and not overloaded.

Member

hyperxpro commented Aug 21, 2025

It won't be a zombie one; it will ship with the next release, but I need to test it. We don't have "SNAPSHOT" maven for this, so merging to a test branch is the only option before confirming full compliance with other implementations.

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

Labels

None yet