Closed Bug 749325 Opened 12 years ago Closed 11 years ago

Security Review: WebNFC


( :: Security Assurance: Review Request, task, P2)


(Not tracked)



(Reporter: curtisk, Assigned: pauljt)



(Whiteboard: [secreview waiting][score:36::Medium ][FxOS])

1. Who is/are the point of contact(s) for this review?
2. Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):
3. Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:
4. Does this request block another bug? If so, please indicate the bug number
5. This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?
6. Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
6a. Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?
6b. Are there any portions of the project that interact with 3rd party services?
6c. Will your application/service collect user data? If so, please describe 
7. If you feel something is missing here or you would like to provide other kind of feedback, feel free to do so here (no limits on size):
8. Desired Date of review (if known from and whom to invite.
I'm the lead point of contact inside mozilla for this review, though
(continuing last comment that got submitted too soon)

1. I'm the lead point of contact inside mozilla for this review, though it's being implemented by Markus and Arno from Deustch Telekom. I've added Markus to the CC on this, and I'll work with him on the sec review schedule.
2. This currently is scoped for V1 of B2G, which just involves tag reading. We may want to break this into multiple reviews since reading and writing will cover vastly different areas.
4. 674741
5. Most likely before July 1, which is the beta date for B2G. However, as it's not a blocker for V1 and may be scoped out of the product depending on need, it may be able to be pushed back farther.
6a. This will add a DOM interface for NFC interaction, so it will affect Firefox, though mostly thru usage via B2G.
6b. Apps may be written to access NFC data.
6c. Hard to say? Depends on the context of the tags being read. Writing, on the other hand, will distribute user data (hence the thought of maybe dividing these reviews)
7. None thus far, though we're still in the very early code review phases, and this is a First Patch review from the submitter. More things may come out as that happens.
8. Definitely sometime after May 22, but no firm date yet.
Whiteboard: [pending secreview][needs info] → [pending secreview][triage needed 2012.05.02]
Assignee: nobody → ptheriault
Whiteboard: [pending secreview][triage needed 2012.05.02] → [pending secreview][start mm/dd/yyyy][target mm/dd/yyyy]
Putting this as a lower priority review since it isnt a blocker for k90.
Priority: -- → P3
Risk/Priority Ranking Exercise

Priority: 4 (P2) - Mozilla Initiative

Operational: 0 - N/A
User: 5 - Blocker
Privacy: 5 - Blocker
Engineering: 3 - Major
Reputational: 5 - Blocker

Priority Score: 72
Severity: normal → blocker
Priority: P3 → P2
Whiteboard: [pending secreview][start mm/dd/yyyy][target mm/dd/yyyy] → [pending secreview][start mm/dd/yyyy][target mm/dd/yyyy][score:72::High]
Curtis, this isn't basecamp so wont be looking at it until other basecamp stuff is done. Not really sure what P2 Blocker means, but just FYI.
Priority: P2 → P4
I dont know what 'blocker' means, but this is currently vaporware, so there is nothing to review at the moment.
Severity: blocker → normal
Vaporware? D: 

It's in active development, just not internally, and we aren't concerned with it on B2G until after V1. We've had patches around since April though, see bug 674741. That said, I'm the lead reviewer on it, and won't be touching it for at least another few weeks, so we've got time.
Sorry ;) I did actually know that, I er mis-spoke. Didnt mean anything, just that it isnt a secreview priority.
Whiteboard: [pending secreview][start mm/dd/yyyy][target mm/dd/yyyy][score:72::High] → [pending secreview][start mm/dd/yyyy][target mm/dd/yyyy][score:36::Medium ]
so the priority levels (minor, normal, major, critical, blocker) are a way of expressing the severity levels for the given areas (like comment 4). Whatever area has the highest level is the one the bug is marked with.
Waiting on implementation (post-basecamp)
Whiteboard: [pending secreview][start mm/dd/yyyy][target mm/dd/yyyy][score:36::Medium ] → [secreview waiting][score:36::Medium ]
looks like they are ready for us to do a review pass, they pinged me in the main bug and I set the flags to you
Whiteboard: [secreview waiting][score:36::Medium ] → [secreview waiting][score:36::Medium ][FxOS]
Some parts are in being refactored but we're starting to look at getting NFC pieces landed. Anything that lands from Bug 674741 should be considered shipping code for FxOS. Should we loop in someone from the security team on these reviews?
Yes, pauljt. I would like to fuzz the implementation, so we should also arrange some day for talking/chatting about what has priority and where easy attack vectors are.

Adding main devs for WebNFC to the CC list on this bug, and I'll introduce you via email.
Sounds good cdiehl, if I could be involved in discussions too that would be great. I dont think I am going to be particularly useful going through the finer detail of the code, but I would like to understand the how the system works as a whole, and the gecko interfaces (webAPIs). What is the target release qdot, and what phone would cdiehl need to perform testing?
Flags: needinfo?(kyle)
We're working on both of those questions right now and I can't even give you a good timeline on when we'll know that yet. I'll update this bug once I find everything out.
Flags: needinfo?(kyle)
Blocks: b2g-nfc
No longer blocks: webnfc
Status update for any who are interested - WebAPI is getting finalised, and there is a lot of code ready for secreview.
Priority: P4 → P2
Design review complete:

Final implementation review will be needed once API is more final (and dependent bugs are resolved).
Closed: 11 years ago
Depends on: 913336, 913340
Resolution: --- → FIXED
Removing secure element support since that wasnt part of this review (future feature)
No longer blocks: 879861
You need to log in before you can comment on or make changes to this bug.