Re: lynx-dev Accepting invalid cookies - continuing discussion

[brian j. pardy]

On Prickle-Prickle, the 4th of Chaos, 3165, Klaus Weide wrote:
> On Tue, 29 Dec 1998, brian j. pardy wrote:
> 
> > Okay.  I'll work for now on getting a proper flag for domains that are
> > specified, and then we can decide on just what to do with it and how.
> 
> So here some thoughts on the other end of the problem, that is the
> "what to do with it" end.

Okay, I put this aside a few days, networked Quake is an evil evil evil
thing that can suck away hacking time...

> Currently store_cookie has this structure:
> 
> {
>     if (co->version != 0 || !LYAcceptAllCookies) {
>       if (FAILS_PATH_CHECK)   /* path must match */
>           REJECT;
>       if (strcmp(co->domain, hostname) != 0) { /* skip ckecks if equal */
>           if (FAILS_HOSTDOT_CHECK) { /* host doesn't have dot */
>               REJECT;
>           }
>           if (FAILS_COND2_CHECK) { /* domain not .something, or top-level*/
>               REJECT;
>           }
>           if (FAILS_COND3_CHECK) { /* host doesn't "domain-match" domain */
>               REJECT;
>           }
>           if (FAILS_COND4_CHECK) { /* host more than one level below domain*/
>               if (!LYAcceptAllCookies)
>                   if (!HTConfirm(INVALID_COOKIE_DOMAIN_CONFIRMATION))
>                       REJECT;
>           }
>       }
>     }
>     /* delete old cookie if it existed */
>     /* various other checks - expired, capacity, length */
>     if (ACCEPT_BASED_ON_DOMAIN_OR_FINAL_PROMPT)
>       STORE;
> 
> We want to replace the dependence on the global LYAcceptAllCookies by
> dependence on some per-domain or per-hostname flag(s), and also change
> how the various checks are bypassed (or not) depending on the new flag(s).

Right.

Personally, I'm not one of the people that is wanting/needing the special
behavior for invalid domains, so more input than mine is definitely 
necessary.  I don't see any problems in your logic.

> The most detailed approach would be to have three possible behaviors for
> _each_ of the checks, separately:
>       CONTINUE_ANYWAY, REJECT_ALWAYS, QUERY_USER   
> similar to the three possibilities for the final decision whether to store
> the cookie
> Basically, each of the
>           if (FAILS_XXX)
>               REJECT;
>           }
> above would be replaced by
>             if (FAILS_XXX) {
>               if (bv_XXX == REJECT_ALWAYS)
>                   REJECT;
>               else if (bv_XXX == QUERY_USER && !HTConfirm(INVALID_XXX))
>                   REJECT;
>               /* else go on to next check */
>           }
> but this would be most certainly overkill.

Agree that that's overkill.

> The least detailed approach would be to have just one boolean flag
> (but still per-domain or per-hostname), to be used in place of 
> LYAcceptAllCookies here.  (And modify to which of the checks it
> applies.)  That seems to be where you are heading.  Let's pretend
> it is a variable INVALID_DOMAIN_ALLOWED (ignoring the way you
> habe implemented as a bitflag).
> 
> We can describe the current behavior (2.8.2dev) in terms of the
> most-detailed flags: (ignoring effects if LYAcceptAllCookies in
> other places; and hoping my notation makes sense to anybody)
>                  LYAcceptAllCookies==0   LYAcceptAllCookies==1
> bv_PATH_CHECK       REJECT_ALWAYS        CONTINUE_ANYWAY
> bv_HOSTDOT_CHECK    REJECT_ALWAYS        CONTINUE_ANYWAY
> bv_COND_2_CHECK     REJECT_ALWAYS        CONTINUE_ANYWAY
> bv_COND_3_CHECK     REJECT_ALWAYS        CONTINUE_ANYWAY
> bv_COND_4_CHECK     QUERY_USER           CONTINUE_ANYWAY

I'd definitely consider that overloaded, then.

> We also can define the two possible settings of the more desirable
> least-detailed (but still host- or domain-dependent) approach
> in terms of the most-detailed flags.  Here are my choices:
>                  INVALID_DOMAIN_ALLOWED==0   INVALID_DOMAIN_ALLOWED==1
> bv_PATH_CHECK       REJECT_ALWAYS        CONTINUE_ANYWAY
> bv_HOSTDOT_CHECK    REJECT_ALWAYS        REJECT_ALWAYS
> bv_COND_2_CHECK     REJECT_ALWAYS        REJECT_ALWAYS
> bv_COND_3_CHECK     REJECT_ALWAYS        REJECT_ALWAYS
> bv_COND_4_CHECK     QUERY_USER           CONTINUE_ANYWAY
> 
> The reasoning: INVALID_DOMAIN_ALLOWED==0 columns stays like current
> default for compatibility.  As for the various checks,
> bv_PATH_CHECK
>   This one triggered the last round of loosening - at least one person
>   had a need for it.  NS spec does not require rejecting.  Bad spec,
>   does not protect separate /~users on same host from each others' cookies.
>   (The name INVALID_DOMAIN_ALLOWED doesn't fit if flag covers this too.)

Have people actually seen cookies sent by users?  I never have, but I
assume they exist.  Does the new(er?) spec require cookies that fail the
path check to be rejected?  

> bv_HOSTDOT_CHECK
>   Checks whether hostname is FQDN or IP (has dots).  Only relevant
>   for local hosts that are resolvable with default search.  We should
>   eventually implement RFC2109-successor's "effective host" stuff
>   (appending .local to unqualified hosts).
> bv_COND2_CHECK
>   Prevents cookies for top-level domains like .com, NS spec has it.
>   NS spec actually has more restrictive version for country-TLDs, (the
>   non-implementation of this further restriction is the topic of the
>   "cookiemonster" "exploit"), we don't.  We may eventually want to
>   provide option to follow NS spec, not be more liberal.
> bv_COND3_CHECK
>   This is the host_matches check.  NS spec has it.  Without this cookies
>   are completely promiscuous across domains.  Not desirable to loosen this.
>   Also current implementation of next_check requires this to have passed.

Agreed that none of these three should fall under accept_all_cookies'
jurisdiction.

> bv_COND4_CHECK
>   The "only one level below" rule.  foo.blah.x.y.z can set cookie for 
>   domain .blah.x.y.z but not for .x.y.z . This is the rule that makes
>   lynx look good on the cookiemonster page, even though we don't have
>   the NS country-TLD restriction.
>   NS spec doesn't have this one.  It was the first one to be made into
>   a prompt, and later to be bypassed.  So there seems to be a real need
>   for the option to bypass this.

I agree that at *least* the original behavior of prompting should be
retained.

I'm not sure, however, that COND4_CHECK and PATH_CHECK should be
encompassed in the same variable, even though I'm slightly hesitant to
have another one added.  If I simply want edit.my.yahoo.com to be able to
set an invalid cookie, I don't always want this to mean /~user1 can set a
cookie for /~user2 as well.

> (Disclaimer: all comparisons to NS spec are approximate, void where
> prohibited.)

Too true. :)

> But instead of only one flage (per-host or per-domain), it may be
> better to have something in betweeen that and the most detailed approach.
> I am thinking of a variable with three possible settings:
> INVALID_COOKIE_CHECKING with values STRICT, LOOSE, ASK:
> 
>                    STRICT          LOOSE              ASK
> bv_PATH_CHECK     REJECT_ALWAYS   CONTINUE_ANYWAY    QUERY_USER
> bv_HOSTDOT_CHECK  REJECT_ALWAYS   REJECT_ALWAYS      REJECT_ALWAYS
> bv_COND_2_CHECK   REJECT_ALWAYS   REJECT_ALWAYS      REJECT_ALWAYS
> bv_COND_3_CHECK   REJECT_ALWAYS   REJECT_ALWAYS      REJECT_ALWAYS
> bv_COND_4_CHECK   REJECT_ALWAYS   CONTINUE_ANYWAY    QUERY_USER
> 
> Probably ASK should be the default; one could also add a 4th column
> for the current behavior (see above) - probably not worth it
> (or even add a CUSTOM columns, for which checks could be controlled
> individually).

