babylonhealth/certificate-transparency-android

Enable reporting mode

yschimke opened this issue · 5 comments

Report errors to report-uri whether or not fail on error is enabled

This is what the Logger class if for and left for the user of the library to implement. They then have the flexibility to choose whether to report this via Firebase, Crashlytics or any other reporting tool they choose.

It also means we're not imposing a particular format of message on the users server in order to receive the result, again being up to the user to choose JSON, XML, Protobuf or whatever

There is a standard reporting mechanism in the RFC. Do you plan to implement that?

https://datatracker.ietf.org/doc/draft-ietf-httpbis-expect-ct/

Additionally, the UA SHOULD report Expect-CT failures for hosts for
which it does not have any stored Expect-CT metadata. That is, when
the UA connects to a host and receives an Expect-CT header field
which contains the "report-uri" directive, the UA SHOULD report an
Expect-CT failure if the the connection does not comply with the UA's
CT Policy.

To flip this around. Rather than each user of the library implementing the RFC reporting, would you accept a PR providing that functionality?

Haven't looked at the Expect-CT header RFC yet as this wasn't a key concern in the writing of the library, mainly focussing on RFC-6962 support first. To that end until all three mechanisms of extracting SCTs from the connection is complete, issue #2, the library shouldn't enable Expect-CT support by default.

At the time of writing I imagined most people using the library would be connecting to a handful of known domains in which case enforcing CT explicitly makes sense.

Expect-CT headers have their flaws, mainly that the first request could still be tampered to remove the header (or set a header on a domain where one shouldn't exist which is why Chromium has a preload list at https://chromium.googlesource.com/chromium/src/net/+/master/http/transport_security_state_static.json although as of writing there are only 19 relevant entries on a brief inspection.

With all that said, adding this reporting makes sense when the library has Expect-CT support, and of course PRs on that functionality, or any other is more than welcome.

Closing this issue as moving the work into a project https://github.com/appmattus/certificatetransparency/projects/1#card-55581293