1920732 - Disable OHTTP-only pings on Android (which doesn't have OHTTP support yet)

Status: NEW

(Data Platform and Tools :: Glean: SDK, defect, P4)

Description

[Jan-Erik Rediger [:janerik]]

Right now pings send over OHTTP are only supported on Desktop. It's in the docs.

Your data must be solely collected on Firefox Desktop

But it's possible to define those pings and still submit them on non-Desktop and then they will be send the regular way. Which seems unexpected.
We should probably not allow those to be send.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:chutten, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Alessio Placitelli [:Dexter]]

Chris, this seems a bit concerning especially if we might send things on a non OHTTP stack without noticing. Would you kindly understand what's the impact of this and prioritize it accordingly?

Comment 3

[Chris H-C :chutten]

There are presently three non-test pings with use_ohttp: true in the tree: "serp-categorization", "bounce-tracking-protection", and "user-characteristics". Of the three, both the second and third are defined as part of "gecko" (a word here meaning the redistributable portion of m-c also in use by Thunderbird, and Mozilla's Android browser projects).

Looking at the data, we have 70 "user-characteristics" pings and over 10M "bounce-tracking-protection" pings presently in our tables received since Nov 1.

None of these pings have ping_info or client_info, but they do not have the OHTTP guarantee of us not seeing the sending client's IP address meaning that the IP-derived metadata of these pings are present when otherwise they wouldn't be (geography, ISP name).

I can't judge how important it is that these metadata are derived from the clients' IP addresses instead of the OHTTP redirector's. :pbz, :tjr, as people attached to these pings can you judge this?

Comment 4

[Emma Zühlcke [:emz]]

I wasn't aware that Android doesn't use OHTTP even when that flag is set. This seems quite misleading and only mentioning it in the docs is risky.

The sensitive data review approval for Bounce Tracking Protection was granted under the conditions that we both strip client ID (and use a separate ping) and use OHTTP. Without OHTTP we can't collect this data as-is. We would need to go through another sensitive data review which may or may not deem this additional privacy risk acceptable.

For now, how can we stop the data collection on Android without OHTTP and purge all the data?

Comment 5

[Chris H-C :chutten]

We could do so with Server Knobs, but I don't feel as though there's sufficient urgency to warrant this (the data being collected (IP addresses) is tightly controlled (and its products -- geo, ISP -- aren't too spicy)). I think we can proceed at the usual pace with changes to the pipeline to recognize and drop this information, and changes to the client to make it harder or impossible to submit this data in future.

:pbz, what do you expect the result of calling submit() on a OHTTP-using ping to be when inside a product that doesn't support OHTTP?

Comment 6

[Tom Ritter [:tjr]]

I would expect submission to not take place with some sort of log message; but no crash.

We caught this behavior before we launched, thankfully, so the 70 pings are from opt-in volunteers.

Comment 7

[Emma Zühlcke [:emz]]

Same as Tom, we must not send the data and we should log a warning.
How do I update the event in m-c to not collect when on Android? Maybe we should flip that logic and only collect on Desktop.

Comment 8

[Chris H-C :chutten]

(In reply to Paul Zühlcke [:pbz] from comment #7)

Same as Tom, we must not send the data and we should log a warning.
How do I update the event in m-c to not collect when on Android? Maybe we should flip that logic and only collect on Desktop.

Same way you guard any code in m-c against being run in Android: #ifndef MOZ_WIDGET_ANDROID

Since you've both confirmed this is your preference, I'll file a bug and put up a patch to wrap the two submit calls in that. Data will still be recorded (for now. We'll see if we can be clever in the design for the general solution to this bug) but it'll never leave the client.

(Not sure if gecko can discern whether it's being built as a Firefox Desktop app or not. If so, I'd guess that'd probably be a runtime thing instead of compile-time.)

Comment 9

[Chris H-C :chutten]

With the flow stemmed by bug 1930770 we can start designing a more systematic solution to "Only Firefox Desktop speaks OHTTP, but parts of gecko might submit pings that require it in non-Firefox Desktop contexts".

Ultimately this comes down to an architectural issue. Only Firefox Desktop's integration of Glean (FOG) knows about use_ohttp: true, but it is the one piece of code that definitely is not able to enforce anything when OHTTP is not present. We need to teach some other piece of the puzzle that OHTTP is a thing that it needs to consider, even if it isn't involved in its support. And the only piece present in all the cases is the Glean SDK itself. So this is now a Glean SDK bug.

(( Also, things might be even wrinklier as :tjr mentions in the patch for bug 1930770 that there might be valid use cases for use_ohttp: true to mean "prefer it if it's there, but if it isn't we'll manage through informed opt-in". This appears to be the edgiest of edge cases, though, so I hope it's okay if ends up easier to ignore it. ))

A straw design would be to add a required parameter to Ping's ctor to hold the uses_ohttp state. Then, if Glean is not configured to support OHTTP (at init via Configuration), then on submit we log a message and clear any stored data for that ping. This would require changes to glean_parser codegen, the Glean SDK to support this new logic, and to FOG to properly advertise its OHTTP support. For extra points the SDK can communicate that OHTTP is required all the way up to the uploader so that FOG doesn't need to match by ping name.

Popping this into the Glean SDK component places it in line for Monday's triage.

Comment 10

[Travis Long [:travis_]]

With the immediate OHTTP pings dealt with, we will put this down in priority until likely the first of the year when we have time to get back to hardening Glean to better prevent OHTTP pings from being sent on Android (or implementing OHTTP for android..)