Skip to content

Add SSL_CTX_set_security_level functions #9523

Description

@fredericgermain

OpenSSL introduced a string restriction on usage of SSL3/TLSv1 in openssl 3.1
(openssl/openssl#19867 (comment))

To override these restrictions, we need tp switch security level to 0.

This can be done in at least two ways :

  • Use SSL_CTX_set_security_level
  • Add '@SECLEVEL=0' as a cipher

It would be nice to make SSL_CTX_set_security_level/SSL_CTX_get_security_level available as binding.

Context: work with transparent proxy on windows XP for education purpose

Activity

  1. alex commented on Aug 31, 2023

    @alex
    Member

    Can you provide more context on how you'd expect these to be used? Do you use our bindings directly, are you expecting to use them in a follow up PR to pyOpenSSL, something else?

    Candidly if the only use case involves Windows XP, we're unlikely to accept this if there's no other valuable use case.

  2. added
    waiting-on-reporterIssue is waiting on a reply from the reporter. It will be automatically cloesd if there is no reply.
    on Aug 31, 2023
  3. fredericgermain commented on Aug 31, 2023

    @fredericgermain
    Author

    I plan to push a commit directly in mitmproxy,
    They already do some not-so-pretty calls to the bindings directly, not using pyOpenSSL.

    My use case involves windows XP and TLS1.
    TLS1.1 might be more used though, there are probably some "educationnal" use case here.

    But probably better uses of this API would be to increase the security level?

    cf "man SSL_CTX_set_security_level"

  4. alex commented on Aug 31, 2023

    @alex
    Member

    @mhils is this something y'all would find useful? If so, happy to land.

  5. mhils commented on Aug 31, 2023

    @mhils
    Member

    Support for old/insecure versions is generally something we find very useful. "I want to use mitmproxy to transparently upgrade insecure protocols to latest TLS" comes up surprisingly often as a use case. That being said, insecure versions will always be off-by-default (see https://docs.mitmproxy.org/stable/concepts-options/#tls_version_client_min).

    @fredericgermain, from your initial post it sounds like adding @SECLEVEL=0 to the cipher spec would achieve the same result, or am I misreading things? How is calling SSL_CTX_set_security_level better/preferable?

  6. fredericgermain commented on Aug 31, 2023

    @fredericgermain
    Author

    @mhils SSL_CTX_set_security_level feels more something to used, and @SECLEVEL sounds a little bit hacky. This is really open to discussion.

    The openssl doc about this can be found in man SSL_CTX_set_security_level for instance.

    I planed to push something like this in mitmproxy/net/tls.py anyway

        if min_version < Version.TLS1_2:
            if hasattr(SSL._lib, "SSL_CTX_set_security_level"):
                SSL._lib.SSL_CTX_set_security_level(context._context, 0)
            else:
                cipher_list += ('@SECLEVEL=0',)
    

    I need to check and test more, but you might need ALL@SECLEVEL=0 or DEFAULT@SECLEVEL=0, and not only @SECLEVEL=0, so my code might be wrong on the else case

    That being said, to actually support TLS1, you also need a special version of openssl, configured with enable-weak-ssl-ciphers (the one from Ubuntu 22.04 won't do for instance).

    I plan to push all that in mitmproxy, but I wanted to see if this would be integrated first.

    Other problem just discovered with the CI, SSL_CTX_set_security_level is still not ported on boringssl, so it looks maybe too much work for what it is since we have the @SECLEVEL=X hack for now.

  7. mhils commented on Aug 31, 2023

    @mhils
    Member

    If @SECLEVEL provides the same capabilities with fewer bindings and without BoringSSL/OpenSSL interop issues, I'd prefer if we stick to that. In mitmproxy, I could see something like this to be useful:

    if min_version < Version.TLS1_2 and "@SECLEVEL" not in cipher_list:
        logger.alert(
            "Custom ciphers may not be working as expected if @SECLEVEL is not set in the cipher list, "
            "see https://www.openssl.org/docs/man1.1.1/man1/ciphers.html"
        )

    But let's move that discussion over to our issue tracker. Feel free to open something there. :)

    Happy to revisit this if we have a good justification for adding SSL_CTX_set_security_level, but I'd rather keep the number of bindings in cryptography and the number of options in mitmproxy small.

  8. fredericgermain commented on Sep 1, 2023

    @fredericgermain
    Author

    @mhils I opened a topic at mitmproxy/mitmproxy#6185

  9. github-actions commented on Sep 5, 2023

    @github-actions

    This issue has been waiting for a reporter response for 3 days. It will be auto-closed if no activity occurs in the next 5 days.

  10. github-actions commented on Sep 10, 2023

    @github-actions

    This issue has not received a reporter response and has been auto-closed. If the issue is still relevant please leave a comment and we can reopen it.

  11. locked as resolved and limited conversation to collaborators on Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Stalewaiting-on-reporterIssue is waiting on a reply from the reporter. It will be automatically cloesd if there is no reply.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions