From 73d03d525d713dcd7111a18e7bff657cce68bca9 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 11:24:10 +0200 Subject: [PATCH 01/12] test: update `openssl` command, re-gen local cert and key Signed-off-by: Norbert Biczo --- resources/test_ssl.cert | 30 --------- resources/test_ssl.crt | 19 ++++++ resources/test_ssl.key | 76 ++++++++--------------- test_integration/test_ssl_verification.py | 15 ++--- 4 files changed, 51 insertions(+), 89 deletions(-) delete mode 100644 resources/test_ssl.cert create mode 100644 resources/test_ssl.crt diff --git a/resources/test_ssl.cert b/resources/test_ssl.cert deleted file mode 100644 index f25d3c4..0000000 --- a/resources/test_ssl.cert +++ /dev/null @@ -1,30 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIFJTCCAw2gAwIBAgIUfElLZwAd6ZSPKCWsVzp5vjUmytQwDQYJKoZIhvcNAQEL -BQAwITELMAkGA1UEBhMCVVMxEjAQBgNVBAMMCWxvY2FsaG9zdDAgFw0yNDAxMjQx -MTM5MzRaGA8yMTIzMTIzMTExMzkzNFowITELMAkGA1UEBhMCVVMxEjAQBgNVBAMM -CWxvY2FsaG9zdDCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBAMT6hPMi -5KO3Jeky+lhsguEqngxKsfSBlicjRGHrYX62ouGafcK4KZJkgwoC7XECi1O//KAJ -EPoOxyesxZKpxa3X+AetI7zUecyfA0J0qQbgtYns4zHOwfpPG4wyprzcEQ9+xlMp -iMJYpY1e0YzqHKaiSlHv6KJBWuNhT/LS7b5P98JJzJtcUQn8fDZ5D7WM+nsHMFn8 -Rt9C715GlCyGBS3qe2Eme23h5XvUM/Tqp9f/FmZ0t9GrEovhDScGvtxO3dQ5hLHV -ifhEKyIGhcKoG5+53iYuWaRDhgnLtKZjNg/cAEkmpU0NgORP3tbNKD4wtGJMXQA8 -uAc0dXsWfFYqMR9i+rM3F3bqV7VSOaw0rfZSztlQhfgm4XuZTouMF9jHE6xFvLum -Bjvzq2YtR/Xdv1lygRzXA6uMO2ppnAw9ofVn2cBirjdi6qzAzf+X+yiORP2QXcG7 -BoGYU3JDLMZVMpQ81/Jm8HuJfX2yBXcKtinXjWCa4up9dI4+RVQg0wyIKtTLpxBh -Gw/iRzyzAto7cjD9NOr7MsqqmhpCwowHeitqCCPJfTmcFxaiKA6SzyUS4+TRJUnf -2DWGjDyb8kfg2NkFVeUkn6WXx8VsbvMYg2VCdpIO0XK40ofBBuVaOrPrQpm6fyFf -6SFusvTOuXJpnI4CeyefO2Nd+2xjN9balXFtAgMBAAGjUzBRMB0GA1UdDgQWBBSx -Lwf6XjMsF54JX6SBpYhwbw8SezAfBgNVHSMEGDAWgBSxLwf6XjMsF54JX6SBpYhw -bw8SezAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4ICAQCEgNyk5Bon -F9hDHb6jiR0p0RkNiPZeOkFFHyonmG7WTkQMJElaFJJQN2wuPeok8ky0B65S4V8e -uGRQSvZ9rt/h5/K09G9h/haN/UTWXj1AUWcblC8hw8ZnOSg2tLkOLwqQsb+Q4pUM -eS+Gr10cTENnW0hwG5husiiNj58Mr1+CL+LqfmHPK85vPco2Q0uQ4MEpmtwM5R2g -t4d7hI81vXKeTBTeB6sEI9EkHPTyy1oTk7uEtBK+d62ZJPQCsco1CExNRIkYTUgh -LIIyDfXxO8Vo2fhbLM95rjlDdoM87Db79EQd6REdwLh1lRByD3IvkXouZnbVfkaw -BeCOGtDjcTXcNHTVdjmNZ723sU4OSAqHDyVazeIpKZbOKIRlHgYqylYcxT/MOL0l -Mtzw2o83xoSYrMQ8rcYO5RX1EIPwUijo3k3T8OMPZbYP724hu3YxbEF+kOgCty4A -9UTDIUqlr6Y25C/vf3L792nrhduqrWALXumbENVelqow8u2HBaMj1k+LjVSNMIa+ -HFP8ex7bYdNjjikTHDM/a4Rs+yBg2WWkfvcYRdkn/IkjAPt4vjdiuPD5pDmjkIBt -ZFVUM9U6V+DAlPULZXJOb348K0KcnQdbsn9ti+LFWsfLJSaUbAirt4yQ/H6FW35u -iyFlAJPkdeGhoBhtbf5f2IVHqcvrOO34Dw== ------END CERTIFICATE----- diff --git a/resources/test_ssl.crt b/resources/test_ssl.crt new file mode 100644 index 0000000..e3bfa1b --- /dev/null +++ b/resources/test_ssl.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDETCCAfmgAwIBAgIUVQfATsBxBkHqAgmrv9Eb/KQhA2IwDQYJKoZIhvcNAQEL +BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MCAXDTI0MDcwMjA5MTkyN1oYDzIxMjQw +NjA4MDkxOTI3WjAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwggEiMA0GCSqGSIb3DQEB +AQUAA4IBDwAwggEKAoIBAQCx7eyXu0A1IH18U0fmVbHlC0OpzpGfGOQayrlrxyxN +kNc8T2ehnt7W33FHI2tjcmO11wgsN+U2+uB2aq0q1OYaqo3OlZtTF4A91CgAdbGd +Ix5aEEzjogiQIBvrBQhaU6uFTzUBQ5tWs+pLcorVrp8G/ONN/1e4Z3NCg036ibSs +Vkfdw1zX6vTR674uTq8aIG7sH3DCF1Q+CzvxhQrhjkZOha+u0H+OhZ9yd30hU/xy +AKZsoGHNY65bVSYAPxP6XLw5inF534TLriggFDonEk16eHjAi7SxcjdKcyxhfX7u +DefD/s9cKUY4Tf1JeAx2F1Y++ffqWlQSde5DKkfC6xU5AgMBAAGjWTBXMBQGA1Ud +EQQNMAuCCWxvY2FsaG9zdDALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH +AwEwHQYDVR0OBBYEFIHsshXxnhcdXRSPxHvTvfMWhfTgMA0GCSqGSIb3DQEBCwUA +A4IBAQCnEU3NtmskVv2/3U0DQM8FM8jKc/V7WCfK/PWPFUZgCkj8pUg4yYxjOZ4K +pFV7cPJjtArmjyU9lB0g2wQvlpXEYbDJ5K0LK9GsdhowZQatZaTB0nVZeG87mlV7 +kxrQTMsMTYf5I6S3SW63SorlJQiuaQjOKwvommCS+6Q5goEOodZpGr+5sQSuRLlw +XMKzcU7ZDfe1jidjjcWSyf6UMKB/mhMQVMTTDURt/jS7koA5lXiU+m0XCSi28wTr +lV9ZRzZiE4mRDANlEkoqUCYPG/PDF0KOgDROiavlrBcBycsqBD5iRdGYGaYwUmSo +UpaVGGLYBguqwEeQ2ixC2rTGkyg7 +-----END CERTIFICATE----- diff --git a/resources/test_ssl.key b/resources/test_ssl.key index 9b7084e..a4ae056 100644 --- a/resources/test_ssl.key +++ b/resources/test_ssl.key @@ -1,52 +1,28 @@ -----BEGIN PRIVATE KEY----- -MIIJRAIBADANBgkqhkiG9w0BAQEFAASCCS4wggkqAgEAAoICAQDE+oTzIuSjtyXp -MvpYbILhKp4MSrH0gZYnI0Rh62F+tqLhmn3CuCmSZIMKAu1xAotTv/ygCRD6Dscn -rMWSqcWt1/gHrSO81HnMnwNCdKkG4LWJ7OMxzsH6TxuMMqa83BEPfsZTKYjCWKWN -XtGM6hymokpR7+iiQVrjYU/y0u2+T/fCScybXFEJ/Hw2eQ+1jPp7BzBZ/EbfQu9e -RpQshgUt6nthJntt4eV71DP06qfX/xZmdLfRqxKL4Q0nBr7cTt3UOYSx1Yn4RCsi -BoXCqBufud4mLlmkQ4YJy7SmYzYP3ABJJqVNDYDkT97WzSg+MLRiTF0APLgHNHV7 -FnxWKjEfYvqzNxd26le1UjmsNK32Us7ZUIX4JuF7mU6LjBfYxxOsRby7pgY786tm -LUf13b9ZcoEc1wOrjDtqaZwMPaH1Z9nAYq43YuqswM3/l/sojkT9kF3BuwaBmFNy -QyzGVTKUPNfyZvB7iX19sgV3CrYp141gmuLqfXSOPkVUINMMiCrUy6cQYRsP4kc8 -swLaO3Iw/TTq+zLKqpoaQsKMB3oraggjyX05nBcWoigOks8lEuPk0SVJ39g1how8 -m/JH4NjZBVXlJJ+ll8fFbG7zGINlQnaSDtFyuNKHwQblWjqz60KZun8hX+khbrL0 -zrlyaZyOAnsnnztjXftsYzfW2pVxbQIDAQABAoICABLp3h2tbkhFA/QgE7ctViTS -L4JNIsiwL6963KxNSlt9JGcmqygpAD7g/U8XCF8HSEMGpnZkYHeuNxO5bHAgco12 -dQehqZKOUVKjOxAkvP0e0veXIhqMeIY1FddQnr94HwA+o0LldE767YyFO/g8m3sp -jprO/yajQVufYqqVc8QIEClc5jNNui9MCc4+MhKz4nIxNsSRK2nxFqRWARDEXpdx -0h56MDRVEjChZ8q+xFaCVQ+J6gONGlcJiTaD2Iw1W2nvCu17bEfFHeIiv7G47Awa -b/j5Dtztqd9jaqlmUdDUhkd/2TPslcF2ZNZ5tQFBsnRU0kI9UktIz3X96vroCrbG -aRryT6sJUHdvdc+d6l53sJLgpviQLHlDiovMSOWeNw+YdUzc55kiwubO4S0AuNho -M93LUuAjqoSpbBuwU9vZcLHKlhzsp290KfSbzEhIPsgpHotw9B1sjrM7ykiT9ggc -w0aaOL5bOc8RHikTARzA9dyR0nJLE22ZTgbtgUoDgku8g5z0QLpGxOkV14qcSthx -TS5qRoRm7gvJo6d474cW1CHjooxdkCqpQYjZ0V4HzqemFjd8M0FGGbsuA/okWzT9 -YKtF5VyBhvgkb0uLAkPb20eauLHZVS7Nm6mU6yf3nD9rYZo4W5bMXZuWt43AXxY7 -B0PnWJPH+VIfIl0ptuDZAoIBAQDnJYuETD1gQSsotZekUZtqyj3kz1NPA2o2tggn -hNgLNKHBbjH0+H0/kj2X1vTuC1LCaISRggEXkqa6O5m7N33idNR8YVL3lBltI1gW -56rIYUggjMyWHUQXF33ALvP2RuLKLHToms/p6tx73/TCLCa4RAB5HOGMVyGwuRoW -73xrdZNRQE0S+fNw/b13GWtMOv8NBHu48BAr1GJ9g/sfXLz/bKvl9A+BleKO2s+q -R9d4jzmz5rnR6S/QJA8Q3u0/belZI71mUyL/9MDAFD4C9v1+DYHdS7LW/Is1i8hH -iGOL683+Ouq6xIgABwyoBwdGqtnnVMa5/OVUCm8EcyUuvoPJAoIBAQDaKHoiGCuY -sVLT6/VABmtw6Q17r+w0CmSo+QRR9u5sKx+mdANU86qSXyDXwNiqY0HoioD5Ng9y -SHpMMaDWoAmbcSMNeHjdwuLgPGKzeZ6aH53OAaazHzrafruG8nTiZkt0/kh0bLvY -rubck5DHmdZJuSCEwGfuzBgZLCrsjwkjawg57+maMmEabyTJy18YfTBI3s7c2AXz -qcqJnhUMKLL7LmGtBlox6m2C0AXjgXDjHO0R4RqJ1yndN5UUEzxZigsuIxjM41JB -7QHJ7B1uPhFf5omZDjQRbqY8pB+KiQbRtP5Bwz+LvBy0NelMIOa116Hv3V4igFxv -AtCKbju2HCqFAoIBAQDY9+AfHiVajbGCc/pUrpmRQyeX+Jh9iXoQwwuidMsKsavI -UrSn+vwuSQpx1b9xFsXnYI5Xu01lIC5Kj5l9J9iNUhcGbaCgbq7zSALu9STVFKPM -kf2URwJcHpvWYvxzRxSoq9RNZswVCXVO/ejUvvbVbld3WAnLXxprtURtFP2YLPRM -h2wRjPfbLwLCoeSa2KICSRwNe6HiUmjk4pc9WCK8K/irUE2h2NyiNXhKoUb7jo2e -dcwk4psT6FUQBAF00aoBF1A4lX87/TVU12tiAw/tW6Zz4BOOQ940M/KaWsb+Vyi0 -I/+jssjqJbPWoUpOJh+GSoiDmoR1P5n39lGHsCMpAoIBAQCX6lnqRhSN1uWLx6NX -+2B0FwYZnI8KSjaAaC+2+BJdZsY6fk0XqjqchPv04ki+ljH+QfzADgJBnfD0ABc1 -fepSwT0ck0jvfFfKuKIuwsFMKDoWi5XO5C9ymY/y0AHO6lcfWDeSQ2mn4VvIPEY0 -iI7tdaoMZ4O4iY06ckRNyOkfLdhjqApvIyf1ZXIjx6goAH1QMT+yEAhM/m6Y2Gll -ty2ztj+0YlkKq2mpDz0aiTfYH3uC2NNHK3runlcEzMRYwcU5Up1hh+bvG6EEQJTa -AQTOWFZ3K6ncfcXrMor4SKVkAPqRRuqIXu1KHMSiC8M827TbuLZlpic38qjPzSVt -kj2VAoIBAQDMiurZJIVLZ+aX/TJ7DoKCkhD/hL2delu0prLBQrd05/0axEXspM03 -kIvO+vQ5CzQ9JssA4A3vYni68yrDLUtucxwaEADzA6vLy6re7Y+ApLFHv5fgQYX9 -EK830RUuVgI9XQc7O0ziAb1CVGg/XaKzuFfubbJIMHPEGVsGll18L1sJCfcgCQpa -pbSyRFUROdR6kDQUsMv3uq1LFLeVL5hYyS4k/LRvlg/3+Zk0XIKOaj5nfk1ax2bF -uViwPLq4l+CAtHNcASv30+P2ejmZDqOt2ctiFhjtZZPg8+LC7gzkJh+e/2FTkBk1 -l7zlgithSVBlZvD3zalH8RxDX+9NhnHw +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCx7eyXu0A1IH18 +U0fmVbHlC0OpzpGfGOQayrlrxyxNkNc8T2ehnt7W33FHI2tjcmO11wgsN+U2+uB2 +aq0q1OYaqo3OlZtTF4A91CgAdbGdIx5aEEzjogiQIBvrBQhaU6uFTzUBQ5tWs+pL +corVrp8G/ONN/1e4Z3NCg036ibSsVkfdw1zX6vTR674uTq8aIG7sH3DCF1Q+Czvx +hQrhjkZOha+u0H+OhZ9yd30hU/xyAKZsoGHNY65bVSYAPxP6XLw5inF534TLrigg +FDonEk16eHjAi7SxcjdKcyxhfX7uDefD/s9cKUY4Tf1JeAx2F1Y++ffqWlQSde5D +KkfC6xU5AgMBAAECggEABbqBSYlP0eYP5DbSM8pChftM3GS4L4UfovUv7xZkiMLH +CzwLPBrfVc+v1/h99p+yMiKQMsxB5vlAzM82cBCWr/kZw7LxY0V4bYUtHIathz+g +NIodz55h5DIEdBafZDkZZptcO4QvtiTowDEZ4zNSD2mI7/PuoRNDlLqhghV46at3 +nfJmOgbWxzciOhpYUXtXl1+n1ywkXIbL58sfp5HgkuM9dL2BTZQdUwbS58TgyYla +DBYZQfiZ+MgrCqJKCYEy+lycVHz9LaSCVXgEYmstfcLqdC7X3/nQuUn587rV5FDA +75bDZ7r2jvrQT7/MEGm3b2fgk0+0gMew52wrDsncuQKBgQDygvVlm6BU2cIm8qvR +l1LqU06jbEVW2fzrXi73RVgCR+ZQoy+hB8LviqNO0ayAOWIH/6iMGU09bK72DlJ0 +K4P58X6iJaop1ntdYa2DApPzrz3/4weMl4ZVjz5tPBUxhkfJCMfZF2q+CoHOsdlQ +rTXNFQgfq4XNGpj3YX0L94pkgwKBgQC702bRFk7lHoMyn8hs8MQqumSsefpTsc6v +3fLl6DgwhU9/FAEymIt+wEIchoU0GkVfZ16OmO1whZAgj3zsBr/zG7C7Din9s7hB +KV57o7KIRP3IUo+N6qQ2xNPG8lrCd7Xz+430RHT5BRudqNXWsZziFOrPXZz7Yh8/ +TMy4ZwzKkwKBgQCOTToh/Uf/gifjItKfkeQdi/TBAG9Pn2pB0mpMvmv+KqKC/r6c +Bynj1b4uKerG8uULPIFydAZW3MdtqsnHUSGIMKTWELPhCPIqwX5HOeQHQfVniZiM +bv1shzlib7cf8GN/G5/pS0xfZ1r0JngWVw0S4hx6OPOyfsDzqEjwFLkocQKBgBOQ +2xYO19sgSZR9dph6oES/M/uPnVcYn6pMWaA/h5LuYDChudo2b9mdV4W3MasSzYU5 +tGzwW1OsZi4uJFpF/brqeIeT2yX1kc0f7Rq+G7v8S9+RUij7d23JJTKFTpUReV/Y +JZp7gx/pu026J8R8rhYTDb7aRp8dQpoKewz+lyOHAoGAUGOfsLybzE2w/gL1gZ8W +L6sdkQ7zq/IF04cqM5jw1g82f9CGdZlWrT6wv5D++O9zpZSmuwJ2p/os5Tsygmb4 +M0IjG23Mw5IWLCC6n2riYwpQ8sjuL3SOhqgt4k5mnu4H0RpVLz69JPBJkaGuE1Ld +k4zeVfI1+X2clcHVfDS6xhc= -----END PRIVATE KEY----- diff --git a/test_integration/test_ssl_verification.py b/test_integration/test_ssl_verification.py index 4b4e2c0..cc9cadd 100644 --- a/test_integration/test_ssl_verification.py +++ b/test_integration/test_ssl_verification.py @@ -12,15 +12,12 @@ # The certificate files that are used in this tests are generated by this command: -# openssl req \ -# -new \ -# -newkey rsa:4096 \ -# -days 36500 \ -# -nodes \ -# -x509 \ -# -subj "/C=US/CN=localhost" \ -# -keyout test_ssl.key \ -# -out test_ssl.cert +""" +openssl req -x509 -out test_ssl.crt -keyout test_ssl.key \ + -newkey rsa:2048 -nodes -sha256 -days 36500 \ + -subj '/CN=localhost' -extensions EXT -config <( \ + printf "[dn]\nCN=localhost\n[req]\ndistinguished_name = dn\n[EXT]\nsubjectAltName=DNS:localhost\nkeyUsage=digitalSignature\nextendedKeyUsage=serverAuth") +""" def test_ssl_verification(): From f65e9653b1a7b457af2bf8b726aaf3257630f1da Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 11:24:48 +0200 Subject: [PATCH 02/12] test: add local cert verification Signed-off-by: Norbert Biczo --- test_integration/test_ssl_verification.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test_integration/test_ssl_verification.py b/test_integration/test_ssl_verification.py index cc9cadd..ce3d344 100644 --- a/test_integration/test_ssl_verification.py +++ b/test_integration/test_ssl_verification.py @@ -22,7 +22,7 @@ def test_ssl_verification(): # Load the certificate and the key files. - cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.cert') + cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.crt') key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key') # Build the SSL context for the server. @@ -30,7 +30,7 @@ def test_ssl_verification(): ssl_context.load_cert_chain(certfile=cert, keyfile=key) # Create and start the server on a separate thread. - server = HTTPServer(('127.0.0.1', 3333), SimpleHTTPRequestHandler) + server = HTTPServer(('localhost', 3333), SimpleHTTPRequestHandler) server.socket = ssl_context.wrap_socket(server.socket, server_side=True) t = threading.Thread(target=server.serve_forever) t.start() @@ -38,7 +38,7 @@ def test_ssl_verification(): # We run everything in a big try-except-finally block to make sure we always # shutdown the HTTP server gracefully. try: - service = BaseService(service_url='https://127.0.0.1:3333', authenticator=NoAuthAuthenticator()) + service = BaseService(service_url='https://localhost:3333', authenticator=NoAuthAuthenticator()) # # First call the server with the default configuration. # It should fail due to the invalid SSL cert. @@ -47,6 +47,10 @@ def test_ssl_verification(): with pytest.raises(SSLError): res = service.send(prepped) + # Next configure it to validate by using our local certificate. Should raise no exception. + res = service.send(prepped, verify=cert) + assert res is not None + # Now disable the SSL verification. The request shouldn't raise any issue. service.set_disable_ssl_verification(True) assert service.disable_ssl_verification is True From ebf7f7bf222dc7e431875d7115ca4cf4bdcc503f Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 11:49:20 +0200 Subject: [PATCH 03/12] test: extend SSL test to reproduce the main issue Signed-off-by: Norbert Biczo --- test_integration/test_ssl_verification.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test_integration/test_ssl_verification.py b/test_integration/test_ssl_verification.py index ce3d344..c1542aa 100644 --- a/test_integration/test_ssl_verification.py +++ b/test_integration/test_ssl_verification.py @@ -1,10 +1,12 @@ # pylint: disable=missing-docstring import os import threading +import warnings from http.server import HTTPServer, SimpleHTTPRequestHandler from ssl import PROTOCOL_TLS_SERVER, SSLContext import pytest +import urllib3 from requests.exceptions import SSLError from ibm_cloud_sdk_core.base_service import BaseService @@ -21,6 +23,9 @@ def test_ssl_verification(): + # Disable warnings caused by the self-signed certificate. + urllib3.disable_warnings() + # Load the certificate and the key files. cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.crt') key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key') @@ -57,7 +62,19 @@ def test_ssl_verification(): prepped = service.prepare_request('GET', url='/') res = service.send(prepped) assert res is not None + + # Lastly, try with an external URL. + # This test case is mainly here to reproduce the regression in the `requests` package that was introduced in `2.32.3`. + # More details on the issue can be found here: https://github.com/psf/requests/issues/6730 + service = BaseService(service_url='https://raw.githubusercontent.com', authenticator=NoAuthAuthenticator()) + assert service.disable_ssl_verification is False + prepped = service.prepare_request('GET', url='/IBM/python-sdk-core/main/README.md') + res = service.send(prepped) + assert res is not None except Exception: # pylint: disable=try-except-raise raise finally: server.shutdown() + # Re-enable warnings. + warnings.resetwarnings() + From 91c241ba0e1fe76be6147dd2db95703910bf6f9c Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 11:49:54 +0200 Subject: [PATCH 04/12] chore: start using the latest version of `requests` again Signed-off-by: Norbert Biczo --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 2dba176..0fd6e2a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -requests>=2.31.0,<2.32.3 +requests>=2.31.0,<3.0.0 urllib3>=2.1.0,<3.0.0 python_dateutil>=2.8.2,<3.0.0 PyJWT>=2.8.0,<3.0.0 From 9530e6c94575de0fc9d919d737823219e0a7c2b7 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 11:57:06 +0200 Subject: [PATCH 05/12] chore: move the `SSLHTTPAdapter` class to a separate file and its test to the `test` folder At this point if `requests 2.32.3` is installed, the issue is reproducable. Signed-off-by: Norbert Biczo --- ibm_cloud_sdk_core/base_service.py | 2 +- ibm_cloud_sdk_core/http_adapter.py | 26 ++++++++++++++++++ ibm_cloud_sdk_core/utils.py | 27 ------------------- .../test_http_adapter.py | 5 ++-- 4 files changed, 30 insertions(+), 30 deletions(-) create mode 100644 ibm_cloud_sdk_core/http_adapter.py rename test_integration/test_ssl_verification.py => test/test_http_adapter.py (96%) diff --git a/ibm_cloud_sdk_core/base_service.py b/ibm_cloud_sdk_core/base_service.py index 196b112..e97fcc1 100644 --- a/ibm_cloud_sdk_core/base_service.py +++ b/ibm_cloud_sdk_core/base_service.py @@ -30,6 +30,7 @@ from ibm_cloud_sdk_core.authenticators import Authenticator from .api_exception import ApiException from .detailed_response import DetailedResponse +from .http_adapter import SSLHTTPAdapter from .token_managers.token_manager import TokenManager from .utils import ( has_bad_first_or_last_char, @@ -38,7 +39,6 @@ cleanup_values, read_external_sources, strip_extra_slashes, - SSLHTTPAdapter, GzipStream, ) from .private_helpers import _build_user_agent diff --git a/ibm_cloud_sdk_core/http_adapter.py b/ibm_cloud_sdk_core/http_adapter.py new file mode 100644 index 0000000..30f73f9 --- /dev/null +++ b/ibm_cloud_sdk_core/http_adapter.py @@ -0,0 +1,26 @@ +import ssl + +from requests.adapters import HTTPAdapter, DEFAULT_POOLBLOCK +from urllib3.util.ssl_ import create_urllib3_context + + +# pylint: disable=fixme +class SSLHTTPAdapter(HTTPAdapter): + """Wraps the original HTTP adapter and adds additional SSL context.""" + + def __init__(self, *args, **kwargs): + self._disable_ssl_verification = kwargs.pop('_disable_ssl_verification', None) + + super().__init__(*args, **kwargs) + + def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool_kwargs): + """Create and use custom SSL configuration.""" + + ssl_context = create_urllib3_context() + ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 + + if self._disable_ssl_verification: + ssl_context.check_hostname = False + ssl_context.verify_mode = ssl.CERT_NONE + + super().init_poolmanager(connections, maxsize, block, ssl_context=ssl_context, **pool_kwargs) diff --git a/ibm_cloud_sdk_core/utils.py b/ibm_cloud_sdk_core/utils.py index d696a94..777d92b 100644 --- a/ibm_cloud_sdk_core/utils.py +++ b/ibm_cloud_sdk_core/utils.py @@ -19,41 +19,14 @@ import io import json as json_import import re -import ssl from os import getenv, environ, getcwd from os.path import isfile, join, expanduser from typing import List, Union from urllib.parse import urlparse, parse_qs -from requests.adapters import HTTPAdapter, DEFAULT_POOLBLOCK -from urllib3.util.ssl_ import create_urllib3_context - import dateutil.parser as date_parser -# pylint: disable=fixme -# TODO: revert the change in the `requirement.txt` once this class become deprecated! -class SSLHTTPAdapter(HTTPAdapter): - """Wraps the original HTTP adapter and adds additional SSL context.""" - - def __init__(self, *args, **kwargs): - self._disable_ssl_verification = kwargs.pop('_disable_ssl_verification', None) - - super().__init__(*args, **kwargs) - - def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool_kwargs): - """Create and use custom SSL configuration.""" - - ssl_context = create_urllib3_context() - ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 - - if self._disable_ssl_verification: - ssl_context.check_hostname = False - ssl_context.verify_mode = ssl.CERT_NONE - - super().init_poolmanager(connections, maxsize, block, ssl_context=ssl_context, **pool_kwargs) - - class GzipStream(io.RawIOBase): """Compress files on the fly. diff --git a/test_integration/test_ssl_verification.py b/test/test_http_adapter.py similarity index 96% rename from test_integration/test_ssl_verification.py rename to test/test_http_adapter.py index c1542aa..aad4453 100644 --- a/test_integration/test_ssl_verification.py +++ b/test/test_http_adapter.py @@ -14,6 +14,7 @@ # The certificate files that are used in this tests are generated by this command: +# pylint: disable=line-too-long,pointless-string-statement """ openssl req -x509 -out test_ssl.crt -keyout test_ssl.key \ -newkey rsa:2048 -nodes -sha256 -days 36500 \ @@ -64,7 +65,8 @@ def test_ssl_verification(): assert res is not None # Lastly, try with an external URL. - # This test case is mainly here to reproduce the regression in the `requests` package that was introduced in `2.32.3`. + # This test case is mainly here to reproduce the regression + # in the `requests` package that was introduced in `2.32.3`. # More details on the issue can be found here: https://github.com/psf/requests/issues/6730 service = BaseService(service_url='https://raw.githubusercontent.com', authenticator=NoAuthAuthenticator()) assert service.disable_ssl_verification is False @@ -77,4 +79,3 @@ def test_ssl_verification(): server.shutdown() # Re-enable warnings. warnings.resetwarnings() - From 6a110fe8ee717ab2c4864e36ee968c5ae081fae9 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 12:04:43 +0200 Subject: [PATCH 06/12] fix: always load the default certs on our custom SSL context Signed-off-by: Norbert Biczo --- ibm_cloud_sdk_core/http_adapter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ibm_cloud_sdk_core/http_adapter.py b/ibm_cloud_sdk_core/http_adapter.py index 30f73f9..11ac825 100644 --- a/ibm_cloud_sdk_core/http_adapter.py +++ b/ibm_cloud_sdk_core/http_adapter.py @@ -17,6 +17,7 @@ def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool """Create and use custom SSL configuration.""" ssl_context = create_urllib3_context() + ssl_context.load_default_certs() ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 if self._disable_ssl_verification: From d772f78558b8167911454ea9532c5b149ac940ae Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 12:26:37 +0200 Subject: [PATCH 07/12] test: make the SSLError check stricter Signed-off-by: Norbert Biczo --- test/test_http_adapter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_http_adapter.py b/test/test_http_adapter.py index aad4453..a45eb9b 100644 --- a/test/test_http_adapter.py +++ b/test/test_http_adapter.py @@ -47,10 +47,10 @@ def test_ssl_verification(): service = BaseService(service_url='https://localhost:3333', authenticator=NoAuthAuthenticator()) # # First call the server with the default configuration. - # It should fail due to the invalid SSL cert. + # It should fail due to the self-signed SSL cert. assert service.disable_ssl_verification is False prepped = service.prepare_request('GET', url='/') - with pytest.raises(SSLError): + with pytest.raises(SSLError, match="certificate verify failed: self-signed certificate"): res = service.send(prepped) # Next configure it to validate by using our local certificate. Should raise no exception. From d975630d88e2868e6c5466fc072f1ae2831d3757 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 2 Jul 2024 14:25:06 +0200 Subject: [PATCH 08/12] test: validate that TLS v1.2 in the minimum required version Signed-off-by: Norbert Biczo --- test/test_http_adapter.py | 138 ++++++++++++++++++++++---------------- 1 file changed, 82 insertions(+), 56 deletions(-) diff --git a/test/test_http_adapter.py b/test/test_http_adapter.py index a45eb9b..041f84f 100644 --- a/test/test_http_adapter.py +++ b/test/test_http_adapter.py @@ -3,7 +3,8 @@ import threading import warnings from http.server import HTTPServer, SimpleHTTPRequestHandler -from ssl import PROTOCOL_TLS_SERVER, SSLContext +from ssl import SSLContext, PROTOCOL_TLSv1_1, PROTOCOL_TLSv1_2 +from typing import Callable import pytest import urllib3 @@ -23,59 +24,84 @@ """ -def test_ssl_verification(): - # Disable warnings caused by the self-signed certificate. - urllib3.disable_warnings() - - # Load the certificate and the key files. - cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.crt') - key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key') - - # Build the SSL context for the server. - ssl_context = SSLContext(PROTOCOL_TLS_SERVER) - ssl_context.load_cert_chain(certfile=cert, keyfile=key) - - # Create and start the server on a separate thread. - server = HTTPServer(('localhost', 3333), SimpleHTTPRequestHandler) - server.socket = ssl_context.wrap_socket(server.socket, server_side=True) - t = threading.Thread(target=server.serve_forever) - t.start() - - # We run everything in a big try-except-finally block to make sure we always - # shutdown the HTTP server gracefully. - try: - service = BaseService(service_url='https://localhost:3333', authenticator=NoAuthAuthenticator()) - # - # First call the server with the default configuration. - # It should fail due to the self-signed SSL cert. - assert service.disable_ssl_verification is False - prepped = service.prepare_request('GET', url='/') - with pytest.raises(SSLError, match="certificate verify failed: self-signed certificate"): - res = service.send(prepped) - - # Next configure it to validate by using our local certificate. Should raise no exception. - res = service.send(prepped, verify=cert) - assert res is not None - - # Now disable the SSL verification. The request shouldn't raise any issue. - service.set_disable_ssl_verification(True) - assert service.disable_ssl_verification is True - prepped = service.prepare_request('GET', url='/') - res = service.send(prepped) - assert res is not None - - # Lastly, try with an external URL. - # This test case is mainly here to reproduce the regression - # in the `requests` package that was introduced in `2.32.3`. - # More details on the issue can be found here: https://github.com/psf/requests/issues/6730 - service = BaseService(service_url='https://raw.githubusercontent.com', authenticator=NoAuthAuthenticator()) - assert service.disable_ssl_verification is False - prepped = service.prepare_request('GET', url='/IBM/python-sdk-core/main/README.md') +# Load the certificate and the key files. +cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.crt') +key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key') + + +def _local_server(tls_version: int, port: int) -> Callable: + def decorator(test_function: Callable) -> Callable: + def inner(): + # Disable warnings caused by the self-signed certificate. + urllib3.disable_warnings() + + # Build the SSL context for the server. + ssl_context = SSLContext(tls_version) + ssl_context.load_cert_chain(certfile=cert, keyfile=key) + + # Create and start the server on a separate thread. + server = HTTPServer(('localhost', port), SimpleHTTPRequestHandler) + server.socket = ssl_context.wrap_socket(server.socket, server_side=True) + t = threading.Thread(target=server.serve_forever) + t.start() + + # We run everything in a big try-except-finally block to make sure we always + # shutdown the HTTP server gracefully. + try: + test_function() + except Exception: # pylint: disable=try-except-raise + raise + finally: + server.shutdown() + t.join() + # Re-enable warnings. + warnings.resetwarnings() + + return inner + + return decorator + + +@_local_server(PROTOCOL_TLSv1_1, 3333) +def test_tls_v1_1(): + service = BaseService(service_url='https://localhost:3333', authenticator=NoAuthAuthenticator()) + prepped = service.prepare_request('GET', url='/') + # The following request should fail, because the server will try + # to use TLS v1.1 but that's not allowed in our client. + with pytest.raises(Exception) as exception: + service.send(prepped, verify=cert) + # Errors can be differ based on the Python version. + assert exception.type is SSLError or exception.type is ConnectionError + + +@_local_server(PROTOCOL_TLSv1_2, 3334) +def test_tls_v1_2(): + service = BaseService(service_url='https://localhost:3334', authenticator=NoAuthAuthenticator()) + + # First call the server with the default configuration. + # It should fail due to the self-signed SSL cert. + assert service.disable_ssl_verification is False + prepped = service.prepare_request('GET', url='/') + with pytest.raises(SSLError, match='certificate verify failed: self-signed certificate'): res = service.send(prepped) - assert res is not None - except Exception: # pylint: disable=try-except-raise - raise - finally: - server.shutdown() - # Re-enable warnings. - warnings.resetwarnings() + + # Next configure it to validate by using our local certificate. Should raise no exception. + res = service.send(prepped, verify=cert) + assert res is not None + + # Now disable the SSL verification. The request shouldn't raise any issue. + service.set_disable_ssl_verification(True) + assert service.disable_ssl_verification is True + prepped = service.prepare_request('GET', url='/') + res = service.send(prepped) + assert res is not None + + # Lastly, try with an external URL. + # This test case is mainly here to reproduce the regression + # in the `requests` package that was introduced in `2.32.3`. + # More details on the issue can be found here: https://github.com/psf/requests/issues/6730 + service = BaseService(service_url='https://raw.githubusercontent.com', authenticator=NoAuthAuthenticator()) + assert service.disable_ssl_verification is False + prepped = service.prepare_request('GET', url='/IBM/python-sdk-core/main/README.md') + res = service.send(prepped) + assert res is not None From c9735f916a5f8f131e2fd8b3073fca6c41ec2126 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Mon, 8 Jul 2024 15:47:34 +0200 Subject: [PATCH 09/12] test: use more realistic url for testing SSL with external site Signed-off-by: Norbert Biczo --- test/test_http_adapter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_http_adapter.py b/test/test_http_adapter.py index 041f84f..2ab766f 100644 --- a/test/test_http_adapter.py +++ b/test/test_http_adapter.py @@ -100,8 +100,8 @@ def test_tls_v1_2(): # This test case is mainly here to reproduce the regression # in the `requests` package that was introduced in `2.32.3`. # More details on the issue can be found here: https://github.com/psf/requests/issues/6730 - service = BaseService(service_url='https://raw.githubusercontent.com', authenticator=NoAuthAuthenticator()) + service = BaseService(service_url='https://cloud.ibm.com', authenticator=NoAuthAuthenticator()) assert service.disable_ssl_verification is False - prepped = service.prepare_request('GET', url='/IBM/python-sdk-core/main/README.md') + prepped = service.prepare_request('GET', url='/status') res = service.send(prepped) assert res is not None From 2aaadab9a9cd8d4e43efebeaee3e5420ea4eb484 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Mon, 8 Jul 2024 15:48:24 +0200 Subject: [PATCH 10/12] test: check the number of the loaded SSL certs Signed-off-by: Norbert Biczo --- test/test_http_adapter.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_http_adapter.py b/test/test_http_adapter.py index 2ab766f..cdf3857 100644 --- a/test/test_http_adapter.py +++ b/test/test_http_adapter.py @@ -102,6 +102,11 @@ def test_tls_v1_2(): # More details on the issue can be found here: https://github.com/psf/requests/issues/6730 service = BaseService(service_url='https://cloud.ibm.com', authenticator=NoAuthAuthenticator()) assert service.disable_ssl_verification is False + + ssl_context = service.http_adapter.poolmanager.connection_pool_kw.get("ssl_context") + assert ssl_context is not None + assert len(ssl_context.get_ca_certs()) > 0 + prepped = service.prepare_request('GET', url='/status') res = service.send(prepped) assert res is not None From 296760a7fa263e8b00aa98c8d755127190422d0d Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 9 Jul 2024 12:03:48 +0200 Subject: [PATCH 11/12] test: fix cert loading in builds Signed-off-by: Norbert Biczo --- test/test_http_adapter.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/test_http_adapter.py b/test/test_http_adapter.py index cdf3857..08f7ec0 100644 --- a/test/test_http_adapter.py +++ b/test/test_http_adapter.py @@ -3,7 +3,7 @@ import threading import warnings from http.server import HTTPServer, SimpleHTTPRequestHandler -from ssl import SSLContext, PROTOCOL_TLSv1_1, PROTOCOL_TLSv1_2 +from ssl import get_default_verify_paths, SSLContext, PROTOCOL_TLSv1_1, PROTOCOL_TLSv1_2 from typing import Callable import pytest @@ -105,6 +105,17 @@ def test_tls_v1_2(): ssl_context = service.http_adapter.poolmanager.connection_pool_kw.get("ssl_context") assert ssl_context is not None + # In some cases (especially in Ubuntu containers that we use for testing on Travis) + # the default CA certifications are stored in a different place, so let's try to + # load those before making the final decision for this test case. + if len(ssl_context.get_ca_certs()) == 0: + try: + default_ca_path = get_default_verify_paths().capath + ssl_context.load_verify_locations(os.path.join(default_ca_path, 'ca-certificates.crt')) + except: + # Errors are ignored, let's jump straight to the assertion. + pass + assert len(ssl_context.get_ca_certs()) > 0 prepped = service.prepare_request('GET', url='/status') From c26344ff90cca13b9c8852df463b6f684ba15fe8 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Tue, 9 Jul 2024 17:20:49 +0200 Subject: [PATCH 12/12] chore: fix typo Signed-off-by: Norbert Biczo --- test/test_http_adapter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_http_adapter.py b/test/test_http_adapter.py index 08f7ec0..0240904 100644 --- a/test/test_http_adapter.py +++ b/test/test_http_adapter.py @@ -106,7 +106,7 @@ def test_tls_v1_2(): ssl_context = service.http_adapter.poolmanager.connection_pool_kw.get("ssl_context") assert ssl_context is not None # In some cases (especially in Ubuntu containers that we use for testing on Travis) - # the default CA certifications are stored in a different place, so let's try to + # the default CA certificates are stored in a different place, so let's try to # load those before making the final decision for this test case. if len(ssl_context.get_ca_certs()) == 0: try: