If an extension gets approved on UMO and we need to "unapprove it", or if it starts making its way around the world and is malicious, we need a way to notify clients. A blacklist.rdf that lists GUID and date it was banned would be good. This should also tie into UMO to make sure that something that's been blacklisted can't get uploaded again.

mconnor and morgamic, not sure if this is the bug you'd like to do the AMO work in but I am going to assume that it is. The blacklist format along with details about the format can be found at. I have submitted the patch in bug 271166 and will need the url for the blocklist xml file when / if it is checked in. http://wiki.mozilla.org/Firefox:1.1_Extension_Manager_Blacklist

I've talked to Rob about database changes needed to support the relationships needed to generate the blacklist XML. We will be discussing the updated schema and will look at mapping out needed functionality during the quarterly meeting (03/27-03/31). This will indeed be the bug to track progress of added functionality for blacklist management, and the generation of the blacklist XML.

Just a note that Rob's got bug 271166 which is the client-side work wrapped up. So we can start testing the server side whenever we are ready.

In Sunbird (which is using the new Add-ons code from trunk) we now get this error: "Blocklist::_loadBlocklistFromFile: XML File does not exist" Putting in the extensions.blocklist.* prefs from Ff makes it go away. The question for Rob is whether that is the correct URL or not, and if anything needs to be set up on the server side for Sunbird. +// Blocklist preferences +pref("extensions.blocklist.enabled", true); +pref("extensions.blocklist.interval", 86400); +pref("extensions.blocklist.url", "https://addons.mozilla.org/blocklist/1/%APP_ID%/%APP_VERSION%/"); +pref("extensions.blocklist.detailsURL", "http://www.mozilla.com/blocklist/");

AMO bugspam. Correcting QA contacts on OLD bugs (mozilla.update@update.bugs) -> Correct QA contact (administration@add-ons.bugs) Filtermeplzkthx

I hope it will cover the case when extension would set "extensions.blocklist.enabled" to false. it should set off a warning of some sort on such action or should be blocked entirely. Blocking just by guid is inaffective. Any self respecting malicious hacker would prepare an xpi with different GUID for every unique website visitor. sorry if this was not the right place to say this concern.

(In reply to comment #7) See http://wiki.mozilla.org/Extension_Blocklisting for what blocklisting is and is not. Yes, more needs to be done to prevent malware installation whether that is as an extension or as part of the application chrome but blocklisting is not the answer.

The two attachments I created today explain the data set that would store the information needed to generate our XML file described by this wiki page: http://wiki.mozilla.org/Extension_Blocklisting:Code_Design As far as I could tell, I've covered the necessary bases (see notes in the ER .jpg). Do you guys see anything missing, or errors in the table syntax that need to be fixed? I'd appreciate it if you could take a look before I code around this.

Alright, so apparently we should use "blocklist" instead of "blacklist" since that's what is in the client's URI. I've done the leadwork on blocklist.php and bloclist.tpl, both of which would back up the server-side component. Waiting for an OK on the data set before I create the get functions. We're at a point where we just need the data so we can iterate and spit it out to the template. :)

Comment on attachment 231260 [details] Create tables SQL for blacklist tables. Mike, item_id really should be item_guid to avoid confusion between the item's id and the item's guid... but why are you keying off of the guid instead of the id? You could use a foreign key and then it would have a little referential integrity. Shouldn't matter much since I doubt the blocklist will get very large. note: the blocklist also supports just specifying an app version range and will use that for the current app but that doesn't mean the provider of the blocklist has to support it. The toolkit version range will probably do a better job of that anyways. r=me with the item_id name change to item_guid or some other representative name and details as to why the blitems id isn't used as the key or change to use the id.

The item_id was indeed supposed to point to the primary key from the blitems table, but I gave it the wrong type (the SQL conflicted w/ my diagrams -- doh!). So, I think the type was just wrong, which was the source of the confusion. Also, I could add a key constraint and switch to the innodb engine for these two tables so we couldn't have blapps rows without a corresponding blitem.id -- will take a look at that switch tomorrow.

Comment on attachment 231288 [details] SQL for blocklist tables, v.2 Ahhh... now that makes much more sense. Thanks!

Should have this staged by close of business today.

