1441989 - Remove dead code from ContentVerifier.cpp

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P1)

Description

[Ursula Sarracini (:ursula)]

Remote newtab was using ContentVerifier.cpp since it was loading a remote resource, and that project has long been dead. The removal of the code has started in bug 1355166 for the files in browser/components/newtab, but there is leftover content verifying stuff that we no longer need. To quote Franziskus on bug 1355166:

The only two services (I know of) using content signatures right now are blocklist updates and normandy. They both use the CS service directly. So I think ContentVerifier.cpp can go completely. To make that work there are changes to nsHTTPChannel necessary.
However, before doing that it might be advisable to check whether this is used anywhere else. I don't think it is and a quick check doesn't reveal any callers of LoadInfo::SetVerifySignedContent (and without it content signature verification is not invoked from nsHTTPChannel). The LoadInfo bits can be removed as well then as it can't really be used anymore.
This is touching a couple different parts but I think it's all dead code right now and should probably be removed.

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Adam, are you still working on this? (fwiw, from what I can tell nothing is using ContentVerifier).

Comment 2

[Adam Kasztenny]

Oops, no I'm not. I haven't had time recently... or for the past year. Can we just delete ContentVerifier altogether?

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

My view is that a) ContentVerifier doesn't have tests b) it's dead code anyway and c) its presence is complicating efforts to improve ContentSignatureVerifier (see e.g. bug 1534600). Removing it isn't quite as simple as hg rm, but it also wouldn't be that difficult. If you're not available to do this, I'll probably pick it up in the next few weeks or so.

Comment 4

[Adam Kasztenny]

Makes sense. I probably won't have spare time for a few weeks.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: bug 1441989 - remove ContentVerifier r?baku,mayhemer

ContentVerifier has been dead code since bug 1355166 (which, incidentally, means
it has no tests). Its presence is preventing improvements to
ContentSignatureVerifier (see e.g. bug 1534600), so this patch removes it.
As a result, the nsILoadInfo attributes verifySignedContent and enforceSRI are
also unused, so this patch removes those as well.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8292a59d1957
remove ContentVerifier r=baku,mayhemer

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/8292a59d1957