[vlc-devel] [PATCH 2/5] access/http: Implement the cookie domain matching algorithm from RFC 6265
Rémi Denis-Courmont
remi at remlab.net
Tue Jul 22 12:34:25 CEST 2014
Le 2014-07-22 13:09, Antti Ajanki a écrit :
> ---
> modules/access/http.c | 94
> ++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 77 insertions(+), 17 deletions(-)
>
> diff --git a/modules/access/http.c b/modules/access/http.c
> index 75ed1cf..2ef6f59 100644
> --- a/modules/access/http.c
> +++ b/modules/access/http.c
> @@ -193,6 +193,7 @@ typedef struct http_cookie_t
> char *psz_name;
> char *psz_value;
> char *psz_domain;
> + bool b_host_only;
> } http_cookie_t;
>
> /* */
> @@ -211,11 +212,14 @@ static int Request( access_t *p_access,
> uint64_t i_tell );
> static void Disconnect( access_t * );
>
> /* Small Cookie utilities. Cookies support is partial. */
> -static http_cookie_t * cookie_parse( const char * cookie_header );
> +static http_cookie_t * cookie_parse( const char * cookie_header,
> const char *request_host );
> static void cookie_destroy( http_cookie_t * cookie );
> static char * cookie_get_content( const char * cookie );
> static char * cookie_get_domain( const char * cookie );
> -static void cookie_append( vlc_array_t * cookies, const char *
> cookie );
> +static void cookie_append( vlc_array_t * cookies, http_cookie_t *
> cookie );
> +static bool cookie_is_valid( const http_cookie_t * cookie, const
> char *host );
> +static bool cookie_domain_matches( const http_cookie_t * cookie,
> const char *host );
> +static bool cookie_domain_is_public_suffix( const char *domain );
>
>
> static void AuthReply( access_t *p_acces, const char *psz_prefix,
> @@ -1201,8 +1205,7 @@ static int Request( access_t *p_access,
> uint64_t i_tell )
> {
> const http_cookie_t * cookie = vlc_array_item_at_index(
> p_sys->cookies, i );
>
> - /* FIXME: This is clearly not conforming to the rfc */
> - bool is_in_right_domain = (!cookie->psz_domain ||
> strstr( p_sys->url.psz_host, cookie->psz_domain ));
> + bool is_in_right_domain = cookie_domain_matches( cookie,
> p_sys->url.psz_host );
>
> if( is_in_right_domain )
> {
> @@ -1506,8 +1509,17 @@ static int Request( access_t *p_access,
> uint64_t i_tell )
> {
> if( p_sys->cookies )
> {
> - msg_Dbg( p_access, "Accepting Cookie: %s", p );
> - cookie_append( p_sys->cookies, p );
> + http_cookie_t *cookie = cookie_parse( p,
> p_sys->url.psz_host );
> + if ( cookie_is_valid( cookie, p_sys->url.psz_host )
> )
> + {
> + msg_Dbg( p_access, "Accepting Cookie: %s", p );
> + cookie_append( p_sys->cookies, cookie );
> + }
> + else
> + {
> + msg_Dbg( p_access, "Ignoring invalid cookie:
> %s", p );
> + cookie_destroy(cookie);
> + }
> }
> else
> msg_Dbg( p_access, "We have a Cookie we won't
> remember: %s", p );
> @@ -1583,7 +1595,7 @@ static void Disconnect( access_t *p_access )
> * them) (FIXME: only support the "domain=" param)
>
>
> *****************************************************************************/
>
> -static http_cookie_t * cookie_parse( const char * cookie_header )
> +static http_cookie_t * cookie_parse( const char * cookie_header,
> const char * request_host )
> {
> char *content = cookie_get_content( cookie_header );
> if ( !content )
> @@ -1603,7 +1615,17 @@ static http_cookie_t * cookie_parse( const
> char * cookie_header )
> cookie->psz_name = strdup( content );
> cookie->psz_value = strdup( "" );
> }
> +
> cookie->psz_domain = cookie_get_domain( cookie_header );
> + if ( !cookie->psz_domain || strlen(cookie->psz_domain) == 0 )
> + {
> + free(cookie->psz_domain);
> + cookie->psz_domain = strdup( request_host );
> + cookie->b_host_only = true;
> + }
> + else
> + cookie->b_host_only = false;
> +
> return cookie;
> }
>
> @@ -1641,9 +1663,10 @@ static char * cookie_get_domain( const char *
> cookie )
> /* Look for a ';' */
> while( *str )
> {
> - if( !strncmp( str, domain, sizeof(domain) - 1 /* minus \0 */
> ) )
> + if( !strncasecmp( str, domain, sizeof(domain) - 1 /* minus
> \0 */ ) )
> {
> str += sizeof(domain) - 1 /* minus \0 */;
> + while ( *str && *str == '.' ) str++;
> char * ret = strdup( str );
> /* Now remove the next ';' if present */
> char * ret_iter = ret;
> @@ -1661,25 +1684,23 @@ static char * cookie_get_domain( const char *
> cookie )
> return NULL;
> }
>
> -/* Add a cookie in cookies, checking to see how it should be added
> */
> -static void cookie_append( vlc_array_t * cookies, const char *
> cookie_header )
> +/* Add a cookie in cookies */
> +static void cookie_append( vlc_array_t * cookies, http_cookie_t *
> cookie )
> {
> int i;
>
> - http_cookie_t *cookie = cookie_parse(cookie_header);
> - if( !cookie )
> - return;
> + assert( cookie->psz_name );
> + assert( cookie->psz_domain );
>
> for( i = 0; i < vlc_array_count( cookies ); i++ )
> {
> http_cookie_t *iter = vlc_array_item_at_index( cookies, i );
>
> assert( iter->psz_name );
> + assert( iter->psz_domain );
>
> - bool is_domain_matching = (
> - ( !cookie->psz_domain && !iter->psz_domain )
> ||
> - ( cookie->psz_domain && iter->psz_domain &&
> - !strcmp( cookie->psz_domain,
> iter->psz_domain ) ) );
> + bool is_domain_matching =
> + strcasecmp( cookie->psz_domain, iter->psz_domain ) == 0;
>
> if( is_domain_matching && !strcmp( cookie->psz_name,
> iter->psz_name ) )
> {
> @@ -1692,6 +1713,45 @@ static void cookie_append( vlc_array_t *
> cookies, const char * cookie_header )
> vlc_array_append( cookies, cookie );
> }
>
> +/* Check if a cookie from host should be added to the cookie jar */
> +static bool cookie_is_valid( const http_cookie_t * cookie, const
> char *host )
> +{
> + return cookie && cookie->psz_name && strlen(cookie->psz_name) >
> 0 &&
> + cookie->psz_domain &&
> + !cookie_domain_is_public_suffix(cookie->psz_domain) &&
> + cookie_domain_matches(cookie, host);
> +}
> +
> +static bool cookie_domain_matches( const http_cookie_t * cookie,
> const char *host )
> +{
> + assert( !cookie || cookie->psz_domain );
> +
> + if ( !cookie || !host )
> + return false;
> + if ( strcasecmp(cookie->psz_domain, host) == 0 )
> + return true;
strcasecmp() is locale-dependent. I am not sure we can assume it does
you want it to do. Perhaps we should add an ASCII-strcasecmp() helper as
this is a recurrent problem.
> + else if ( cookie->b_host_only )
> + return false;
> +
> + size_t host_len = strlen(host);
> + size_t cookie_domain_len = strlen(cookie->psz_domain);
> + int i = host_len - cookie_domain_len;
> + bool is_suffix = ( i > 0 ) &&
> + strcasecmp( &host[i], cookie->psz_domain ) == 0;
> + bool has_dot_before_suffix = host[i-1] == '.';
> + bool host_is_ip = strspn(host, "0123456789.") == host_len;
Does that not fail for IPv6?
> + return is_suffix && has_dot_before_suffix && !host_is_ip;
> +}
> +
> +static bool cookie_domain_is_public_suffix( const char *domain )
> +{
> + // FIXME: should check if domain is one of "public suffixes" at
> + // http://publicsuffix.org/. The purpose of this check is to
> + // prevent a host from setting a "too wide" cookie, for example
> + // "example.com" should not be able to set a cookie for "com".
> + // Approximate by preventing all single component domains.
> + return domain && !strchr(domain, '.');
> +}
>
>
>
> /*****************************************************************************
> * HTTP authentication
--
Rémi Denis-Courmont
More information about the vlc-devel
mailing list