Skip to content

Commit

Permalink
GH-33: Use strong security level by default
Browse files Browse the repository at this point in the history
  • Loading branch information
@negrutiu
negrutiu committed Aug 31, 2024
1 parent 171f27a commit 4c1a60f
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 16 deletions.
11 changes: 6 additions & 5 deletions src/nscurl/NScurl.readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
```
Expand Down
8 changes: 4 additions & 4 deletions src/nscurl/curl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/nscurl/curl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tests/NScurl-Debug.nsi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
6 changes: 3 additions & 3 deletions tests/NScurl-Test.nsi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down

0 comments on commit 4c1a60f

Please sign in to comment.