Skip to content

Commit

Permalink
Don't send secure cookies by insecure transports (#5839)
Browse files Browse the repository at this point in the history
By default, the transport is secure if https or wss scheme is used.
Use `CookieJar(treat_as_secure_origin="http://127.0.0.1")` to override the default security checker.

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
  • Loading branch information
derlih and asvetlov committed Oct 24, 2021
1 parent f7c2e3d commit ca2a24d
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGES/5571.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Don't send secure cookies by insecure transports.

By default, the transport is secure if https or wss scheme is used.
Use `CookieJar(treat_as_secure_origin="http://127.0.0.1")` to override the default security checker.
28 changes: 25 additions & 3 deletions aiohttp/cookiejar.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import contextlib
import datetime
import os # noqa
import pathlib
Expand All @@ -11,6 +12,7 @@
Dict,
Iterable,
Iterator,
List,
Mapping,
Optional,
Set,
Expand All @@ -23,7 +25,7 @@

from .abc import AbstractCookieJar, ClearCookiePredicate
from .helpers import is_ip_address, next_whole_second
from .typedefs import LooseCookies, PathLike
from .typedefs import LooseCookies, PathLike, StrOrURL

__all__ = ("CookieJar", "DummyCookieJar")

Expand Down Expand Up @@ -59,7 +61,8 @@ def __init__(
*,
unsafe: bool = False,
quote_cookie: bool = True,
loop: Optional[asyncio.AbstractEventLoop] = None
treat_as_secure_origin: Union[StrOrURL, List[StrOrURL], None] = None,
loop: Optional[asyncio.AbstractEventLoop] = None,
) -> None:
super().__init__(loop=loop)
self._cookies = defaultdict(
Expand All @@ -68,6 +71,18 @@ def __init__(
self._host_only_cookies = set() # type: Set[Tuple[str, str]]
self._unsafe = unsafe
self._quote_cookie = quote_cookie
if treat_as_secure_origin is None:
treat_as_secure_origin = []
elif isinstance(treat_as_secure_origin, URL):
treat_as_secure_origin = [treat_as_secure_origin.origin()]
elif isinstance(treat_as_secure_origin, str):
treat_as_secure_origin = [URL(treat_as_secure_origin).origin()]
else:
treat_as_secure_origin = [
URL(url).origin() if isinstance(url, str) else url.origin()
for url in treat_as_secure_origin
]
self._treat_as_secure_origin = treat_as_secure_origin
self._next_expiration = next_whole_second()
self._expirations = {} # type: Dict[Tuple[str, str], datetime.datetime]
# #4515: datetime.max may not be representable on 32-bit platforms
Expand Down Expand Up @@ -225,7 +240,14 @@ def filter_cookies(
SimpleCookie() if self._quote_cookie else BaseCookie()
)
hostname = request_url.raw_host or ""
is_not_secure = request_url.scheme not in ("https", "wss")
request_origin = URL()
with contextlib.suppress(ValueError):
request_origin = request_url.origin()

is_not_secure = (
request_url.scheme not in ("https", "wss")
and request_origin not in self._treat_as_secure_origin
)

for cookie in self:
name = cookie.key
Expand Down
14 changes: 10 additions & 4 deletions docs/client_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,7 @@ BasicAuth
CookieJar
^^^^^^^^^

.. class:: CookieJar(*, unsafe=False, quote_cookie=True, loop=None)
.. class:: CookieJar(*, unsafe=False, quote_cookie=True, treat_as_secure_origin = [])

The cookie jar instance is available as :attr:`ClientSession.cookie_jar`.

Expand Down Expand Up @@ -1810,11 +1810,17 @@ CookieJar

.. versionadded:: 3.7

:param bool loop: an :ref:`event loop<asyncio-event-loop>` instance.
See :class:`aiohttp.abc.AbstractCookieJar`
:param treat_as_secure_origin: (optional) Mark origins as secure
for cookies marked as Secured. Possible types are

.. deprecated:: 2.0
Possible types are:

- :class:`tuple` or :class:`list` of
:class:`str` or :class:`yarl.URL`
- :class:`str`
- :class:`yarl.URL`

.. versionadded:: 3.8

.. method:: update_cookies(cookies, response_url=None)

Expand Down
1 change: 1 addition & 0 deletions docs/spelling_wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ fallback
fallbacks
filename
finalizers
formatter
formatters
frontend
getall
Expand Down
3 changes: 2 additions & 1 deletion docs/testing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,8 @@ Test Client
first with ``TestServer(app)``.

:param cookie_jar: an optional :class:`aiohttp.CookieJar` instance,
may be useful with ``CookieJar(unsafe=True)``
may be useful with
``CookieJar(unsafe=True, treat_as_secure_origin="http://127.0.0.1")``
option.

:param str scheme: HTTP scheme, non-protected ``"http"`` by default.
Expand Down
32 changes: 32 additions & 0 deletions tests/test_cookiejar.py
Original file line number Diff line number Diff line change
Expand Up @@ -739,3 +739,35 @@ async def test_cookie_jar_clear_domain():
assert morsel.value == "bar"
with pytest.raises(StopIteration):
next(iterator)


@pytest.mark.parametrize(
"url",
[
"http://127.0.0.1/index.html",
URL("http://127.0.0.1/index.html"),
["http://127.0.0.1/index.html"],
[URL("http://127.0.0.1/index.html")],
],
)
async def test_treat_as_secure_origin_init(url) -> None:
jar = CookieJar(unsafe=True, treat_as_secure_origin=url)
assert jar._treat_as_secure_origin == [URL("http://127.0.0.1")]


async def test_treat_as_secure_origin() -> None:
endpoint = URL("http://127.0.0.1/")

jar = CookieJar(unsafe=True, treat_as_secure_origin=[endpoint])
secure_cookie = SimpleCookie(
"cookie-key=cookie-value; HttpOnly; Path=/; Secure",
)

jar.update_cookies(
secure_cookie,
endpoint,
)

assert len(jar) == 1
filtered_cookies = jar.filter_cookies(request_url=endpoint)
assert len(filtered_cookies) == 1

0 comments on commit ca2a24d

Please sign in to comment.