I wasted quite a bit of time trying to get the XML output to be perfect, but getting the output from db and throwing it into a template loop (simple logic only) given all of the possible NULL values turned into a major headache. This alternative may not work because it contains duplicated emItem id's. Question for you guys is how the client processes this RDF, and if the output below would be valid: ... <emItem id="item_1@domain.com"> <versionRange minVersion="1.0" maxVersion="2.0.*"> <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}"> <versionRange minVersion="1.5" maxVersion="1.5.*"/> </targetApplication> </versionRange> </emItem> <emItem id="item_1@domain.com"> <versionRange minVersion="1.0" maxVersion="2.0.*"> <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}"> <versionRange minVersion="1.7" maxVersion="1.7.*"/> </targetApplication> </versionRange> </emItem> ... If so, then we'd be alright -- if not, then I might need more time to figure out how to create a perfectly formed XML document (no duped id's) given all of the constraints.

Looking through more of the docs, I found: "At first glance it may seem more appropriate to use a plain text file and parse delimited values. The draw back of this method are that extension id's and application id's would have to be specified multiple times whereas with this format we are able to specify each extension id one time only and each application id for an extension id one time only." So, unless it's just "losing style points", simply iterating through won't cut it (correct me if I'm wrong). I will instead try generating blocklist objects in PHP for each XML node, then traverse lists of these objects in the template file -- which was beyond my original intended scope of work in PHP, but it simply wasn't possible to account for all of the null cases using just arrays/hashes.

(In reply to comment #13) >... > note: the blocklist also supports just specifying an app version range and will > use that for the current app but that doesn't mean the provider of the > blocklist has to support it. The toolkit version range will probably do a > better job of that anyways. This applies to all null values and it is up to the blocklist provider to choose what supported methods they want to implement from the spec. I haven't looked at the patch yet but there should only be one entry per extension. http://wiki.mozilla.org/Extension_Blocklisting:Code_Design After I wake up a bit more I'll take a look

(In reply to comment #19) > I haven't > looked at the patch yet but there should only be one entry per extension. > http://wiki.mozilla.org/Extension_Blocklisting:Code_Design > > After I wake up a bit more I'll take a look Rob, thanks -- that's what I needed to know. Will have to spend some more time figuring out how to pull the data. I'll get to the whiteboard and drum up a better approach. Moving ETA back to Friday -- forming the XML has turned out to be more complicated than originally estimated.

Comment on attachment 231728 [details] [diff] [review] PHP plus template file for delivering RDF. This won't cut it -- logic in .php and .tpl file needs to exist to create properly formed XML doc according to specs.

I would think that you could just order by itemGUID, appGUID and then use a couple of loops when generating the xml. When the itemGUID changes add a new emItem otherwise the data will be added as a child to that itemGUID. When appGUID is the same as the previous appGUID and the itemGUID hasn't changed the data will be added as a child to that appGUID.

I had most of that working, but the relationship that killed me last night was the null appGUID. I had gotten everything up to the app version ranges to work, but got stuck then went crazy. Who knows, might have just been too tired -- I think I'll have a better experience today.

re: the null appGUID... that is your call on whether to implement it or not. I implemented it on the client because it was simple to do but it is by no means necessary that you implement it on the blocklist provider side. Also see comment #19

This run-through was much easier. I was held up on some of the identifying characteristics per row. Using itemId properly as a unique key, it's possible to set up two hashes, then use simple iterations in the template file. So after some more tinkering I was able to get the output to look exactly like the wiki example (plus .com since item_1@domain isn't legal).

just a note... text@text is legit. It just has a pseudo tld look to it.

doh... thought it had to be a valid email. my bad.

Comment on attachment 231810 [details] [diff] [review] PHP plus template file, v.2 So, if we have an empty data set we want to return a valid blocklist to handle the case where one day we have one item in the blocklist and the next we have zero so the clients will remove that one item. Another option would be to always have at least one item per app in the blocklist which would facilitate testing... I'd prefer this. It looks like the spacing on the output doubles per line per node. reqVersion and appVersion shouldn't be needed and aren't used. Not sure what reqVersion is for and we don't want to just return a result set for the client's app version so if the app is upgraded the blocklist will still be valid which the code already does unless I am mistaken. I'll review this again after I fix bug 347140 in my tree :(

(In reply to comment #28) > (From update of attachment 231810 [details] [diff] [review] [edit]) > So, if we have an empty data set we want to return a valid blocklist to handle > the case where one day we have one item in the blocklist and the next we have > zero so the clients will remove that one item. Another option would be to > always have at least one item per app in the blocklist which would facilitate > testing... I'd prefer this. Okay -- when no results exist, it should be valid (<blocklist></blocklist>). Are you saying we need an <emItems></emItems> value even if there are no items? > It looks like the spacing on the output doubles per line per node. Yeah, the loops in the template messed up the whitespace -- I can work on that. > reqVersion and appVersion shouldn't be needed and aren't used. Not sure what > reqVersion is for and we don't want to just return a result set for the > client's app version so if the app is upgraded the blocklist will still be > valid which the code already does unless I am mistaken. The reqVersion was agreed upon as a way to at some later day change the URI schema, which would change behavior in the script -- mostly when setting up variables. I will add the switch statement in so adding in future reqVersions will be easier. > I'll review this again after I fix bug 347140 in my tree :( Thanks Rob -- I will work on the changes above.

It should have at minimum <?xml version="1.0"?> <blocklist xmlns="http://www.mozilla.org/2006/addons-blocklist"> </blocklist> Thanks... I forgot about the future use for those vars. btw: there appears to be an issue when using <versionRange minVersion="3.1" maxVersion="4.*"> </versionRange> instead of <versionRange minVersion="3.1" maxVersion="4.*"/> I'm working on making the code allow that. Thanks again.

What about: <emItem id="item_5@domain.com"> </emItem> vs: <emItem id="item_5@domain.com"/> That might require a special array of 'items that don't have any children' to be set in the PHP file. Not easy to do in the template.

(In reply to comment #31) It now handles these cases so it should be fine.

It's pretty messed up that you'd have to change the parser because of the template file -- so I spent some more time getting it right. I had to work a little around Smarty's retardedness, but I got it to work. I also was able to save some logic by short-circuting the $blocklist entries for itemGuid's that don't have any further data (no versionRanges or targetApplications). The template file can detect the absence of the array because it's null in these cases. Also added more comments to the PHP file to explain variables and the logic behind populating the $blocklist array, which eventually gets pushed to the template file.

Same as before, but removed an unnecessary loop in template file.

Comment on attachment 232085 [details] [diff] [review] PHP plus template file, v.4 Thank you very much for doing such an excellent job on this... I really appreciate it.

Same as v.2, except the blapps.guid column can be null.

All, thanks for helping review and for putting up w/ the bugspam. We're ready to push this live tomorrow (08/05, 8am-12pm) during the AMO migration to its new cluster. Will update and resolve this bug when it has been pushed.