From 395914fb4d439ce5220a44af231d3e16bf3fe18d Mon Sep 17 00:00:00 2001 From: Batuhan Taskaya Date: Fri, 4 Mar 2022 14:09:16 +0300 Subject: [PATCH] Apply suggestions from the review --- SECURITY.md | 10 ++++ docs/README.md | 17 ------ httpie/legacy/__init__.py | 0 httpie/legacy/cookie_format.py | 103 +++++++++++++++++++++++++++++++++ httpie/manager/cli.py | 6 +- httpie/manager/tasks.py | 24 +------- httpie/sessions.py | 98 ++++++++++--------------------- 7 files changed, 150 insertions(+), 108 deletions(-) create mode 100644 SECURITY.md create mode 100644 httpie/legacy/__init__.py create mode 100644 httpie/legacy/cookie_format.py diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..b10980cb --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,10 @@ +# Security Policy + +## Reporting a Vulnerability + +To report a vulnerability, please send an email to `security@httpie.io` describing the: + +- The description of the vulnerability itself +- A short reproducer to verify it (you can submit a small HTTP server, a shell script, a docker image etc.) +- The severity level classification (`LOW`/`MEDIUM`/`HIGH`/`CRITICAL`) +- If associated with any, the [CWE](https://cwe.mitre.org/) ID. diff --git a/docs/README.md b/docs/README.md index efd579a3..30daa4b6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2422,23 +2422,6 @@ And since there’s neither data nor `EOF`, it will get stuck. So unless you’r Also, it might be good to set a connection `--timeout` limit to prevent your program from hanging if the server never responds. -### Security - -#### Exposure of Cookies To The 3rd Party Hosts On Redirects - -*Vulnerability Type*: [CWE-200](https://cwe.mitre.org/data/definitions/200.html) -*Severity Level*: LOW -*Affected Versions*: `<3.1.0` - -The handling of [cookies](#cookies) was not compatible with the [RFC 6265](https://datatracker.ietf.org/doc/html/rfc6265) -on the point of handling the `Domain` attribute when they were saved into [session](#sessions) files. All cookies were shared -across all hosts during the runtime, including redirects to the 3rd party hosts. - -This vulnerability has been fixed in [3.1.0](https://github.com/httpie/httpie/releases/tag/3.1.0) and the -[`httpie cli sessions upgrade`](#upgrading-sessions)/[`httpie cli sessions upgrade-all`]((#upgrading-sessions) commands -have been put in place in order to allow a smooth transition to the new session layout from the existing [session](#sessions) -files. - ## Plugin manager HTTPie offers extensibility through a [plugin API](https://github.com/httpie/httpie/blob/master/httpie/plugins/base.py), diff --git a/httpie/legacy/__init__.py b/httpie/legacy/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/httpie/legacy/cookie_format.py b/httpie/legacy/cookie_format.py new file mode 100644 index 00000000..b5c6392b --- /dev/null +++ b/httpie/legacy/cookie_format.py @@ -0,0 +1,103 @@ +import argparse +from typing import Any, Type, List, Dict, TYPE_CHECKING + +if TYPE_CHECKING: + from httpie.sessions import Session + +INSECURE_COOKIE_JAR_WARNING = '''\ +Outdated layout detected for the current session. Please consider updating it, +in order to not get affected by potential security problems. + +For fixing the current session: + + With binding all cookies to the current host (secure): + $ httpie cli sessions upgrade --bind-cookies {hostname} {session_id} + + Without binding cookies (leaving them as is) (insecure): + $ httpie cli sessions upgrade {hostname} {session_id} +''' + + +INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS = '''\ + +For fixing all named sessions: + + With binding all cookies to the current host (secure): + $ httpie cli sessions upgrade-all --bind-cookies + + Without binding cookies (leaving them as is) (insecure): + $ httpie cli sessions upgrade-all +''' + +INSECURE_COOKIE_SECURITY_LINK = '\nSee https://pie.co/docs/security for more information.' + + +def pre_process(session: 'Session', cookies: Any) -> List[Dict[str, Any]]: + """Load the given cookies to the cookie jar while maintaining + support for the old cookie layout.""" + + is_old_style = isinstance(cookies, dict) + if is_old_style: + normalized_cookies = [ + { + 'name': key, + **value + } + for key, value in cookies.items() + ] + else: + normalized_cookies = cookies + + should_issue_warning = is_old_style and any( + cookie.get('domain', '') == '' + for cookie in normalized_cookies + ) + + if should_issue_warning and not session.refactor_mode: + warning = INSECURE_COOKIE_JAR_WARNING.format(hostname=session.bound_host, session_id=session.session_id) + if not session.is_anonymous: + warning += INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS + warning += INSECURE_COOKIE_SECURITY_LINK + + session.env.log_error( + warning, + level='warning' + ) + + return normalized_cookies + + +def post_process( + normalized_cookies: List[Dict[str, Any]], + *, + original_type: Type[Any] +) -> Any: + """Convert the cookies to their original format for + maximum compatibility.""" + + if issubclass(original_type, dict): + return { + cookie.pop('name'): cookie + for cookie in normalized_cookies + } + else: + return normalized_cookies + + +def fix_layout(session: 'Session', hostname: str, args: argparse.Namespace) -> None: + if not isinstance(session['cookies'], dict): + return None + + session['cookies'] = [ + { + 'name': key, + **value + } + for key, value in session['cookies'].items() + ] + for cookie in session.cookies: + if cookie.domain == '': + if args.bind_cookies: + cookie.domain = hostname + else: + cookie._rest['is_explicit_none'] = True diff --git a/httpie/manager/cli.py b/httpie/manager/cli.py index 9ad4eca6..1473ccf9 100644 --- a/httpie/manager/cli.py +++ b/httpie/manager/cli.py @@ -4,7 +4,7 @@ from httpie import __version__ CLI_SESSION_UPGRADE_FLAGS = [ { - 'variadic': ['--bind-cookies'], + 'flags': ['--bind-cookies'], 'action': 'store_true', 'default': False, 'help': 'Bind domainless cookies to the host that session belongs.' @@ -102,8 +102,8 @@ def generate_subparsers(root, parent_parser, definitions): for argument in properties: argument = argument.copy() - variadic = argument.pop('variadic', []) - command_parser.add_argument(*variadic, **argument) + flags = argument.pop('flags', []) + command_parser.add_argument(*flags, **argument) parser = HTTPieManagerArgumentParser( diff --git a/httpie/manager/tasks.py b/httpie/manager/tasks.py index c04ed9bc..297767b0 100644 --- a/httpie/manager/tasks.py +++ b/httpie/manager/tasks.py @@ -1,9 +1,10 @@ import argparse from typing import TypeVar, Callable, Tuple -from httpie.sessions import SESSIONS_DIR_NAME, Session, get_httpie_session +from httpie.sessions import SESSIONS_DIR_NAME, get_httpie_session from httpie.status import ExitStatus from httpie.context import Environment +from httpie.legacy import cookie_format as legacy_cookies from httpie.manager.cli import missing_subcommand, parser T = TypeVar('T') @@ -51,27 +52,8 @@ def is_version_greater(version_1: str, version_2: str) -> bool: return split_version(version_1) > split_version(version_2) -def fix_cookie_layout(session: Session, hostname: str, args: argparse.Namespace) -> None: - if not isinstance(session['cookies'], dict): - return None - - session['cookies'] = [ - { - 'name': key, - **value - } - for key, value in session['cookies'].items() - ] - for cookie in session.cookies: - if cookie.domain == '': - if args.bind_cookies: - cookie.domain = hostname - else: - cookie._rest['is_explicit_none'] = True - - FIXERS_TO_VERSIONS = { - '3.1.0': fix_cookie_layout + '3.1.0': legacy_cookies.fix_layout } diff --git a/httpie/sessions.py b/httpie/sessions.py index c23cb568..e4a20a53 100644 --- a/httpie/sessions.py +++ b/httpie/sessions.py @@ -8,7 +8,7 @@ import re from http.cookies import SimpleCookie from http.cookiejar import Cookie from pathlib import Path -from typing import Any, Dict, Optional, Union +from typing import Any, Dict, List, Optional, Union from requests.auth import AuthBase from requests.cookies import RequestsCookieJar, remove_cookie_by_name @@ -18,6 +18,7 @@ from .cli.dicts import HTTPHeadersDict from .config import BaseConfigDict, DEFAULT_CONFIG_DIR from .utils import url_as_host from .plugins.registry import plugin_manager +from .legacy import cookie_format as legacy_cookies SESSIONS_DIR_NAME = 'sessions' @@ -32,37 +33,25 @@ SESSION_IGNORED_HEADER_PREFIXES = ['Content-', 'If-'] KEPT_COOKIE_OPTIONS = ['name', 'expires', 'path', 'value', 'domain', 'secure'] DEFAULT_COOKIE_PATH = '/' -INSECURE_COOKIE_JAR_WARNING = '''\ -Outdated layout detected for the current session. Please consider updating it, -in order to not get affected by potential security problems. - -For fixing the current session: - - With binding all cookies to the current host (secure): - $ httpie cli sessions upgrade --bind-cookies {hostname} {session_id} - - Without binding cookies (leaving them as is) (insecure): - $ httpie cli sessions upgrade {hostname} {session_id} -''' - -INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS = '''\ - -For fixing all named sessions: - - With binding all cookies to the current host (secure): - $ httpie cli sessions upgrade-all --bind-cookies - - Without binding cookies (leaving them as is) (insecure): - $ httpie cli sessions upgrade-all - -See https://pie.co/docs/security for more information. -''' - def is_anonymous_session(session_name: str) -> bool: return os.path.sep in session_name +def session_hostname_to_dirname(hostname: str, session_name: str) -> str: + # host:port => host_port + hostname = hostname.replace(':', '_') + return os.path.join( + SESSIONS_DIR_NAME, + hostname, + f'{session_name}.json' + ) + + +def strip_port(hostname: str) -> str: + return hostname.split(':')[0] + + def materialize_cookie(cookie: Cookie) -> Dict[str, Any]: materialized_cookie = { option: getattr(cookie, option) @@ -92,22 +81,18 @@ def get_httpie_session( # HACK/FIXME: httpie-unixsocket's URLs have no hostname. bound_hostname = 'localhost' - # host:port => host_port - hostname = bound_hostname.replace(':', '_') if is_anonymous_session(session_name): path = os.path.expanduser(session_name) session_id = path else: - path = ( - config_dir / SESSIONS_DIR_NAME / hostname / f'{session_name}.json' - ) + path = config_dir / session_hostname_to_dirname(bound_hostname, session_name) session_id = session_name session = Session( path, env=env, session_id=session_id, - bound_host=bound_hostname.split(':')[0], + bound_host=strip_port(bound_hostname), refactor_mode=refactor_mode ) session.load() @@ -142,60 +127,35 @@ class Session(BaseConfigDict): def pre_process_data(self, data: Dict[str, Any]) -> Dict[str, Any]: cookies = data.get('cookies') - if isinstance(cookies, dict): - normalized_cookies = [ - { - 'name': key, - **value - } - for key, value in cookies.items() - ] - elif isinstance(cookies, list): - normalized_cookies = cookies + if cookies: + normalized_cookies = legacy_cookies.pre_process(self, cookies) else: normalized_cookies = [] - should_issue_warning = False for cookie in normalized_cookies: domain = cookie.get('domain', '') - if domain == '' and isinstance(cookies, dict): - should_issue_warning = True - elif domain is None: + if domain is None: # domain = None means explicitly lack of cookie, though - # requests requires domain to be string so we'll cast it + # requests requires domain to be a string so we'll cast it # manually. cookie['domain'] = '' cookie['rest'] = {'is_explicit_none': True} self.cookie_jar.set(**cookie) - if should_issue_warning and not self.refactor_mode: - warning = INSECURE_COOKIE_JAR_WARNING.format(hostname=self.bound_host, session_id=self.session_id) - if not is_anonymous_session(self.session_id): - warning += INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS - - self.env.log_error( - warning, - level='warning' - ) - return data def post_process_data(self, data: Dict[str, Any]) -> Dict[str, Any]: cookies = data.get('cookies') - # Save in the old-style fashion normalized_cookies = [ materialize_cookie(cookie) for cookie in self.cookie_jar ] - if isinstance(cookies, dict): - data['cookies'] = { - cookie.pop('name'): cookie - for cookie in normalized_cookies - } - else: - data['cookies'] = normalized_cookies + data['cookies'] = legacy_cookies.post_process( + normalized_cookies, + original_type=type(cookies) + ) return data @@ -251,7 +211,7 @@ class Session(BaseConfigDict): def cookies(self, jar: RequestsCookieJar): self.cookie_jar = jar - def remove_cookies(self, cookies: Dict[str, str]): + def remove_cookies(self, cookies: List[Dict[str, str]]): for cookie in cookies: remove_cookie_by_name( self.cookie_jar, @@ -293,3 +253,7 @@ class Session(BaseConfigDict): def auth(self, auth: dict): assert {'type', 'raw_auth'} == auth.keys() self['auth'] = auth + + @property + def is_anonymous(self): + return is_anonymous_session(self.session_id)