Re: lynx-dev Accepting invalid cookies - continuing discussion

[Klaus Weide]

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.

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).

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.

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

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.)
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.
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.
(Disclaimer: all comparisons to NS spec are approximate, void where
prohibited.)

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).

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).

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.  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...
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?

  Klaus