Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibreSSL does not tolerate setting minimum_version greater than maximum_version #78182

Closed
alanhuang122 mannequin opened this issue Jun 29, 2018 · 4 comments
Closed

LibreSSL does not tolerate setting minimum_version greater than maximum_version #78182

alanhuang122 mannequin opened this issue Jun 29, 2018 · 4 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes tests Tests in the Lib/test dir topic-SSL type-bug An unexpected behavior, bug, or error

Comments

@alanhuang122
Copy link
Mannequin

alanhuang122 mannequin commented Jun 29, 2018

BPO 34001
Nosy @Yhg1s, @tiran, @alex, @dstufft, @matrixise, @alanhuang122
PRs
  • bpo-34001: Change handling of SSL protocol bounds with LibreSSL #8055
  • bpo-34001: Fix test_ssl with LibreSSL #13783
  • [3.8] bpo-34001: Fix test_ssl with LibreSSL (GH-13783) #15997
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tiran'
    closed_at = <Date 2021-04-17.19:36:30.716>
    created_at = <Date 2018-06-29.16:09:13.706>
    labels = ['3.7', 'expert-SSL', '3.8', 'type-bug', 'tests']
    title = 'LibreSSL does not tolerate setting minimum_version greater than maximum_version'
    updated_at = <Date 2021-04-17.19:36:30.716>
    user = 'https://github.com/alanhuang122'

    bugs.python.org fields:

    activity = <Date 2021-04-17.19:36:30.716>
    actor = 'christian.heimes'
    assignee = 'christian.heimes'
    closed = True
    closed_date = <Date 2021-04-17.19:36:30.716>
    closer = 'christian.heimes'
    components = ['Tests', 'SSL']
    creation = <Date 2018-06-29.16:09:13.706>
    creator = 'Alan.Huang'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34001
    keywords = ['patch']
    message_count = 4.0
    messages = ['320722', '320887', '351990', '352006']
    nosy_count = 7.0
    nosy_names = ['twouters', 'janssen', 'christian.heimes', 'alex', 'dstufft', 'matrixise', 'Alan.Huang']
    pr_nums = ['8055', '13783', '15997']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue34001'
    versions = ['Python 3.7', 'Python 3.8']

    @alanhuang122
    Copy link
    Mannequin Author

    alanhuang122 mannequin commented Jun 29, 2018

    LibreSSL has a function called ssl_clamp_version_range that is called before attempting to set the minimum and maximum protocol versions in ssl_version_set_{min,max}. The function disallows setting ranges that are invalid (i.e., where minimum_version > maximum_version). OpenSSL does not disallow this behavior.

    As a result, attempting to set a minimum_version greater than a maximum_version results in a ValueError when Python is built with LibreSSL.

    There are two things that might need fixing here:

    1. Replace the ValueError "Unsupported protocol version 0x%x" message with a more generic one. The assumption that the only way the operation can fail is if the underlying library does not support the protocol version is incorrect.
      This can be done by either making the message more generic or by introducing another error message to handle this case.
    2. Change test_min_max_version lines 3575-3576 to set the maximum_version before the minimum_version.

    Here's some Python code to reproduce the above-mentioned error:

    import ssl
    
    ctx = ssl.SSLContext()
    ctx.maximum_version = ssl.TLSVersion.TLSv1_1
    ctx.minimum_version = ssl.TLSVersion.TLSv1_2
    
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/alan/src/cpython/Lib/ssl.py", line 491, in minimum_version
        super(SSLContext, SSLContext).minimum_version.__set__(self, value)
    ValueError: Unsupported protocol version 0x303
    

    Here's some example C code:

    #include <openssl/ssl.h>
    #include <openssl/ossl_typ.h>
    #include <stdio.h>
    
    int main(){
        SSL_CTX *ctx = NULL;
        ctx = SSL_CTX_new(TLS_method());
    
        printf("setting max to TLSv1.1: ");
        if(SSL_CTX_set_max_proto_version(ctx, TLS1_1_VERSION)){
            printf("success\n");
        }   
        else{
            printf("failure\n");
        }   
        printf("setting min to TLSv1.2: ");
        if(SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION)){
            printf("success\n");
        }   
        else{
            printf("failure\n");
        }   
    
        printf("min ver: %d\n", SSL_CTX_get_min_proto_version(ctx));
        printf("max ver: %d\n", SSL_CTX_get_max_proto_version(ctx));
        return 0;
    }
    

    Under LibreSSL 2.7.4, this produces:

    setting max to TLSv1.1: success
    setting min to TLSv1.2: failure
    min ver: 769
    max ver: 770
    

    Under OpenSSL 1.1.0g, this produces:

    setting max to TLSv1.1: success
    setting min to TLSv1.2: success
    min ver: 771
    max ver: 770
    

    The test that failed:
    ======================================================================
    ERROR: test_min_max_version (test.test_ssl.ThreadedTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/alan/src/cpython/Lib/test/test_ssl.py", line 3575, in test_min_max_version
        server_context.minimum_version = ssl.TLSVersion.TLSv1_2
      File "/home/alan/src/cpython/Lib/ssl.py", line 491, in minimum_version
        super(SSLContext, SSLContext).minimum_version.__set__(self, value)
    ValueError: Unsupported protocol version 0x303

    @alanhuang122 alanhuang122 mannequin added 3.7 (EOL) end of life 3.8 only security fixes labels Jun 29, 2018
    @alanhuang122 alanhuang122 mannequin assigned tiran Jun 29, 2018
    @alanhuang122 alanhuang122 mannequin added tests Tests in the Lib/test dir topic-SSL type-bug An unexpected behavior, bug, or error labels Jun 29, 2018
    @alanhuang122
    Copy link
    Mannequin Author

    alanhuang122 mannequin commented Jul 2, 2018

    Strangely, LibreSSL's ssl_clamp_version_range function is perfectly happy to accept minimum protocol versions lower than the lowest supported protocol version, and likewise is happy to accept maximum protocol versions higher than the highest supported protocol version.
    In said case, the minimum/maximum protocol version is set to clamp_min/clamp_max (i.e., the minimum/maximum protocol version supported by the internal method).

    As such, the assertion test ctx.minimum_version = 42 on line 1127 in test_min_max_version in test_ssl.py fails.

    A possible fix would be to add another check in set_min_max_proto_version _ssl.c that checks if the current set protocol version is equal to the value passed (with exceptions for the magic constants of MINIMUM_SUPPORTED and MAXIMUM_SUPPORTED), and if not, raise a ValueError as well.

    One dilemma is whether to reset the respective version back to what it was before the attempt, which I think should be done.

    @Yhg1s
    Copy link
    Member

    Yhg1s commented Sep 11, 2019

    New changeset c9bc49c by T. Wouters (Christian Heimes) in branch 'master':
    bpo-34001: Fix test_ssl with LibreSSL (GH-13783)
    c9bc49c

    @matrixise
    Copy link
    Member

    New changeset d6ac67f by Stéphane Wirtel (Miss Islington (bot)) in branch '3.8':
    bpo-34001: Fix test_ssl with LibreSSL (GH-13783) (bpo-15997)
    d6ac67f

    @tiran tiran closed this as completed Apr 17, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    CyberTailor added a commit to CyberTailor/asyncpg that referenced this issue Nov 2, 2022
    CyberTailor added a commit to CyberTailor/asyncpg that referenced this issue Nov 2, 2022
    CyberTailor added a commit to CyberTailor/asyncpg that referenced this issue Nov 29, 2022
    elprans pushed a commit to MagicStack/asyncpg that referenced this issue Dec 3, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes tests Tests in the Lib/test dir topic-SSL type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants