Repository navigation
fix(redact): a login in a logged URL is masked - #198
Conversation
lib.redact masked JSON members and key=value pairs but not a URL's userinfo, so rtsp://user:pass@host reached a log whole. The text path now masks the login up to the last @ before the host, as a browser reads it, and leaves an @ in a path, query or fragment alone.
![svc-finitelabs[bot]](https://avatars.githubusercontent.com/in/3141848?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approving. The login pattern takes the last @ before /, ?, #, whitespace or ", so I probed what sits on either side of that: an IPv6 host (http://user:PASS@[2001:db8::1]:554/live), an uppercase scheme and a percent-encoded @ in the password all mask the login and keep the host. Beyond CI, I mutated the class one exclusion at a time (dropping ", %s, and ?#). Each mutation fails a different case in the new "stays readable" section, so every character the class excludes has a test that pins it.
| end) | ||
| -- A URL's login: rtsp://user:pass@host. As a browser reads it, the login runs to the last "@" before the | ||
| -- host ends at "/", "?" or "#", so a raw "@" in a password is masked whole and one in a query is left alone. | ||
| text = text:gsub('(://)[^/?#%s"]+@', "%1" .. REDACTED .. "@") |
There was a problem hiding this comment.
Nit: the class stops at " but not at ' or ,, so a single-quoted or comma-separated URL that is followed by an email loses its host: url='http://host',user=a@b.com becomes url='http://***REDACTED***@b.com. That fails closed, so nothing leaks, but the line no longer says which host it was. Adding ' next to " ([^/?#%s"']+@) covers the quoted case.
There was a problem hiding this comment.
Declined, and pinned with a test in b66dede. ' and , are sub-delims, legal raw in a URL login (RFC 3986 section 3.2.1), so a class that stops at either prints a password holding one: with [^/?#%s"'], rtsp://viewer:it's@192.0.2.20/live comes out whole. The current class can hide the host in your quoted case but never a secret, and failing closed is the side lib.redact takes. " is different: it can't stand raw in a URL, and in JSON text it ends the string. The new check "a raw ' or , in the password masked whole" fails if either character is added to the class.
Both may stand raw in a login, so a class that stops at either prints the password.
There was a problem hiding this comment.
The new check pins the class against both characters with one string: adding either ' or , alone to the class in redact.lua:152 leaves rtsp://viewer:it's,ok_LEAK@192.0.2.20/live printed whole and fails only this check, and it passes at this head. Nothing else changed since my last review.
lib.redactmasked JSON members andkey=valuepairs, but not the login in a URL, sortsp://viewer:pass@192.0.2.20/livereached a log line whole. Home Connect puts camera stream addresses in its bridge traces and masks them with a helper of its own, which misses a?token=in the same URL.What I changed:
redact.text, and soredact.valueon any string or table leaf, masks a URL's login before it masks the pairs. That trace line now readsrtsp://***REDACTED***@192.0.2.20/live?token=***REDACTED***.@before the host ends at/,?or#, as a browser reads it. A raw@in a password is masked whole, and an@in a path, query or fragment, as inhttps://example.com?email=user@example.com, stays readable.lib.httplogs a request's URL throughredact.value, so its Trace line masks a login too.Each driver repo picks this up on its next
copier update, and Home Connect then drops its helper.