feat: Add HTTP Digest Auth as an option for HTTP monitors - #7714
Conversation
|
Thanks for the PR! If anyone would like to help with testing, run: |
|
Hello and thanks for lending a paw to Uptime Kuma! 🐻👋 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // The digest itself must be computed over the raw, unescaped username - an | ||
| // implementation that hashed the escaped wire form instead would fail this. | ||
| const ha1 = crypto.createHash("md5").update(`${username}:example.com:pa:ss:word`).digest("hex"); |
There was a problem hiding this comment.
Use of MD5 is a necessary evil to support the majority of digest auth server implementations.
autocarl
left a comment
autocarl
left a comment
There was a problem hiding this comment.
Reviewed 9c2aca27 as a correctness/interoperability pass. I found five reproducible issues; details are inline. The targeted ESLint run and all 19 added tests pass, but local HTTP verifier probes show that the current tests miss cache-busting, redirects, multiple challenges, qop-less session algorithms, and SHA-512-256. The check suite is otherwise green; CodeQL remains red on the MD5 alert already covered by its bot thread, so I did not duplicate it.
| } | ||
|
|
||
| const params = Object.fromEntries( | ||
| [...challenge.matchAll(/(\w+)=(?:"([^"]*)"|([^\s,]+))/g)].map(([, key, quotedValue, bareValue]) => [ |
There was a problem hiding this comment.
Medium — Parse one authentication challenge at a time. On 9c2aca27, Axios/Node combines multiple WWW-Authenticate fields into one comma-delimited value. This regex consumes everything after the first Digest, and Object.fromEntries then lets parameters from later schemes/challenges overwrite the selected Digest challenge. A local server returning Digest (realm="digest-realm") plus Basic (realm="basic-realm") produced a Digest response for basic-realm and stayed at 401; two Digest challenges likewise select the last parameters even though RFC 7616 orders algorithms by preference. Escaped quoted-pairs are also truncated. Please use a challenge-aware parser (or split rigorously outside quoted strings), unescape quoted values, and select the first supported Digest challenge. Add tests for multiple header fields/schemes, multiple Digest algorithms, and escaped quotes/backslashes.
| const algorithm = (challenge.algorithm || "MD5").toUpperCase(); | ||
| const hash = (value) => | ||
| crypto | ||
| .createHash(algorithm.startsWith("SHA-256") ? "sha256" : "md5") |
There was a problem hiding this comment.
Medium — Do not silently compute unsupported algorithms with MD5. SHA-512-256 is defined by RFC 7616, but this branch emits algorithm=SHA-512-256 while generating a 32-hex-character MD5 response; a local verifier correctly kept the monitor at 401. An arbitrary token such as FOO is mislabeled the same way. Please map supported algorithm tokens explicitly (sha512-256 is available in supported Node versions) and fail clearly for unsupported tokens rather than sending a response computed with another algorithm. Add the RFC SHA-512-256 vector and an unknown-algorithm rejection test.
| `response=${quoted(response)}`, | ||
| ]; | ||
| if (qop) { | ||
| parts.push(`qop=${qop}`, `nc=${nc}`, `cnonce=${quoted(cnonce)}`); |
There was a problem hiding this comment.
Medium — Send the cnonce used by a -sess algorithm even when qop is absent. Lines 69–70 include cnonce in session HA1, but this block puts it on the wire only when qop=auth was selected. For a qop-less MD5-sess challenge, I reproduced a second 401: the server cannot recompute HA1 because the response contains no cnonce (the RFC 2617 ambiguity is corrected by erratum 1649). Emit cnonce whenever the selected algorithm needs it, while keeping qop/nc conditional as appropriate, and add a qop-less MD5-sess verifier test.
| username: this.basic_auth_user, | ||
| password: this.basic_auth_pass, | ||
| method: options.method, | ||
| uri: pathname + search, |
There was a problem hiding this comment.
High — Hash the serialized request target, including Axios params. The monitor adds options.params.uptime_kuma_cachebuster, but this URI is derived only from options.url. I reproduced both requests reaching /health?uptime_kuma_cachebuster=abc while the retry sent uri="/health"; the server computed a different HA2 and returned 401. This makes Digest monitors with Cache Bust enabled stay DOWN against a conforming server. Build the Digest request-target from the same fully serialized Axios config that the adapter sends (including existing query parameters plus options.params) and add an end-to-end test with cache busting enabled.
| }), | ||
| }; | ||
|
|
||
| return this.makeAxiosRequest(options, true); |
There was a problem hiding this comment.
Medium — Bind the retry to the URL that actually issued the challenge. Axios follows redirects before surfacing the final 401, while options.url still names the original monitor URL. In a local /start -> /protected probe, this code hashed /start, restarted at /start, and remained 401. In a cross-origin variant, the retry sent the target realm/nonce response to the original origin, while the challenged target received no Authorization header. Capture and validate the effective response URL (or disable automatic redirects during Digest negotiation), then retry only the challenged target with its exact request-target. Please cover both same-origin and cross-origin redirects.
3 participants
Summary
There is a longstanding request to add support for HTTP digest auth (see #1798 and #6333). This change adds support for digest auth for HTTP(S) monitors, including keyword monitors.
The bulk of the complexity of this PR is in the actual digest auth implementation. I could not find a ready-to-use package to handle the digest parsing and response generation (at least, not to my own satisfaction):
axios-digest-authwould seem to want to own the complete request lifecycle, making integration difficult. It also references an oldaxiosversion.digest-headerhas an obvious bug where passwords cannot contain:. Ignoring this bug, it does not support RFC7616 handshakes using SHA256.node-digest-authagain wants to own the request lifecycle, and is furthermore not based onaxios.indigestiondoes not support RFC7616 handshakes using SHA256.http-auth-utils,auth-header,www-authenticate- these came up in my searching, but they only parse the challenge header and/or format the response, they do not do the hard work of digest computation.Is it a good idea to bake this much new logic into the application itself, rather than accepting the limitations of one of the above libraries (or publishing my own)? Maybe not! The code is structured in a way to isolate this logic to ease future refactors, at the very least.
AI was used to generate the HTTP digest auth implementation, which I reviewed afterwards for correctness and clarity.
I have tested this end-to-end against a real Hikvision NVR (circa 2018, MD5 algorithm) and against lighttpd (MD5 and SHA256 algorithms tested).
To minimize the scope of the PR, the username/password fields are recycled from the basic auth implementation, i.e. the digest auth username/password end up stored in the "basic auth" database columns, etc.
Please follow this checklist to avoid unnecessary back and forth (click to expand)
I understand that I am responsible for and able to explain every line of code I submit.
Screenshots for Visual Changes
New option in the auth dropdown for HTTP(S) and HTTP(S) Keyword monitors.