twilio / twilio-python

A Python module for communicating with the Twilio API and generating TwiML.
MIT License
1.83k stars 699 forks source link

Security Improvemet #774

Closed miguelmir123 closed 3 months ago

miguelmir123 commented 5 months ago

Issue Summary

There is an easy way to improve the security when using the jwt token, in the jwt class the token is not being verify which is common a bad practice in working with this kind of tokens.

Code Snippet

the problems appears here in the jwt class: @classmethod

def from_jwt(cls, jwt, key=""):

    """

    Decode a JWT string into a Jwt object

    :param str jwt: JWT string

    :param Optional[str] key: key used to verify JWT signature, if not provided then validation

                              is skipped.

    :raises JwtDecodeError if decoding JWT fails for any reason.

    :return: A DecodedJwt object containing the jwt information.

    """

    verify = True if key else False

    try:

        headers = jwt_lib.get_unverified_header(jwt)

is in the last line where the token signature is not being verified, The verification can be easily added by using this function to decode the token:

import jwt

jwt.decode(token, key, algorithms="HS256")

I hope it helps improve the code, pd: I am not doing the commit about because I propose this as a part of College Project and I do not have enough time to test it...

miniluz commented 5 months ago

Hello Miguel,

This is not an issue. The verification here is not needed: this is just checking the headers to check that the operation is valid to discard before decoding if it isn't. If the header is valid, it is always decoded and thus verified before processing. The logic is clear:

headers = jwt_lib.get_unverified_header(jwt)

alg = headers.get("alg")
if alg != cls.ALGORITHM:
    raise ValueError(
        f"Incorrect decoding algorithm {alg}, "
        f"expecting {cls.ALGORITHM}."
    )

payload = jwt_lib.decode(
    ...
)
tiwarishubham635 commented 4 months ago

Is this still an issue?

tiwarishubham635 commented 3 months ago

Closing as no response was received in last 30 days