Enhance DigestScheme for RFC 7616 Compliance and Expanded Hash Algorithm Support #597
Conversation
There was a problem hiding this comment.
@arturobernalg Looks good to me.
Please do consider investing some effort into creating compatibility integration tests with Apache HTTPD or Ngnix in httpcore5-testing
| @@ -367,7 +365,7 @@ private String createDigestResponse(final HttpRequest request) throws Authentica | |||
| } | |||
|
|
|||
| // 3.2.2.2: Calculating digest | |||
| if ("MD5-sess".equalsIgnoreCase(algorithm)) { | |||
| if ("MD5-sess".equalsIgnoreCase(algorithm) || "SHA-256-sess".equalsIgnoreCase(algorithm) || "SHA-512-256-sess".equalsIgnoreCase(algorithm)) { | |||
There was a problem hiding this comment.
@arturobernalg You might want to se a set here
| * @deprecated Use {@link DigestScheme#createCnonce(String)} instead. | ||
| */ | ||
| @Deprecated | ||
| static byte[] createCnonce() { |
There was a problem hiding this comment.
@arturobernalg This method is package private. Just delete it.
| * @return the resolved base algorithm name, or the original algorithm name if no mapping applies | ||
| */ | ||
| private String resolveAlgorithm(final String algorithm) { | ||
| if (algorithm == null || algorithm.equalsIgnoreCase("MD5-sess")) { |
There was a problem hiding this comment.
@arturobernalg Maybe we should use an enum to represent supported algorithms?
There was a problem hiding this comment.
@olegk I’d say yes in principle, but enforcing a fixed set of algorithms would restrict client flexibility. Currently, we support any algorithm, as demonstrated in the testDigestAuthenticationWithSHA test, which validates a SHA algorithm. According to RFC 7616, only SHA-256, SHA-512/256, and MD5 are actually supported (with MD5 being discouraged).
There was a problem hiding this comment.
@arturobernalg I am not suggesting this enum be made a part of the public APIs. It could be package private and could be changes whenever required. But it is merely a thought.
|
…thm support and charset Enhanced DigestScheme to support SHA-256, SHA-512/256, algorithms in compliance with RFC 7616. Adjusted cnonce generation for adequate entropy in SHA-256 and SHA-512/256 contexts.
|
I am very surprised that Digest isn't dead, at least in SASL it is dead and has long been superseded by SCRAM... |
@ok2c AFAIK, neither |
@arturobernalg It was a nice-to-have, not a blocker. Go ahead and merge the change-set as is. |
This PR upgrades the DigestScheme implementation to align with RFC 7616 requirements, introducing support for SHA-256 and SHA-512/256 hash algorithms in addition to MD5