[MIR] python-asgiref

Review for Package: python-asgiref

[Summary]
I can confirm all the checks done when filing this. It seems to be a
useful well maintainer library with not many known issues.

MIR team ACK
Sadly it isn't ready for promotion yet, as due to the nature of the code
between servers and web-apps it does need a security review, so I'll assign
ubuntu-security

List of specific binary packages to be promoted to main: python3-asgiref
Specific binary packages built, but NOT to be promoted to main: None

Notes:
- No TODOs identified at the moment

[Duplication]
OK:
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does not parse data formats
- does process arbitrary web content - it is meant for "web apps <-> server"
  which by its nature is rather exposed

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- does have a non-trivial test suite that runs as autopkgtest
- no new python2 dependency
- Python package, but using dh_python

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok (if needed, e.g. non-native)
- Upstream update history is good
- Debian/Ubuntu update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- It is not on the lto-disabled list

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this case

Required for the new LTS Django in 22.04, setting Critical + Milestone 22.02 (FeatureFreeze)

Hi Server team,

could you, please, take a look into the following lines in wgsi.py:

    def build_environ(self, scope, body):
        ...
        environ = {
            ...
            "SCRIPT_NAME": scope.get("root_path", "").encode("utf8").decode("latin1"),
            "PATH_INFO": scope["path"].encode("utf8").decode("latin1"),
            "QUERY_STRING": scope["query_string"].decode("ascii"),
            ...
        }
    ...

there is a concern around encode and decode non validated data that caught our attention.
could you give us your feedback if you think that it is possible that someone could
use malicious data in order to cause damage to the operation? (maybe some sort of data
garbage in http headers)

thank you very much.

Hi Rodrigo,

I looked into the lines and did find a possible issue.

SCRIPT_NAME and PATH_INFO should not have any issues as the scope's root_path and path are setup as strs beforehand and the conversion encoding utf8 then decoding to latin1 are well defined in this case.

However, QUERY_STRING could cause a crash if the user is able to send in an extended ASCII character with a byte value from 128-255. I tested this by sending a byte array to the function with the first value being 128, aka Ç. It crashed with the following error:

UnicodeDecodeError: 'ascii' codec can't decode byte 0x80 in position 0: ordinal not in range(128)

Django will handle these requests properly without crashing when running alongside asgiref, but on its own this case is not handled.

Thanks

Hi Lena,

Thanks for checking and testing it.

I raised an issue in the upstream to ask about it:
https://github.com/django/asgiref/issues/317

Thanks!

From:
 https://github.com/django/asgiref/issues/317

Upstream confirmed that it is in fact an issue, but, it's not exploitable.
My understanding is that it will hit other guards before falling in that case.
And, changing it would be a potential risk of breaking other things.

I'm pretty satisfied with the upstream response time and quality.

I reviewed python-asgiref 3.5.0-1 as checked into jammy. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

python-asgiref is part of the django framework. It is an interface between
async-capable Python web servers, frameworks, and applications. The package
has ASGI (Asynchronous Server Gateway Interface) base libraries that can be
extended.

- CVE History:
  - None
- Build-Depends?
  - debhelper-compat (= 13)
    dh-python
    python3-async-timeout
    python3-all
    python3-pytest
    python3-pytest-asyncio
    python3-setuptools
    python3-six
- pre/post inst/rm scripts?
  - dh_python3 added
- init scripts?
  - None
- systemd units?
  - None
- dbus services?
  - None
- setuid binaries?
  - None
- binaries in PATH?
  - None
- sudo fragments?
  - None
- polkit files?
  - None
- udev rules?
  - None
- unit tests / autopkgtests?
    Unit tests are executed in build time in python3.9 and python3.10,
    using pytest, which seems good and it is easible executable.
    It has a good integration with the code as the most part of the code
    has its own test routines.
    The same test routine is executed for autopkgtests.

    Every test is passing, including one that is expected to fail at the moment:
      tests/test_sync.py::test_sync_to_async_with_blocker_thread_sensitive

    Things to note:
      In both test versions (3.9 and 3.10) there is a warning:
        PytestConfigWarning: Unknown config option: asyncio_mode
      In test version 3.10 there is a DeprecationWarning:
         DeprecationWarning: There is no current event loop
    event_loop = asyncio.get_event_loop()
- cron jobs?
  - None
- Build logs:
  - No Errors, one warning on setup.py deprecation:
      /usr/lib/python3/dist-packages/setuptools/command/install.py:34:
          SetuptoolsDeprecationWarning: setup.py install is deprecated.
          Use build and pip and other standards-based tools.
  - Lintian is passing.
- Processes spawned?
  - None
- Memory management?
  - None
- File IO?
  - None
- Logging?
  - Everything looks OK.
- Environment variable usage?
  - Yes. ASGI interface with WSGI (wsgi.py) has the WSGI environment variables
    defined. Some are encoded to utf-8 and decoded to latin1, that could bring
    concerns over exploitations because they are not sanitized.
    Also, sync.py is using one enviroment variable from the OS that is not
    sanitized before being converted to integer.
    handled with upstream: https://github.com/django/asgiref/issues/317
- Use of privileged functions?
  - None
- Use of cryptography / random number sources etc?
  - None
- Use of temp files?
  - None
- Use of networking?
  - Yes. There is no defensive code to filter/sanitize untrusted inputs.
- Use of WebKit?
  - None
- Use of PolicyKit?
  - None
- Any significant cppcheck results?
  - None
- Any significant Coverity results?
  - Just one report that is not significant.
- Any significant shellcheck results?
  - None
- Any significant bandit results?
  - None

From a security perspective, the package itself is simple, some security
guards could be extended, like validation of environment variables, but
as already discussed with upstream a...

Security and MIR team ack, this can be promoted

FYI - I now also added the server team subscription which was still missing

Override component to main
python-asgiref 3.5.0-1 in jammy: universe/misc -> main
python3-asgiref 3.5.0-1 in jammy amd64: universe/python/optional/100% -> main
python3-asgiref 3.5.0-1 in jammy arm64: universe/python/optional/100% -> main
python3-asgiref 3.5.0-1 in jammy armhf: universe/python/optional/100% -> main
python3-asgiref 3.5.0-1 in jammy i386: universe/python/optional/100% -> main
python3-asgiref 3.5.0-1 in jammy ppc64el: universe/python/optional/100% -> main
python3-asgiref 3.5.0-1 in jammy riscv64: universe/python/optional/100% -> main
python3-asgiref 3.5.0-1 in jammy s390x: universe/python/optional/100% -> main
8 publications overridden.