Merge pull request #35 from negrutiu/feature/GH-33-strong-security

GH-33: Use `strong` security level by default
negrutiu · Aug 31, 2024 · 978524e · 978524e
2 parents 171f27a + 4c1a60f
commit 978524e
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 16 deletions.
diff --git a/src/nscurl/NScurl.readme.md b/src/nscurl/NScurl.readme.md
@@ -375,6 +375,7 @@ Parameter      | Details
 > [!caution]
 > If all certificate sources are empty (e.g. `/CACERT none /CASTORE false` and no `/CERT` arguments), SSL certificate validation is disabled. `NScurl` would connect to any server, including untrusted ones (aka _insecure transfers_).
 > By default, both the built-in `cacert.pem` and the __native CA store__ are used for validation.
+> See [/SECURITY](#security) for more security options.
 
 ### /CASTORE
 ```nsis
@@ -435,19 +436,19 @@ Pop $0
 
 ### /SECURITY
 ```
-/SECURITY weak|strong
+/SECURITY strong|weak
 ```
 Configure the security level for the current transfer.  
-The default security is `weak` to favor compatibility with legacy servers.
+The default security is `strong`.
+
+Security level `strong`:
+- use the default `openssl` crypto algorithms and standards that are considered secure
 
 Security level `weak`:
 - call [SSL_CTX_set_options(..., SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);](https://docs.openssl.org/3.1/man3/SSL_CTX_set_options/#notes) to enable unsafe legacy renegociation
 - call [SSL_CTX_set_security_level( 0 )](https://docs.openssl.org/1.1.1/man3/SSL_CTX_set_security_level/#default-callback-behaviour) to enable weak cryptographic algorithms
 - call [curl_easy_setopt(..., CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1);](https://curl.se/libcurl/c/CURLOPT_SSLVERSION.html) to enable `SSL3`, `TLS 1.0` and `TLS 1.1` protocols
 
-Security level `strong`:
-- use the default `openssl` crypto algorithms and standards that are considered secure
-
 
 ### /DEPEND
 ```

diff --git a/src/nscurl/curl.c b/src/nscurl/curl.c
@@ -459,9 +459,9 @@ ULONG CurlParseRequestParam( _In_ ULONG iParamIndex, _In_ LPTSTR pszParam, _In_
 	} else if (lstrcmpi( pszParam, _T( "/SECURITY" ) ) == 0) {
 		if (popstring( pszParam ) == NOERROR) {
 			if (lstrcmpi(pszParam, _T("weak")) == 0) {
-				pReq->bStrongSecurity = FALSE;
+				pReq->bWeakSecurity = TRUE;
 			} else if (lstrcmpi(pszParam, _T("strong")) == 0) {
-				pReq->bStrongSecurity = TRUE;
+				pReq->bWeakSecurity = FALSE;
 			} else {
 				err = ERROR_INVALID_PARAMETER;
 			}
@@ -650,7 +650,7 @@ CURLcode CurlSSLCallback( CURL *curl, void *ssl_ctx, void * userdata)
 	}
 
 	// https://docs.openssl.org/1.1.1/man3/SSL_CTX_set_security_level/#default-callback-behaviour
-	if (!req->bStrongSecurity)
+	if (req->bWeakSecurity)
 	{
 		SSL_CTX_set_security_level(sslctx, 0);
 	}
@@ -1157,7 +1157,7 @@ void CurlTransfer( _In_ PCURL_REQUEST pReq )
 			}
 
 			// Security level
-			if (!pReq->bStrongSecurity)
+			if (pReq->bWeakSecurity)
 			{
 				// GH-31: allow "unsafe legacy renegotiation"
 				// Symptomatic URL: https://publicinfobanjir.water.gov.my/hujan/data-hujan/?state=PLS&lang=en

diff --git a/src/nscurl/curl.h b/src/nscurl/curl.h
@@ -40,7 +40,7 @@ typedef struct _CURL_REQUEST {
 	BOOLEAN		bMarkOfTheWeb : 1;
 	BOOLEAN     bHttp11       : 1;
 	BOOLEAN     bEncoding     : 1;
-	BOOLEAN     bStrongSecurity : 1;	/// Use default openssl security level. By default, we use SSL_CTX_set_security_level(0) and SSL3 as minimum protocol
+	BOOLEAN     bWeakSecurity : 1;		/// weak crypto, weak protocols (tls 1.0+), unsafe renegociation
 	BOOLEAN     bCastore      : 1;      /// Use native CA store (CURLSSLOPT_NATIVE_CA)
 	LPCSTR		pszCacert;				/// can be CACERT_BUILTIN(NULL), CACERT_NONE, or a file path
 	struct curl_slist *pCertList;		/// List of sha1 certificate thumprints. can be NULL

diff --git a/tests/NScurl-Debug.nsi b/tests/NScurl-Debug.nsi
@@ -916,7 +916,7 @@ Section "Unsafe legacy renegociation"
 
     !define /redef CURLE_SSL_CONNECT_ERROR 35
 
-    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       http 200
+    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       curl ${CURLE_SSL_CONNECT_ERROR} ; 'strong' by default
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'weak'   http 200
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'strong' curl ${CURLE_SSL_CONNECT_ERROR} ; OpenSSL/3.3.1: error:0A000152:SSL routines::unsafe legacy renegotiation disabled
 
@@ -935,15 +935,15 @@ Section "Weak protocols"
 	!define /redef LINK 'https://tls-v1-0.badssl.com:1010/'
 	!define /redef FILE '$EXEDIR\_test_weaktls10'
 
-    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       http 200
+    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       curl ${CURLE_SSL_CONNECT_ERROR}    ; 'strong' by default
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'weak'   http 200
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'strong' curl ${CURLE_SSL_CONNECT_ERROR}    ; OpenSSL/3.3.1: error:0A000102:SSL routines::unsupported protocol
 
 
 	!define /redef LINK 'https://tls-v1-1.badssl.com:1011/'
 	!define /redef FILE '$EXEDIR\_test_weaktls11'
 
-    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       http 200
+    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       curl ${CURLE_SSL_CONNECT_ERROR}    ; 'strong' by default
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'weak'   http 200
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'strong' curl ${CURLE_SSL_CONNECT_ERROR}    ; OpenSSL/3.3.1: error:0A000102:SSL routines::unsupported protocol
 

diff --git a/tests/NScurl-Test.nsi b/tests/NScurl-Test.nsi
@@ -629,7 +629,7 @@ Section "Unsafe legacy renegociation"
 
     !define /redef CURLE_SSL_CONNECT_ERROR 35
 
-    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       http 200
+    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       curl ${CURLE_SSL_CONNECT_ERROR} ; 'strong' by default
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'weak'   http 200
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'strong' curl ${CURLE_SSL_CONNECT_ERROR} ; OpenSSL/3.3.1: error:0A000152:SSL routines::unsafe legacy renegotiation disabled
 
@@ -645,15 +645,15 @@ Section "Weak protocols"
 	!define /redef LINK 'https://tls-v1-0.badssl.com:1010/'
 	!define /redef FILE '$EXEDIR\_test_weaktls10'
 
-    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       http 200
+    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       curl ${CURLE_SSL_CONNECT_ERROR}    ; 'strong' by default
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'weak'   http 200
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'strong' curl ${CURLE_SSL_CONNECT_ERROR}    ; OpenSSL/3.3.1: error:0A000102:SSL routines::unsupported protocol
 
 
 	!define /redef LINK 'https://tls-v1-1.badssl.com:1011/'
 	!define /redef FILE '$EXEDIR\_test_weaktls11'
 
-    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       http 200
+    !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' ''       curl ${CURLE_SSL_CONNECT_ERROR}    ; 'strong' by default
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'weak'   http 200
     !insertmacro TRANSFER_TEST '${LINK}' '${FILE}' '' '' '' 'strong' curl ${CURLE_SSL_CONNECT_ERROR}    ; OpenSSL/3.3.1: error:0A000102:SSL routines::unsupported protocol