Customization seems like overkill -- IMO if someone wants to be that
liberal with cookies for some reason they're probably going to be hacking
on the source anyways.  Maybe.  Or I'm just being lazy. :)

I do like the triple option idea.

> With those three possible settings, the invalid-cookie prompts could
> then be made to act like the accept-cookie prompt currently: there
> would be a four-way choice, the behavior could be set to STRICT or
> LOOSE at the prompt.  Also this could be modified from the Cookie Jar
> Page interface.  The way to permanently configure this could also be
> analogous to the current COOKIE_ACCEPT_DOMAINS and COOKIE_REJECT_DOMAINS
> lists: there would be two new list, INVALID_COOKIE_ACCEPT_DOMAINS and
> INVALID_COOKIE_REJECT_DOMAINS (or some better names).
> 
> Does it make sence to have this level of control?  I think it does,
> although few users would use it (or understand it - but that becomes
> a question of explaining it properly, and using names for the options
> describe the behavior well).

Erring on the side of more customization can rarely be a bad thing, IMO,
and it's apparent that at least two people on this list would use it.

> One could argue that the whether-to-accept-(valid)cookies behavior
> (for a given domain) should not be an independent property from the
> whether-to-allow-invalid-cookies behavior; shouldn't REJECT_ALWAYS
> (even for valid cookies) mean we always want STRICT for the invalid-
> cookie-checking? - probably for most users, but it would still be
> interesting to have a way to auto-REJECT normal cookies but also be
> alerted if an invalid cookie gets sent.

This would be nice, I agree.  

> And if I accept a cookie
> at the invalid-cookie prompt, doesn't that mean that I really want
> to accept it (so there shouldn't be another "normal" accept-cookie?
> prompt for this one)?  Maybe.  But this is more a question of the
> user interface (don't show redundant prompts), it dosen't need to
> be reflected in the data structures.  Also it isn't completely redundant,
> note there is some processing going on between the end of the checks
> in store_cookie and the final prompt - specifically, if a cookie passes
> the first hurdle it can delete an older cookie (same name/path/domain)
> even if it finally gets rejected. 
> 
> So, after thinking about this more, I think that making all the per-domain
> settings into one bitflag, as you did, is not a good idea.  It looked
> natural to me at first, too, but now I can't remember why...

Given your comments, I agree on this.  It made sense earlier...

> It doesn't make sense to have independent flags for QUERY_USER, 
> REJECT_ALWAYS, QUERY_USER which may contradict each other (and FROM_FILE
> isn't used any more).  But it does make sence to have invalidity-check
> settings independent of that set.  IOW,
>   struct _domain_entry {
>     char *            domain;
>     behaviour         bv;          /* normal, final accept/reject */
>     invcheck_behaviour        invcheck_bv; /* for invalidity checks */
>     HTList *          cookie_list;
>   };
> 
> with check_behaviour either a BOOLEAN; or an enum with three values (as
> I prefer); or an enum with more values (if we add evenmore detailed control).
> Or, in the most-detailed scenario
>     ...
>     behaviour         bv;
>     ...
>     invcheck_behaviour        cond2_bv;
>     invcheck_behaviour        cond3_bv;
>     invcheck_behaviour        cond4_bv;
>     ...
> 
> What do you think?

I think some more thought needs to come in from the people who will
actually use this (I only accept cookies from about 3 specific sites 
anyways), but I pretty much agree with your triple-option approach with
the possibility/probability of making them permanently configurable
(perhaps also with a global flag), with the possible caveat of splitting
it up into separate variables for PATH_CHECK and COND4_CHECK.

Sounds like a lot of work.

-- 
Warp 7 -- It's a law we can live with.