Closed Bug 176950 Opened 20 years ago Closed 20 years ago
cookie permissions site blocking does not appy to fully qualified node names from within the site
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0 blocking or allowing an entire site does not block/allow specific nodes within that site. If I specify that cookies/images from site foo.com are allowable, cookies/images from fully qaulified names such as nodeA.foo.com are not covered by the more general permissions entry. Furthermore, URI's with port designations are always treated as unique, distinct entities. Thus blocking nodeA.foo.com will NOT block nodeA.foo.com:8080. A review of the code for nsPopupWindowManager implies that site blocking applies to node specific cookies. This is what I expected. The lack of this feature for cookies seriously degrades the utility of cookie blocking. Reproducible: Always Steps to Reproduce: THis is troublesome, I can reliably reproduce this in our site where we have a library proxy server that sets a site cookie of lib.ncsu.edu,followd by a more specifc cookies for www.lib.ncsu.edu, and finally a proxied service cookie of www.lib.ncsu.edu:2257. Obviously, you folk do not have access to this environment. I did some random poking and although I encountered several situations similar to the above, I'm darned if I can recall exactly which other sites showed similar behavior... Here's a short approach that might work: 1. pick any site that uses fully qualified node names for its cookies 2. note the node name 3. exit all ocurrances of mozilla 4. manually edit the cookperm.txt (or equivelant) and specify a site name entry for the node 5. start mozilla 6. revisit the site Actual Results: blocking/allowing does not occurr Expected Results: blocked the fully qualified cookie based onthe site name blocking I reviewed the code from your website for (seamonkey). The following suggestions can remedy the situation: In module http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsPermissions.cpp, method Permission_Check, at line 257, the method permission_CheckFromList is called rather than the method PERMISSION_TestForBlocking. The former does an exact string comparison on the host name while the latter checks the full host name followed by checks on the more generalized portions of the domain name. The fix requires that line 257 be rewritten to invoke PERMISSION_TestForBlocking and that the implementation of PERMISSION_TestForBlocking be revised to return the permission object like PERMISSION_TestForBlocking does. This change to the return value should not disrupt anything - nsPopupWindowManager::TestPermission appears to be the only caller of the method and it does not use the reutrn value. Additionally, the method PERMISSION_TestForBlocking ought to be modified to first call permission_CheckFromList with on URI's that include port designation, second stip off any port designation and proceede as normal. There is a comment in module http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsPopupWindowManager.cpp#195 (nsPopupWindowManager::TestPermission) at line 185 regarding this "quirky" behavior of the existing algorithm.
*** Bug 178158 has been marked as a duplicate of this bug. ***
-> darin (code for this should probably leverage networking's "no proxy for" code)
Assignee: morse → darin
Status: UNCONFIRMED → NEW
Ever confirmed: true
Permissions should probably be using a hierarchy approach for checking a site permission - begin from the most specific entry that domain-matches the host, and work backward from there. This will have a perf cost though (especially with big permission lists) so it'll have to wait until we move to hashes in the perm backend. reassigning to mvl, please implement this feature in the ->hashtable patch ;)
Assignee: darin → mvl
Domain-walking is currently only for popups, but we really should enable this for cookies and images too. I've discussed this with jag & darin and the concensus seems to be that it should be a fairly simple change, but we'd like to think about it further and collect comments before we make the change. After the permission work, this is now very easy to patch. Before we do, we need to think what that will mean to users. When you are now blocking images from domain.com, you will suddenly not see images from img.domain.com anymore. It'd be nice to get this decided fairly quickly - say, in a week or so - but if this needs more thought, that's fine. Comments?
It seems to me that using a hierarchy approach that starts with the most specific match and then proceedes to a more general match is a fair solution. This is especially true if blocking an entire domain still allows one to explicitly grant permissions to a specific node within the domain. Furthermore, granting permission to a domain should also grant permission to all members of the domain. The interesting quesiton arises when port specifications are used. It seems to me (though I am not 100% certain) that interpreting the lack of a port specification as meaining all ports may create a situation where in a sneaky programmer might attempt to piggy back on a domain level access permission by using alternate port numbers. My understanding is that the lack of a port specification implies a protocol specific default (80 in the case of http). A possible approach for dealing with this would be to have the [knowlegable] user explicitly specify a wild card port interpretation. Thus, a specification such as FOO.COM would apply to the protocol specific default port whilst foo.com:* would apply to all ports in the domain. This would cover my original situation where in I was dealing with an academic libary proxy server that kept serving up different port numbers and machines all within the same domain.
WRT ports: we should store the cookie permissions with a port number (or the * wildcard if it's practical to support it). That way you won't have to match multiple hash entries during implementation. Personally I think the * port-wildcard would cause performance problems and is not worth it. mvl: you can store the port number as a PRUint16 in the hash-entryclass, instead of as part of the text string. It would save a byte, and make things easier in general, I think. Let me know if you need help writing a hash-generator function.
Severity: major → normal
OS: Windows 2000 → All
Hardware: PC → All
I've thought a bit about the port stuff... currently it's not really implemented properly, and we should address it. I think it's of lower priority than getting domainwalking done, and the upcoming hashtable conversion for permissions will affect the port-related code significantly, so we should leave the port stuff til later. I agree with doug's concept, but I don't like the idea of tacking a default port onto e.g. an http:// URI when we store a permission. If a user wants to block a site, we should default to blocking all ports from that URI. If they are knowledgable enough to know they only want to block one port, then they can edit the permission entry themselves to do this. so we could do something like: 1) given a URI e.g. "http://foo.com/", first check the permission list for the host foo.com on the default http port, which would be "foo.com:80". 2) if no entry was found, then check for foo.com with no port, which would be "foo.com". the code to do this is very simple; the only issue that bsmedberg pointed out was that we now have to do twice the number of hash lookups, but it's the most flexible and powerful option. if we decide we're okay with assuming a specific port for everything (i.e. blocking "http://foo.com" will only block port 80, which would make doug's library example impossible to solve), then we can assume default ports for unspecified entries, and eliminate step 2. we could also drop ports altogether, and keep things simple. either of these three options sounds valid to me.
here's a 1-line patch to enable domainwalking for all permission types. requesting r/sr - darin, jag, can you think of any objections to this? if we are going to do it, better sooner than later so we can get as much user testing time on-trunk and in beta as possible.
Comment on attachment 119412 [details] [diff] [review] domainwalking patch yup, i agree. let's give this a try. r=darin
Attachment #119412 - Flags: review?(darin) → review+
Comment on attachment 119412 [details] [diff] [review] domainwalking patch sr=jag
Attachment #119412 - Flags: superreview?(jaggernaut) → superreview+
Checking in extensions/cookie/nsPermissionManager.cpp; /cvsroot/mozilla/extensions/cookie/nsPermissionManager.cpp,v <-- nsPermissionManager.cpp new revision: 1.22; previous revision: 1.21 done
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 80712 has been marked as a duplicate of this bug. ***
How does this work? If I right click on an image and select Block Images From This Server and the image is on server "a.foobar.tld" it is added as such to the list of servers from which images can't load and not as "foobar.tld". So if I get it correctly it now blocks al images from "x.a.foobar.tld" instead of "x.foobar.tld"? Or is there some other function which allows you to add a domain manualy to the list? Maybe there should be two options: Block from this server and Block from this domain.
>How does this work? If I right click on an image and select Block Images From >This Server and the image is on server "a.foobar.tld" it is added as such to >the list of servers from which images can't load and not as "foobar.tld". that's correct - the host that the image resides on is the one that gets added to the list. >So if I get it correctly it now blocks al images from "x.a.foobar.tld" instead >of "x.foobar.tld"? right. all images from *.a.foobar.tld will be blocked; foobar.tld is not included in this. >Or is there some other function which allows you to add a domain manualy to the >list? Maybe there should be two options: Block from this server and Block from >this domain. it's difficult to have an automatic "block from this domain" kind of option, because it's not trivial to determine what that domain should be; further, it might not be intuitive to all users, and unneccessarily complicates things. one idea which i'm semi-fond of is to allow the user to specify what server to block from manually (a dialog), but i'm not sure how to integrate that nicely into the UI yet. it's a future enhancement we might consider.
*** Bug 158299 has been marked as a duplicate of this bug. ***
Dan Witte wrote, "it's difficult to have an automatic "block from this domain" kind of option" For anyone interested in discussing this feature, head over to Mozillazine: http://www.mozillazine.org/forums/viewtopic.php?t=8604
oh yeah, one thing to mention while I'm here. I don't test Image Blocking. If you want to talk about something that is Image Blocking that relates to a cookie feature, please create a separate bug and mark it depends. I am getting too much bugmail to worry about non-cookies stuff. As for the blocking aspects, I don't think doing port wildcards makes sense. It does logically, if you think of the posible socket tuples as a two dimensional space of IP addresses to port numbers (because blocking by network is blocking a row, while blocking sockets would be the analgous blocking of a column. But domains are a hierarchical overlay that is one-many and non-contigous by nature (that is the whole point of having a level of indirection), and I don't think that there is any end-user logic to blocking a port across a non-contiguous block of sockets in the numberspace. The relevant type of indirection to DNS is protocol scheme. A user might say: I hate FTP from this domain. A user might say, this socket (domain -> IP address + port) annoys me. nobody says: I dislike all image servers running on port 81 from the adserver.com, which might be 20 ip addresses living on every major ISP on the planet. That is too easy to circumvent anyhow w/ dynamic link generation. AOL's IM server accepts connectionns from every port, to allow maximum blocking evasion. real audio is similar, it comes with what looks like a built in port scanner.
QA Contact: tever → cookieqa
You need to log in before you can comment on or make changes to this bug.