Re: Adding flags to SChannel cred

2021-02-28 Thread Ray Satiro via curl-library

On 2/28/2021 4:14 PM, Morten Minde Neergaard via curl-library wrote:

At 17:11, Sat 2021-02-27, Ray Satiro via curl-library wrote:

On 2/26/2021 2:56 PM, Morten Minde Neergaard via curl-library wrote:

[...]

The first thing that came to mind would be to add an option
CURLOPT_SSL_BACKEND_FLAGS where each backend could use these flags as
desired. The implementation-specific part of the patch would be like
this for SChannel:

--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -557,6 +557,8 @@ schannel_connect_step1(struct Curl_easy *data, struct 
connectdata *conn,
  "names in server certificates.\n"));
   }
+schannel_cred.dwFlags |= SSL_CONN_CONFIG(backend_flags);
+
   switch(conn->ssl_config.version) {
   case CURL_SSLVERSION_DEFAULT:
   case CURL_SSLVERSION_TLSv1:

[...]

I've proposed two PRs to address the auto credentials issue. One would leave
auto credentials as the default and add an option to disable it [1], and the
other would disable auto credentials as the default (breaking change) and
add an option to enable it [2]. Please take any discussion about it to the
latter PR.

Cool, agree with the change. Since I'm not too familiar with the libcurl
code base, I'd hardly call my looking at the code a review, but gave it
a try nonetheless =)


Regarding strong ciphers, CURLOPT_SSL_CIPHER_LIST [3] (--ciphers for the
curl tool [4]) can be used with Schannel to set some algorithms but unlike
other SSL backends it's relatively limited without ciphersuite support or
umbrella terms like "USE_STRONG_CRYPTO". We would consider a patch for that
to signal strong crypto.

To be clear, you're suggesting this should be possible?

   curl_easy_setopt(curl, CURLOPT_SSL_CIPHER_LIST, "USE_STRONG_CRYPTO");

... and that would also be possible to combine with the current ALGID
stuff? Not that it's a particularly sane use case, but would this be
acceptable?

   curl_easy_setopt(curl, CURLOPT_SSL_CIPHER_LIST,
   
"CALG_RSA_SIGN:CALG_DH_EPHEM:CALG_AES_256:CALG_SHA_384:USE_STRONG_CRYPTO");



Looks fine to me. Older versions should (and already will) error if the 
term is not supported. For example,


> curld --ciphers "USE_STRONG_CRYPTO" https://google.com
curl: (59) Unable to set ciphers to passed via SSL_CONN_CONFIG


---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Re: Adding flags to SChannel cred

2021-02-28 Thread Morten Minde Neergaard via curl-library
At 17:11, Sat 2021-02-27, Ray Satiro via curl-library wrote:
> On 2/26/2021 2:56 PM, Morten Minde Neergaard via curl-library wrote:
[...]
> > The first thing that came to mind would be to add an option
> > CURLOPT_SSL_BACKEND_FLAGS where each backend could use these flags as
> > desired. The implementation-specific part of the patch would be like
> > this for SChannel:
> > 
> > --- a/lib/vtls/schannel.c
> > +++ b/lib/vtls/schannel.c
> > @@ -557,6 +557,8 @@ schannel_connect_step1(struct Curl_easy *data, struct 
> > connectdata *conn,
> >  "names in server certificates.\n"));
> >   }
> > +schannel_cred.dwFlags |= SSL_CONN_CONFIG(backend_flags);
> > +
> >   switch(conn->ssl_config.version) {
> >   case CURL_SSLVERSION_DEFAULT:
> >   case CURL_SSLVERSION_TLSv1:
[...]
> 
> 
> I've proposed two PRs to address the auto credentials issue. One would leave
> auto credentials as the default and add an option to disable it [1], and the
> other would disable auto credentials as the default (breaking change) and
> add an option to enable it [2]. Please take any discussion about it to the
> latter PR.

Cool, agree with the change. Since I'm not too familiar with the libcurl
code base, I'd hardly call my looking at the code a review, but gave it
a try nonetheless =)

> Regarding strong ciphers, CURLOPT_SSL_CIPHER_LIST [3] (--ciphers for the
> curl tool [4]) can be used with Schannel to set some algorithms but unlike
> other SSL backends it's relatively limited without ciphersuite support or
> umbrella terms like "USE_STRONG_CRYPTO". We would consider a patch for that
> to signal strong crypto.

