-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
Hash SSIDs before logging them. #37524
Conversation
PR #37524: Size comparison from 9c8cb33 to 878b444 Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
chip::Encoding::BytesToHex(hashedValue, sizeof(hashedValue), hexValue, sizeof(hexValue), chip::Encoding::HexFlags::kUppercase); | ||
ssidString = [[NSString alloc] initWithBytes:hexValue | ||
length:sizeof(hexValue) | ||
encoding:NSUTF8StringEncoding]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just [string hash] per discussion? This is awfully complicated/expensive (not that the cost matters), what's the value here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said in the discussion if you do [string hash]
then to check whether a given SSID is the one that went through this code you have to run [string hash]
on the exact same operating system version of the same operating system as the one that created the log, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, which is fine.
chip::Encoding::BytesToHex(hashedValue, sizeof(hashedValue), hexValue, sizeof(hexValue), chip::Encoding::HexFlags::kUppercase); | ||
ssidString = [[NSString alloc] initWithBytes:hexValue | ||
length:sizeof(hexValue) | ||
encoding:NSUTF8StringEncoding]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, which is fine.
Per discussion: logging the SSID here is not actually useful, so we'll just stop doing that: #37576 |
Testing
Logging changes only.