To be clear, you're suggesting this should be possible?

  curl_easy_setopt(curl, CURLOPT_SSL_CIPHER_LIST, "USE_STRONG_CRYPTO");

... and that would also be possible to combine with the current ALGID
stuff? Not that it's a particularly sane use case, but would this be
acceptable?

  curl_easy_setopt(curl, CURLOPT_SSL_CIPHER_LIST,
  
"CALG_RSA_SIGN:CALG_DH_EPHEM:CALG_AES_256:CALG_SHA_384:USE_STRONG_CRYPTO");


Kind regards,
-- 
Morten Minde Neergaard
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Re: Adding flags to SChannel cred

2021-02-27 Thread Ray Satiro via curl-library

On 2/26/2021 2:56 PM, Morten Minde Neergaard via curl-library wrote:

I'm using libcurl in a project I'm doing, and I'd like to specify some
extra flags to the SCHANNEL_CRED struct to enhance security and remove
potential error sources:

  SCH_USE_STRONG_CRYPTO:
Disables some older cipher suites.

  SCH_CRED_NO_DEFAULT_CREDS
Found a TODO about this flag at
https://curl.haxx.se/docs/todo.html#Add_option_to_disable_client_cer

I'm hoping to avoid forking curl to set the flags, and was basically
wondering how it would make sense to implement this.

The first thing that came to mind would be to add an option
CURLOPT_SSL_BACKEND_FLAGS where each backend could use these flags as
desired. The implementation-specific part of the patch would be like
this for SChannel:

--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -557,6 +557,8 @@ schannel_connect_step1(struct Curl_easy *data, struct 
connectdata *conn,
 "names in server certificates.\n"));
  }
  
+schannel_cred.dwFlags |= SSL_CONN_CONFIG(backend_flags);

+
  switch(conn->ssl_config.version) {
  case CURL_SSLVERSION_DEFAULT:
  case CURL_SSLVERSION_TLSv1:


Now, I see that this isn't particularly pretty. Is such a patch likely
to be merged, and if not does anyone have a better way of solving this?



I've proposed two PRs to address the auto credentials issue. One would 
leave auto credentials as the default and add an option to disable it 
[1], and the other would disable auto credentials as the default 
(breaking change) and add an option to enable it [2]. Please take any 
discussion about it to the latter PR.


Regarding strong ciphers, CURLOPT_SSL_CIPHER_LIST [3] (--ciphers for the 
curl tool [4]) can be used with Schannel to set some algorithms but 
unlike other SSL backends it's relatively limited without ciphersuite 
support or umbrella terms like "USE_STRONG_CRYPTO". We would consider a 
patch for that to signal strong crypto.



[1]: https://github.com/curl/curl/pull/6672
[2]: https://github.com/curl/curl/pull/6673
[3]: https://curl.se/libcurl/c/CURLOPT_SSL_CIPHER_LIST.html
[4]: https://curl.se/docs/manpage.html#--ciphers

---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Adding flags to SChannel cred

2021-02-26 Thread Morten Minde Neergaard via curl-library
Hi,

I'm using libcurl in a project I'm doing, and I'd like to specify some
extra flags to the SCHANNEL_CRED struct to enhance security and remove
potential error sources:

 SCH_USE_STRONG_CRYPTO:
Disables some older cipher suites.

 SCH_CRED_NO_DEFAULT_CREDS
Found a TODO about this flag at
https://curl.haxx.se/docs/todo.html#Add_option_to_disable_client_cer

I'm hoping to avoid forking curl to set the flags, and was basically
wondering how it would make sense to implement this.

The first thing that came to mind would be to add an option
CURLOPT_SSL_BACKEND_FLAGS where each backend could use these flags as
desired. The implementation-specific part of the patch would be like
this for SChannel:

--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -557,6 +557,8 @@ schannel_connect_step1(struct Curl_easy *data, struct 
connectdata *conn,
"names in server certificates.\n"));
 }
 
+schannel_cred.dwFlags |= SSL_CONN_CONFIG(backend_flags);
+
 switch(conn->ssl_config.version) {
 case CURL_SSLVERSION_DEFAULT:
 case CURL_SSLVERSION_TLSv1:


Now, I see that this isn't particularly pretty. Is such a patch likely
to be merged, and if not does anyone have a better way of solving this?


Kind regards,
-- 
Morten Minde Neergaard
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html