RE: [PATCH] Proxy: support configuration of socket buffer sizes

2017-12-12 Thread Karstens, Nate
Maxim,

I wanted to follow up on this because our software has been released and so I 
can provide a few more details about our setup.

Our use case involves the user transferring a large amount of data from a 
mobile device (running iOS or Android) through the HTTP proxy and storing it 
onto an SD card (the SD card is provided by the user). If the SD card's write 
speed is too slow, then the proxy send buffer will eventually fill up, causing 
a long delay between when the HTTP client sends packets, which ultimately leads 
to the client timing out.

Because the request contains a large amount of data, "proxy_request_buffering" 
is set to "off".

We looked into adjusting the behavior of the client, but this was not possible. 
To explain, there are generally two ways a mobile app can use HTTP to transfer 
data. The most straightforward way is to actively transfer the data using 
either an HTTP client integrated into the app or the HTTP libraries provided by 
the platform. However, for large transfers this requires the user to keep the 
mobile app in the foreground -- if the mobile OS detects that the user is not 
actively using the app then it will suspend the app, which interrupts the 
transfer.

The alternative method to transfer data is to utilize the HTTP background 
transfer functionality provided by the mobile OS. With this method, the app 
configures the HTTP transfer and then asks the mobile OS to complete it on the 
app's behalf. The advantage here is that the user can use other apps while the 
mobile OS completes the transfer. Unfortunately, the mobile OS provides no 
ability to customize connection parameters, such as timeouts.

I read through the Cloudflare blog entry (thank you -- that was interesting). 
Their case is different because they were having problems with a download, 
while we are having problems with an upload. As such, perhaps the 
"proxy_send_timeout" setting applies in this case instead of the "send_timeout" 
that they were using. Still, it seems undesirable to increase the timeout value 
to be too large because it increases the time before you can detect a problem 
with the connection. The Cloudflare team also considered reducing the TCP 
buffer size, but ultimately decided on a different solution. I noticed that 
their solution has not been incorporated into the main distribution, do you 
know why that is?

For us, the option to configure the TCP buffer sizes seems to be the most 
straightforward.

In our system, the default TCP send buffer size is 16kB. However, this can grow 
to a maximum of 4MB (the Linux stack adjusts this in tcp_sndbuf_expand() -- see 
tcp_input.c). We do not think that reducing the system-wide maximum TCP buffer 
size is ideal in this case because the server provides many other TCP 
applications that work well with the default value; adjusting that value 
presents additional risk and may lead to more widespread application-specific 
tuning.

Thanks for your consideration,

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Tuesday, May 23, 2017 3:25 PM
To: nginx-devel@nginx.org
Subject: Re: [PATCH] Proxy: support configuration of socket buffer sizes

Hello!

On Mon, May 22, 2017 at 07:02:04PM +, Karstens, Nate wrote:

> Maxim,
>
> I'd be happy to explain. Our application is actually relying more on
> the change to support to SO_SNDBUF option, but I noticed the SO_RCVBUF
> option there and thought it was worth exposing that at the same time.
>
> The application we're having issues with is essentially a file
> transfer through a proxy to a relatively slow storage medium.
> Because we're using a proxy there are three buffers involved on the
> receive end of the HTTP request: 1) receive buffer on external nginx
> socket, 2) send buffer in nginx proxy module, and
> 3) receive buffer on proxied server. So, in a system where each buffer
> is a maximum of 5 MB, you can have 3 x 5 = 15 MB of data in the TCP
> buffers at a given point in time.

Send buffer on the client contributes to overral buffering as well, and 
probably should be counted too.  But, frankly, 5MB is a lot, and much higher 
than typical default, and may result in problems by itself.  See 
https://blog.cloudflare.com/the-curious-case-of-slow-downloads/
for a description of a problem Cloudflare faced with such socket buffer sizes.

> In most circumstances, I don't think this would be a problem.
> However, writing the data is so slow that the HTTP client times out
> waiting for a reply (which is only sent once all data has been written
> out). Unfortunately, we cannot solve this by increasing the client's
> timeout. We found that reducing the size

Are you using some custom client, or a browser?

Even with buffers as huge as 3x5MB, a really slow backend will be required to 
trigger a timeout in a typical browser, as browsers

RE: [PATCH] [PATCH 4 of 4] SSL: add identity hint config directive

2017-09-06 Thread Karstens, Nate
Yes, I see what you were saying now, sorry for the confusion. Your updates look 
good.

How do you plan to get additional feedback? Maybe at nginx.conf? Our particular 
use case is more of an embedded environment, so our security constraints are 
different from a high traffic web server. Perhaps we can extrapolate the best 
approach for that environment given what we know?

As I see it, we have a few alternatives:

1) Read the file from the PSK worker process (as is being done in the current 
patch)
2) Read the file when the configuration is loaded and cache results in memory 
(file only needs to be read by master process)
3) Allow the user to choose between 1 and 2 (using a config file setting)
4) Adding encryption to PSK file (encryption key is loaded when configuration 
is loaded and PSK file is decrypted by worker process)

Can you think of any others?

The main disadvantage of the second option is that you have to reload the 
configuration file whenever a PSK is added/removed/changed, right? Can you help 
me understand the performance implications of reloading the configuration file? 
If severe, maybe those performance implications could be mitigated by 
functionality specific to the environment the server is operating in? For 
example, in an environment with thousands of users maybe the administrators 
could institute a policy of updating the PSK file and reloading the 
configuration only once every 5 minutes? Other, less complicated environments 
could operate without any additional functionality.

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Monday, September 04, 2017 11:01 AM
To: nginx-devel@nginx.org
Subject: Re: [PATCH] [PATCH 4 of 4] SSL: add identity hint config directive

Hello!

On Fri, Sep 01, 2017 at 01:18:56PM +, Karstens, Nate wrote:

> Maxim,
>
> Your changes look good and test well -- works for me!
>
> Thanks for your work on this, and for your patience!

Thank you for your work, too.  Some additionall problems I'v found looking into 
this:

1. In the previous review,
http://mailman.nginx.org/pipermail/nginx-devel/2017-August/010419.html,
I've asked to not set callback if file is not empty:

: Always configuring a callback is probably not a good idea.
: Instead, there should be a check if file is not empty.
:
: In particular, this will result in cryptic messages like:
:
: ... [error] ... open() "" failed (SSL:) (2: No such file or directory) while 
SSL handshaking
:
: on servers without PSK file configured if a client tries to
: use PSK.

I think you've probably get me wrong, and instead tried to check the file 
contents.  This is not what I mean, I've asked to check the "file" string to 
see if it is empty, and do nothing if it is - that is, if there is no PSK file 
specified in the configuration.

There should be something like this in the code, similar to
ngx_ssl_dhparam():

@@ -1181,6 +1181,10 @@ ngx_ssl_psk_file(ngx_conf_t *cf, ngx_ssl  {  #ifdef 
PSK_MAX_IDENTITY_LEN

+if (file->len == 0) {
+return NGX_OK;
+}
+
 if (ngx_conf_full_name(cf->cycle, file, 1) != NGX_OK) {
 return NGX_ERROR;
 }

This should prevent configurations without ssl_psk_file configured from trying 
to use PSK callback and reading the configuration directory in an attempt to 
find a secret as the code without the check does.

2. Error logging in the ngx_ssl_psk_callback() function is not correct.  It 
uses ngx_ssl_error() function, which is intended to log OpenSSL-specific errors 
with error details extracted from the OpenSSL error stack.  This function uses 
normal file operations though, and the OpenSSL error stack is always empty.  As 
such, all messages will contain meaningless "(SSL:)" part.  To avoid this, 
errors should be logged using normal ngx_log_error().

Updated patches below.

Note well that I'm still not sure if it is good idea to read ssl_psk_file from 
worker processes instead of reading it once on startup as we do with SSL 
certificate keys (or, for example, ticket keys).  This and the fact that PSK 
keys are not protected anyhow, neither with additional password nor hashing, 
implies that PSK keys can be easily compromissed if an attacker is able to read 
files accessible to nginx user.

I'm going to postpone further work on this till at least some feedback from 
people using it in the real world.

# HG changeset patch
# User Nate Karstens <nate.karst...@garmin.com> # Date 1503540018 18000
#  Wed Aug 23 21:00:18 2017 -0500
# Node ID a87e224e8d6b2993dfcd8903bfb0e7eb7fd934fa
# Parent  c7d4017c8876af6d8570e400320537d7d39e9578
Core: add function to decode hexadecimal strings.

Adds functionality to convert a hexadecimal string into binary data.
This will be used to decode PSKs stored in hexadecimal representation.

Signed-off-by: Nate Karstens <nate.karst...@garmin.com>

diff --git a/src/core/ngx_string.c b/s

RE: [PATCH] [PATCH 4 of 4] SSL: add identity hint config directive

2017-09-01 Thread Karstens, Nate
Maxim,

Your changes look good and test well -- works for me!

Thanks for your work on this, and for your patience!

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Thursday, August 31, 2017 9:45 AM
To: nginx-devel@nginx.org
Subject: Re: [PATCH] [PATCH 4 of 4] SSL: add identity hint config directive

Hello!

On Wed, Aug 23, 2017 at 09:22:43PM -0500, Nate Karstens wrote:

> # HG changeset patch
> # User Nate Karstens  # Date 1503540237
> 18000
> #  Wed Aug 23 21:03:57 2017 -0500
> # Node ID 62b4032371bd45217d40e2f0daf8ecd6956601d8
> # Parent  a11e114a2bcde4afb515dd0b70f3ef39693f475a
> [PATCH 4 of 4] SSL: add identity hint config directive.

As in previous patches, there should be no "[PATCH 4 of 4] ".
Otherwise loos good.

Following this and previous patches review, here are all four patches adjusted 
according to the comments.  Please take a look if it works for you.

# HG changeset patch
# User Nate Karstens  # Date 1503540018 18000
#  Wed Aug 23 21:00:18 2017 -0500
# Node ID a87e224e8d6b2993dfcd8903bfb0e7eb7fd934fa
# Parent  c7d4017c8876af6d8570e400320537d7d39e9578
Core: add function to decode hexadecimal strings.

Adds functionality to convert a hexadecimal string into binary data.
This will be used to decode PSKs stored in hexadecimal representation.

Signed-off-by: Nate Karstens 

diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c
--- a/src/core/ngx_string.c
+++ b/src/core/ngx_string.c
@@ -1118,6 +1118,56 @@ ngx_hex_dump(u_char *dst, u_char *src, s  }


+ngx_int_t
+ngx_hex_decode(u_char *dst, u_char *src, size_t len) {
+u_char  ch, decoded;
+
+if (len & 1) {
+return NGX_ERROR;
+}
+
+while (len) {
+ch = *src++;
+len -= 2;
+
+if (ch >= '0' && ch <= '9') {
+decoded = ch - '0';
+goto second;
+}
+
+ch |= 0x20;
+
+if (ch >= 'a' && ch <= 'f') {
+decoded = ch - 'a' + 10;
+goto second;
+}
+
+return NGX_ERROR;
+
+second:
+
+ch = *src++;
+
+if (ch >= '0' && ch <= '9') {
+*dst++ = (u_char) ((decoded << 4) + ch - '0');
+continue;
+}
+
+ch |= 0x20;
+
+if (ch >= 'a' && ch <= 'f') {
+*dst++ = (u_char) ((decoded << 4) + ch - 'a' + 10);
+continue;
+}
+
+return NGX_ERROR;
+}
+
+return NGX_OK;
+}
+
+
 void
 ngx_encode_base64(ngx_str_t *dst, ngx_str_t *src)  { diff --git 
a/src/core/ngx_string.h b/src/core/ngx_string.h
--- a/src/core/ngx_string.h
+++ b/src/core/ngx_string.h
@@ -177,6 +177,7 @@ time_t ngx_atotm(u_char *line, size_t n)  ngx_int_t 
ngx_hextoi(u_char *line, size_t n);

 u_char *ngx_hex_dump(u_char *dst, u_char *src, size_t len);
+ngx_int_t ngx_hex_decode(u_char *dst, u_char *src, size_t len);


 #define ngx_base64_encoded_length(len)  (((len + 2) / 3) * 4) # HG changeset 
patch # User Nate Karstens  # Date 1503540059 18000
#  Wed Aug 23 21:00:59 2017 -0500
# Node ID d89f77108fa8a20bc1fb9cdbaf43fefbc5e07119
# Parent  a87e224e8d6b2993dfcd8903bfb0e7eb7fd934fa
SSL: add support for PSK cipher suites.

Adds support for TLS connections using PSK cipher suites. A new configuration 
directive, ssl_psk_file, specifies the file that contains a list of identities 
and associated PSKs. Each line of the file begins with the identity, followed 
by a colon character (':'), and ending with the PSK. As required by RFC 4279 
section 5.4, PSKs may be entered either as plain text or using hexadecimal 
encoding. Hexadecimal PSKs must begin with "{HEX}". PSKs without this prefix 
are assumed to be plain text, but they may optionally begin with "{PLAIN}" to 
denote this. Some examples:

gary:plain_text_password
min:{PLAIN}another_text_password
cliff:{HEX}ab0123CD

PSK functionality can be easily tested with the OpenSSL s_client using the 
"-psk" and "-psk_identity" options.

Signed-off-by: Nate Karstens 

diff --git a/contrib/vim/syntax/nginx.vim b/contrib/vim/syntax/nginx.vim
--- a/contrib/vim/syntax/nginx.vim
+++ b/contrib/vim/syntax/nginx.vim
@@ -550,6 +550,7 @@ syn keyword ngxDirective contained ssl_p  syn keyword 
ngxDirective contained ssl_prefer_server_ciphers  syn keyword ngxDirective 
contained ssl_preread  syn keyword ngxDirective contained ssl_protocols
+syn keyword ngxDirective contained ssl_psk_file
 syn keyword ngxDirective contained ssl_session_cache  syn keyword ngxDirective 
contained ssl_session_ticket_key  syn keyword ngxDirective contained 
ssl_session_tickets diff --git a/src/event/ngx_event_openssl.c 
b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -11,6 +11,7 @@


 #define NGX_SSL_PASSWORD_BUFFER_SIZE  4096
+#define NGX_SSL_PSK_BUFFER_SIZE   4096


 typedef struct {
@@ -24,6 

RE: [PATCH] [PATCH 2 of 4] SSL: add support for PSK cipher suites

2017-09-01 Thread Karstens, Nate
> In the previous review I've pointed out that the patch needs to adjust 
> severity levels in ngx_ssl_connection_error():

Sorry, I think I got confused with something else.

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Thursday, August 31, 2017 9:44 AM
To: nginx-devel@nginx.org
Subject: Re: [PATCH] [PATCH 2 of 4] SSL: add support for PSK cipher suites

Hello!

On Wed, Aug 23, 2017 at 09:20:50PM -0500, Nate Karstens wrote:

> # HG changeset patch
> # User Nate Karstens  # Date 1503540059
> 18000
> #  Wed Aug 23 21:00:59 2017 -0500
> # Node ID 97953fe374455a04973268c4b2fbadd7ced91ffe
> # Parent  17c038b56051f922ec440d40e23e8d1b23d835df
> [PATCH 2 of 4] SSL: add support for PSK cipher suites.

There is no need to include "[PATCH 2 of 4] " into patch summary line.

[...]

> @@ -1163,6 +1177,211 @@ ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_s
>
>
>  ngx_int_t
> +ngx_ssl_psk_file(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file) {
> +#ifdef PSK_MAX_IDENTITY_LEN
> +
> +ngx_fd_tfd;
> +ngx_int_t   err;
> +ngx_file_info_t fi;

Style: there should be two spaces between the longest type and a variable.  
I've already explained details in one of the previous reviews here:

http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010247.html

> +
> +err = NGX_OK;
> +
> +if (ngx_conf_full_name(cf->cycle, file, 1) != NGX_OK) {
> +return NGX_ERROR;
> +}
> +
> +fd = ngx_open_file(file->data, NGX_FILE_RDONLY, NGX_FILE_OPEN, 0);
> +if (fd == NGX_INVALID_FILE) {
> +ngx_ssl_error(NGX_LOG_EMERG, ssl->log, ngx_errno,
> +  ngx_open_file_n " \"%V\" failed", file);
> +return NGX_ERROR;
> +}
> +
> +if (ngx_fd_info(fd, ) == NGX_FILE_ERROR) {
> +ngx_ssl_error(NGX_LOG_EMERG, ssl->log, ngx_errno,
> +  ngx_fd_info_n " \"%V\" failed", file);
> +err = NGX_ERROR;
> +goto failed;
> +}
> +
> +if (ngx_file_size() == 0) {
> +ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> +  "PSK file \"%V\" is empty", file);
> +err = NGX_ERROR;
> +goto failed;
> +}

What's the point in opening the file here?  As long the file is being read at 
run-time, it doesn't seem to make sense.  The file can be legitimately created 
later.

Checking ngx_file_size() looks completely wrong as well.  It will prevent 
configurations with no currently configured PSK keys from running if the file 
size is 0 - but won't do this as long as there is a comment and/or some garbage 
in the file.

> +
> +failed:
> +
> +if (ngx_close_file(fd) == NGX_FILE_ERROR) {
> +ngx_ssl_error(NGX_LOG_EMERG, ssl->log, ngx_errno,
> +  ngx_close_file_n " %V failed", file);
> +err = NGX_ERROR;
> +}
> +
> +if (err != NGX_OK) {
> +return err;
> +}
> +
> +if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_psk_index, file) == 0) {
> +ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> +  "SSL_CTX_set_ex_data() failed");
> +return NGX_ERROR;
> +}
> +
> +SSL_CTX_set_psk_server_callback(ssl->ctx, ngx_ssl_psk_callback);
> +
> +#endif
> +
> +return NGX_OK;
> +}
> +
> +
> +#ifdef PSK_MAX_IDENTITY_LEN
> +
> +static unsigned int
> +ngx_ssl_psk_callback(ngx_ssl_conn_t *ssl_conn, const char *identity,
> +unsigned char *psk, unsigned int max_psk_len) {
> +u_char *p, *last, *end, *colon;
> +size_t  len;
> +ssize_t n;
> +SSL_CTX*ssl_ctx;
> +ngx_fd_tfd;
> +ngx_str_t  *file;
> +unsigned intpsk_len;
> +ngx_connection_t   *c;
> +u_char  buf[NGX_SSL_PSK_BUFFER_SIZE];

Style: there should be two spaces between "ngx_connection_t" and "*c".

> +
> +psk_len = 0;

This looks to early for the psk_len initialization.  Rather, I would move the 
initialization to somewhere near "len = 0; last = buf" below where it will look 
more logical.

> +c = ngx_ssl_get_connection(ssl_conn);
> +
> +ssl_ctx = SSL_get_SSL_CTX(ssl_conn);
> +file = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_psk_index);
> +
> +fd = ngx_open_file(file->data, NGX_FILE_RDONLY, NGX_FILE_OPEN, 0);
> +if (fd == NGX_INVALID_FILE) {
> +ngx_ssl_error(NGX_LOG_ERR, c->log, ngx_errno,
> +  ngx_open_file_n " \"%V\" failed", file);
> +return 0;
> +}
> +
> +len = 0;
> +last = buf;
> +
> +do {
> +n = ngx_read_fd(fd, last, NGX_SSL_PSK_BUFFER_SIZE - len);
> +
> +if (n == -1) {
> +ngx_ssl_error(NGX_LOG_ERR, c->log, ngx_errno,
> +  ngx_read_fd_n " \"%V\" failed", file);
> +psk_len = 0;

There is no need to set psk_len to 0 here, as it is already initialized to 0 
and never set to anything else except before "goto cleanup".

> + 

RE: [PATCH 1 of 3] PSK: connection support

2017-07-25 Thread Karstens, Nate
Maxim,

I'm starting to get back to this now and wanted to follow up on your comments 
in http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010247.html to see 
if you had a preference on when the PSKs should be read from the file. I 
believe we have two options:

1) At startup into a hash.
Advantages: Faster lookups, PSK file can be read-only by root, can warn on 
syntax errors
Disadvantages: Must reload to add/remove PSKs, more config options (for the 
hash max size and bucket size)

2) As needed. This is what was done in the last patch, but changed to no longer 
check for syntax errors at startup.
Advantages: Do not need to reload to add/remove users, can control access using 
groups
Disadvantages: Slower lookups (linear, but this can be improved using a RAM 
disk)

Thanks,

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Friday, June 30, 2017 6:58 AM
To: nginx-devel@nginx.org
Subject: Re: [PATCH 1 of 3] PSK: connection support

Hello!

On Thu, Jun 29, 2017 at 10:00:45PM +, Karstens, Nate wrote:

> Thanks for the comments. I'll try to start on those in a couple of
> days.

Just to make it clear: there is no need to hurry.  Likely I won't be able to 
review new patches in at least a couple of weeks, so feel free to spend more 
time polishing the patches.

> My company uses Outlook/Exchange for email, so I don't think I'll be
> able to use hg email, do you have any other suggestions? Thanks also
> for your patience, I've used Git quite a bit but am new to Mercurial.

The "hg email" command can work with any SMTP server, including Exchange.  Or 
you can ensure proper threading manually by using a "reply" function.

> Utkarsh sounds like he is trying to use PSK for TLS v1.3 session
> resumption. Given that each TLS connection could potentially result in
> a new PSK I think only reading them at startup could result in too
> many refreshes. I think there might be some benefit to the original
> approach in regards to storing each PSK in its own file in a
> designated directory. Benefits include:

TLS v1.3 session resumption uses PSK internally, but it is very different from 
internal usage point of view.  It is handled well enough with existing session 
cache / session tickets mechanisms.

[...]

> > +ngx_int_t
> > +ngx_ssl_psk_file(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file)
> > +
> > +{
>
> Style: extra empty line.
>
> > +ngx_int_t   rc;
> > +
> > +if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_psk_index, file) == 0) {
> > +ngx_ssl_error(NGX_LOG_ALERT, ssl->log, 0,
> > +  "SSL_CTX_set_ex_data() failed");
> > +return NGX_ERROR;
> > +}
> > +
> > +rc = ngx_ssl_psk_read(file, NULL, NULL, 0);
> > +
> > +return rc == 0 ? NGX_OK : NGX_ERROR; }

[...]

> > @@ -800,6 +810,12 @@
> >
> >  }
> >
> > +if (ngx_ssl_psk_file(cf, >ssl, >psk_file)
> > +!= NGX_OK)
> > +{
> > +return NGX_CONF_ERROR;
> > +}

Note: this calls ngx_ssl_psk_file() unconditionally, and
ngx_ssl_psk_file() also doesn't check if a file is configured.  As a result, a 
configuration without ssl_psk_file fails.

--
Maxim Dounin
http://nginx.org/
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel



CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


RE: [PATCH 1 of 3] PSK: connection support

2017-06-29 Thread Karstens, Nate
Maxim,

Thanks for the comments. I'll try to start on those in a couple of days. My 
company uses Outlook/Exchange for email, so I don't think I'll be able to use 
hg email, do you have any other suggestions? Thanks also for your patience, 
I've used Git quite a bit but am new to Mercurial.

Utkarsh sounds like he is trying to use PSK for TLS v1.3 session resumption. 
Given that each TLS connection could potentially result in a new PSK I think 
only reading them at startup could result in too many refreshes. I think there 
might be some benefit to the original approach in regards to storing each PSK 
in its own file in a designated directory. Benefits include:

1) We can utilize the file system to avoid name collisions. With TLS v1.3 
session resumption the PSK identity doesn't have to be user-friendly, so 
something like mkstemp() could be used to generate the identity/filename.
2) Files include metadata about when they were created, so you could have a 
cron job that periodically cleans up old entries. Without this you have to 
maintain creation time in a separate database.
3) No need to worry about hex vs. text -- the contents of the file are the PSK 
and can be plain text or binary data as appropriate.

Utkarsh -- do you have any thoughts on the preferred way to set up the PSKs?

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Thursday, June 29, 2017 4:09 PM
To: nginx-devel@nginx.org
Subject: Re: [PATCH 1 of 3] PSK: connection support

Hello!

On Thu, Jun 22, 2017 at 01:24:54PM +, Karstens, Nate wrote:

> # HG changeset patch
> # User Nate Karstens <nate.karst...@garmin.com> # Date 1498137180 
> 18000
> #  Thu Jun 22 08:13:00 2017 -0500
> # Node ID 3fb3c4928d06029ca1d57853a163c9f56fa90bca
> # Parent  6169dbad37d85fa8642b9d0a51f0f0f6c19dd3d1
> PSK: connection support

Please use "SSL: " prefix for all SSL-related commits.  Please use dot at the 
end of summary.  Overral, this should look like:

SSL: PSK cipher suites support.

Some additional comments mostly unrelated to the code:

- Please add something like

  [diff]
  showfunc=1

  to your Mercurial's ~/.hgrc to simplify further review.  This will
  ensure that function names are directly visible in patches.

- When submitting a patch series, please do so in a single thread
  using In-Reply-To / References.  This will happen automatically
  when using "hg email".  Please also provide references to the
  previous threads, if any, or use "hg email --in-reply-to
  " to submit a new / updated series to the same
  thread.

> Adds support for TLS connections using PSK cipher suites. A new 
> configuration directive, ssl_psk_file, specifies the file that 
> contains a list of identities and associated PSKs. Each line of the 
> file begins with the identity, followed by a colon character (':'), 
> and ending with the PSK. As required by RFC 4279 section 5.4, PSKs may 
> be entered either as plain text or using hexadecimal encoding. 
> Hexadecimal PSKs must begin with "{HEX}". PSKs without this prefix are 
> assumed to be plain text, but they may optionally begin with "{PLAIN}" 
> to denote this. Some examples:
> 
> gary:plain_text_password
> min:{PLAIN}another_text_password
> cliff:{HEX}ab0123CD
> 
> The format of the given PSK file is checked at server startup.

This checking at server startup looks wrong.  As long as the file can be 
modified at any time, it doesn't provide anything but an additional maintanance 
problems - as syntax errors can be introduced (and fixed) at any time.

Note well that there is another problem with runtime-modifiable secrets file 
though.  Being able to access the file at runtime means that worker process 
have to able to read it, so the file can't be only readable by root.  Given 
that secrets are not encrypted (in contrast to auth basic passwords, which are 
hashed and generally non-recoverable even if leaked), this might not be secure 
enough.  It might be actually be a better idea to read all the keys at start as 
you initially suggested.  Not sure though.

> 
> PSK functionality can be easily tested with the OpenSSL s_client using 
> the "-psk" and "-psk_identity" options.
> 
> Signed-off-by: Nate Karstens <nate.karst...@garmin.com>
> 
> diff -r 6169dbad37d8 -r 3fb3c4928d06 contrib/vim/syntax/nginx.vim
> --- a/contrib/vim/syntax/nginx.vim  Wed Jun 14 20:13:41 2017 +0300
> +++ b/contrib/vim/syntax/nginx.vim  Thu Jun 22 08:13:00 2017 -0500
> @@ -550,6 +550,7 @@
>  syn keyword ngxDirective contained ssl_prefer_server_ciphers  syn 
> keyword ngxDirective contained ssl_preread  syn keyword ngxDirective 
> contained ssl_protocols
> +syn keyword ngxDirective contained ssl_psk_file
>  syn keyword ngxDirective contained ssl_session_cache  syn keyw

RE: Testing TLSv1.3 session resumption in Nginx using PSK

2017-06-28 Thread Karstens, Nate
Hello,

I submitted some patches a few days ago that implement PSK support in nginx, 
maybe you could take a look and comment if these will meet your requirements 
(or at least serve as a starting point)? Please see:


  *   http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010203.html
  *   http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010202.html
  *   http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010204.html

Thanks,

Nate Karstens
Garmin International, Inc.

From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Utkarsh 
Tewari
Sent: Wednesday, June 28, 2017 5:08 PM
To: nginx-devel@nginx.org
Subject: Testing TLSv1.3 session resumption in Nginx using PSK

Hi,

I have been trying to implement a TLSv1.3 session resumption mechanism with 
Nginx and openssl-1.1.1-dev.
Basic TLSv1.3 handshake works fine. Now I wanna test session resumption via PSK 
but couldn't find a way to do it.

Please guide me on how to enable the Nginx server to send a new_session_ticket 
to the client in order to enable session resumption using PSK in TLSv1.3

Thank you,
Utkarsh Tewari
Graduate student at San Jose State University
[Image removed by sender.]ᐧ



CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 3 of 3] PSK: add identity hint config directive

2017-06-22 Thread Karstens, Nate
# HG changeset patch
# User Nate Karstens 
# Date 1498137243 18000
#  Thu Jun 22 08:14:03 2017 -0500
# Node ID b706695658216c88716904519467a36c1aac7ac9
# Parent  a4635fa4a0cabf5312cda617b8010ea14279ab1c
PSK: add identity hint config directive

Adds the directive "ssl_psk_identity_hint" to the ngx_http_ssl_module.
This allows the user to specify the PSK identity hint given to the
connecting client.

Signed-off-by: Nate Karstens 

diff -r a4635fa4a0ca -r b70669565821 contrib/vim/syntax/nginx.vim
--- a/contrib/vim/syntax/nginx.vim  Thu Jun 22 08:13:27 2017 -0500
+++ b/contrib/vim/syntax/nginx.vim  Thu Jun 22 08:14:03 2017 -0500
@@ -551,6 +551,7 @@
 syn keyword ngxDirective contained ssl_preread
 syn keyword ngxDirective contained ssl_protocols
 syn keyword ngxDirective contained ssl_psk_file
+syn keyword ngxDirective contained ssl_psk_identity_hint
 syn keyword ngxDirective contained ssl_session_cache
 syn keyword ngxDirective contained ssl_session_ticket_key
 syn keyword ngxDirective contained ssl_session_tickets
diff -r a4635fa4a0ca -r b70669565821 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.cThu Jun 22 08:13:27 2017 -0500
+++ b/src/http/modules/ngx_http_ssl_module.cThu Jun 22 08:14:03 2017 -0500
@@ -241,6 +241,13 @@
   offsetof(ngx_http_ssl_srv_conf_t, psk_file),
   NULL },

+{ ngx_string("ssl_psk_identity_hint"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
+  ngx_conf_set_str_slot,
+  NGX_HTTP_SRV_CONF_OFFSET,
+  offsetof(ngx_http_ssl_srv_conf_t, psk_identity_hint),
+  NULL },
+
   ngx_null_command
 };

@@ -550,6 +557,7 @@
  * sscf->stapling_file = { 0, NULL };
  * sscf->stapling_responder = { 0, NULL };
  * sscf->psk_file = { 0, NULL };
+ * sscf->psk_identity_hint = { 0, NULL };
  */

 sscf->enable = NGX_CONF_UNSET;
@@ -632,6 +640,7 @@
  prev->stapling_responder, "");

 ngx_conf_merge_str_value(conf->psk_file, prev->psk_file, "");
+ngx_conf_merge_str_value(conf->psk_identity_hint, prev->psk_identity_hint, 
"");

 conf->ssl.log = cf->log;

@@ -819,6 +828,15 @@
 return NGX_CONF_ERROR;
 }

+if (conf->psk_identity_hint.len != 0) {
+if (SSL_CTX_use_psk_identity_hint(conf->ssl.ctx,
+  (char *) 
conf->psk_identity_hint.data)
+!= 1)
+{
+return NGX_CONF_ERROR;
+}
+}
+
 return NGX_CONF_OK;
 }

diff -r a4635fa4a0ca -r b70669565821 src/http/modules/ngx_http_ssl_module.h
--- a/src/http/modules/ngx_http_ssl_module.hThu Jun 22 08:13:27 2017 -0500
+++ b/src/http/modules/ngx_http_ssl_module.hThu Jun 22 08:14:03 2017 -0500
@@ -56,6 +56,7 @@
 ngx_str_t   stapling_responder;

 ngx_str_t   psk_file;
+ngx_str_t   psk_identity_hint;

 u_char *file;
 ngx_uint_t  line;



CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[PATCH 1 of 3] PSK: connection support

2017-06-22 Thread Karstens, Nate
# HG changeset patch
# User Nate Karstens 
# Date 1498137180 18000
#  Thu Jun 22 08:13:00 2017 -0500
# Node ID 3fb3c4928d06029ca1d57853a163c9f56fa90bca
# Parent  6169dbad37d85fa8642b9d0a51f0f0f6c19dd3d1
PSK: connection support

Adds support for TLS connections using PSK cipher suites. A new
configuration directive, ssl_psk_file, specifies the file that
contains a list of identities and associated PSKs. Each line of
the file begins with the identity, followed by a colon character
(':'), and ending with the PSK. As required by RFC 4279 section
5.4, PSKs may be entered either as plain text or using hexadecimal
encoding. Hexadecimal PSKs must begin with "{HEX}". PSKs without
this prefix are assumed to be plain text, but they may optionally
begin with "{PLAIN}" to denote this. Some examples:

gary:plain_text_password
min:{PLAIN}another_text_password
cliff:{HEX}ab0123CD

The format of the given PSK file is checked at server startup.

PSK functionality can be easily tested with the OpenSSL s_client
using the "-psk" and "-psk_identity" options.

Signed-off-by: Nate Karstens 

diff -r 6169dbad37d8 -r 3fb3c4928d06 contrib/vim/syntax/nginx.vim
--- a/contrib/vim/syntax/nginx.vim  Wed Jun 14 20:13:41 2017 +0300
+++ b/contrib/vim/syntax/nginx.vim  Thu Jun 22 08:13:00 2017 -0500
@@ -550,6 +550,7 @@
 syn keyword ngxDirective contained ssl_prefer_server_ciphers
 syn keyword ngxDirective contained ssl_preread
 syn keyword ngxDirective contained ssl_protocols
+syn keyword ngxDirective contained ssl_psk_file
 syn keyword ngxDirective contained ssl_session_cache
 syn keyword ngxDirective contained ssl_session_ticket_key
 syn keyword ngxDirective contained ssl_session_tickets
diff -r 6169dbad37d8 -r 3fb3c4928d06 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Wed Jun 14 20:13:41 2017 +0300
+++ b/src/event/ngx_event_openssl.c Thu Jun 22 08:13:00 2017 -0500
@@ -11,6 +11,7 @@


 #define NGX_SSL_PASSWORD_BUFFER_SIZE  4096
+#define NGX_SSL_PSK_BUFFER_SIZE   4096


 typedef struct {
@@ -23,6 +24,8 @@
 static int ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store);
 static void ngx_ssl_info_callback(const ngx_ssl_conn_t *ssl_conn, int where,
 int ret);
+static unsigned int ngx_ssl_psk_callback(SSL *ssl, const char *identity,
+unsigned char *psk, unsigned int max_psk_len);
 static void ngx_ssl_passwords_cleanup(void *data);
 static void ngx_ssl_handshake_handler(ngx_event_t *ev);
 static ngx_int_t ngx_ssl_handle_recv(ngx_connection_t *c, int n);
@@ -65,6 +68,11 @@
 #endif
 ASN1_TIME *asn1time);

+static ngx_int_t ngx_ssl_psk_convert_hex(u_char **buf, const u_char *psk,
+const u_char *last, ngx_uint_t line);
+static ngx_int_t ngx_ssl_psk_read(ngx_str_t *file, const char *identity,
+unsigned char *psk, int max_psk_len);
+
 static void *ngx_openssl_create_conf(ngx_cycle_t *cycle);
 static char *ngx_openssl_engine(ngx_conf_t *cf, ngx_command_t *cmd, void 
*conf);
 static void ngx_openssl_exit(ngx_cycle_t *cycle);
@@ -114,6 +122,7 @@
 int  ngx_ssl_next_certificate_index;
 int  ngx_ssl_certificate_name_index;
 int  ngx_ssl_stapling_index;
+int  ngx_ssl_psk_index;


 ngx_int_t
@@ -225,6 +234,13 @@
 return NGX_ERROR;
 }

+ngx_ssl_psk_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL);
+
+if (ngx_ssl_psk_index == -1) {
+ngx_ssl_error(NGX_LOG_ALERT, log, 0, "SSL_get_ex_new_index() failed");
+return NGX_ERROR;
+}
+
 return NGX_OK;
 }

@@ -345,6 +361,7 @@
 SSL_CTX_set_read_ahead(ssl->ctx, 1);

 SSL_CTX_set_info_callback(ssl->ctx, ngx_ssl_info_callback);
+SSL_CTX_set_psk_server_callback(ssl->ctx, ngx_ssl_psk_callback);

 return NGX_OK;
 }
@@ -875,6 +892,29 @@
 }


+static unsigned int ngx_ssl_psk_callback(SSL *ssl, const char *identity,
+unsigned char *psk, unsigned int max_psk_len)
+{
+SSL_CTX*ssl_ctx;
+ngx_str_t  *psk_file;
+ngx_int_t   psk_len;
+
+ssl_ctx = SSL_get_SSL_CTX(ssl);
+
+psk_file = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_psk_index);
+if (psk_file == NULL) {
+return 0;
+}
+
+psk_len = ngx_ssl_psk_read(psk_file, identity, psk, max_psk_len);
+if (psk_len < 0) {
+return 0;
+}
+
+return psk_len;
+}
+
+
 RSA *
 ngx_ssl_rsa512_key_callback(ngx_ssl_conn_t *ssl_conn, int is_export,
 int key_length)
@@ -3137,6 +3177,24 @@
 #endif


+ngx_int_t
+ngx_ssl_psk_file(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file)
+
+{
+ngx_int_t   rc;
+
+if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_psk_index, file) == 0) {
+ngx_ssl_error(NGX_LOG_ALERT, ssl->log, 0,
+  "SSL_CTX_set_ex_data() failed");
+return NGX_ERROR;
+}
+
+rc = ngx_ssl_psk_read(file, NULL, NULL, 0);
+
+return rc == 0 ? NGX_OK : NGX_ERROR;
+}
+
+
 void
 ngx_ssl_cleanup_ctx(void *data)
 {
@@ -4127,6 +4185,223 @@
 }


+static ngx_int_t
+ngx_ssl_psk_convert_hex(u_char **buf, const 

RE: PSK Support

2017-06-08 Thread Karstens, Nate
Maxim,

OK, we can skip the patch for turning off the certificate warnings (and just 
use a dummy certificate) and just support a single PSK file.

The {HEX} prefix seems OK. I think it would also be good to support an {ASC}. 
It is unlikely that anyone would have an ASCII-based PSK that starts with 
{HEX}, but using {ASC} would provide a way to make prevent that case.

Also, instead of referring to text-based PSKs as ASCII, maybe they should be 
UTF8-encoded and referred to as {TXT}?

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Wednesday, June 07, 2017 1:13 PM
To: nginx-devel@nginx.org
Subject: Re: PSK Support

Hello!

On Wed, Jun 07, 2017 at 05:02:52AM +, Karstens, Nate wrote:

> Maxim,
>
> The biggest downside that I can see to the dummy certificate approach
> is documentation. Using a dummy certificate wasn't immediately obvious
> to me, though perhaps my familiarity with OpenSSL's "nocert" option
> may have affected that. Which do you think would be easier for the
> user to find in the documentation:
> 1) a description of the dummy certificate approach under the
> "ssl_certificate" directive, 2) a separate directive ("ssl_nocert"),
> or 3) an explicit option to the "ssl_certificate" directive (e.g., "
> Syntax: ssl_certificate file | off;")?

In most cases, normal users won't need PSK at all, or will use it combined with 
certificates anyway.  Describing in the documentation that "if you need to use 
PSK only, you still have to configure a certificate" doesn't looks like a big 
problem to me.

Note well that there is a side problem with "listen ... ssl" used in server 
block but no ssl_certificate configured for the server.
As of now, it is not reported during configuration parsing and may result in 
non-obvious behaviour in some cases, see ticket #178 
(https://trac.nginx.org/nginx/ticket/178).  This is a separate problem though, 
and it can't be fixed with the changes suggested.
On the other hand, properly fixing it will greatly improve user experience in 
many cases, including PSK.

> I'm OK with changing it to read from a password file (formatted in a
> manner similar to stunnel) that is searched as needed (an
> "ssl_psk_file" directive). Would it be OK to support multiple files
> and stipulate that files are searched in the order that they are
> included in nginx.conf?

I don't think that supporting multiple files is a good idea, a single one 
should be enough.

Possible alternative names:

- "ssl_psk_secrets", similar to the one used by stunnel;
- "ssl_pre_shared_keys".

Not sure if these are better though.

> Can we support both ASCII and binary PSKs? RFC 4279 section 5.4 seems
> to require both types, and I need binary keys for my application :).
> Maybe a parameter to the "ssl_psk_file"
> directive could indicate how the PSKs are stored in the file?

We can consider providing an alternative form to allow arbitrary keys, e.g., by 
using hex-encoded keys.  This can be done using some explicit prefix, something 
like this:

identity:key
identity:0x6b6579
identity:{HEX}6b6579

(The last variant is in line with "{scheme}data" syntax as used in 
auth_basic_user_file.  Not sure if we need it here, just "0x"
might be easier.)

--
Maxim Dounin
http://nginx.org/
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel



CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


RE: PSK Support

2017-06-06 Thread Karstens, Nate
Maxim,

The biggest downside that I can see to the dummy certificate approach is 
documentation. Using a dummy certificate wasn't immediately obvious to me, 
though perhaps my familiarity with OpenSSL's "nocert" option may have affected 
that. Which do you think would be easier for the user to find in the 
documentation: 1) a description of the dummy certificate approach under the 
"ssl_certificate" directive, 2) a separate directive ("ssl_nocert"), or 3) an 
explicit option to the "ssl_certificate" directive (e.g., " Syntax: 
ssl_certificate file | off;")?

I'm OK with changing it to read from a password file (formatted in a manner 
similar to stunnel) that is searched as needed (an "ssl_psk_file" directive). 
Would it be OK to support multiple files and stipulate that files are searched 
in the order that they are included in nginx.conf?

Can we support both ASCII and binary PSKs? RFC 4279 section 5.4 seems to 
require both types, and I need binary keys for my application :). Maybe a 
parameter to the "ssl_psk_file" directive could indicate how the PSKs are 
stored in the file?

Thanks,

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Monday, June 05, 2017 1:40 PM
To: nginx-devel@nginx.org
Subject: Re: PSK Support

Hello!

On Mon, Jun 05, 2017 at 02:08:15PM +, Karstens, Nate wrote:

> Maxim,
>
> Thanks for the reply. I understand your concerns about PSK. We
> discussed it quite a bit, but ultimately decided that a PKI was not
> practical for our environment. We have to rely on the end user to
> configure security and any solution using PKI would be so difficult to
> work with that they just wouldn't bother with security at all.

Ok, understood.  I think that PSK can be a reasonable alternative to using 
plain http in many cases.

> I considered some alternatives on the "ssl_nocert" option. My
> preference would have been to analyze the supported cipher suites
> (from "ssl_ciphers") and determine if any include a PSK, but it does
> not look like OpenSSL exposes APIs to accomplish this.

By default, nginx uses "HIGH:!aNULL:!MD5" as ciphers list, and this includes 
various PSK ciphers as well, so this approach doesn't look working even if 
there were appropriate APIs.

> Using a dummy certificate seemed more complicated than the other two
> suggestions you had (using "ssl_certificate" with a value of "off" or
> disabling the tests if there are PSK secrets), so I'd prefer one of
> those two. What is your preference?

Using a dummy certificate has an obvious benefit of not requiring any changes 
to the code, and might actually be a good starting option.

Disabling the tests with PSK secrets might not work as expected when they are 
defined at the http{} level.  Using "ssl_certificate off" is obviously most 
explicit of all options, but I would rather consider a dummy certificate 
instead for now, as long as there are no other downsides.

> One advantage of the PSK path concept is that it provides a lot of
> flexibility. It allows, for example, multiple applications to each
> independently manage their own PSKs without the need to coordinate
> changes to a single file (note that in this scenario each application
> would want to use $ssl_psk_identity to check the key).

On the one hand, it is a plus.  On the other - it is a nightmare when something 
goes wrong.  I would rather avoid such approach.

> stunnel uses a single file and seems to assume that keys will be ASCII
> strings. Its format, for example, would not allow NUL to appear in the
> string, as that would terminate the key early and, at best, lead to a
> reduced key size.

Yes, and stunnel author considers this to be a feature, see 
https://www.stunnel.org/pipermail/stunnel-users/2015-October/005275.html.
If you are targeting end-users, it might be actually easier to use sufficiently 
long printable keys then arbitrary binary strings.

> I might be mistaken, but wouldn't changing a certificate also require
> reloading the configuration? Do you have some ideas on how this could
> be done without requiring a reload?

Yes, changing a certificate requires a reload.  But the "path"
concept is generally used in SSL where appropriate filesystem lookups are done 
on the fly, in contrast to loading a file into memory and then working with the 
data from memory.  Consider "openssl verify -CApath" vs. "openssl verify 
-CAfile".

Additionally, PSK keys look much more dynamic than certificates, as adding a 
user requires configuration changes.  With PKI, you don't need any certificate 
changes on the server to add a user.
With PSK, you have to add a key to introduce a new user.

Overall, PSK seems to be very close to basic authentication, and it 

RE: PSK Support

2017-06-05 Thread Karstens, Nate
Maxim,

Thanks for the reply. I understand your concerns about PSK. We discussed it 
quite a bit, but ultimately decided that a PKI was not practical for our 
environment. We have to rely on the end user to configure security and any 
solution using PKI would be so difficult to work with that they just wouldn't 
bother with security at all.

I considered some alternatives on the "ssl_nocert" option. My preference would 
have been to analyze the supported cipher suites (from "ssl_ciphers") and 
determine if any include a PSK, but it does not look like OpenSSL exposes APIs 
to accomplish this. Using a dummy certificate seemed more complicated than the 
other two suggestions you had (using "ssl_certificate" with a value of "off" or 
disabling the tests if there are PSK secrets), so I'd prefer one of those two. 
What is your preference?

One advantage of the PSK path concept is that it provides a lot of flexibility. 
It allows, for example, multiple applications to each independently manage 
their own PSKs without the need to coordinate changes to a single file (note 
that in this scenario each application would want to use $ssl_psk_identity to 
check the key). stunnel uses a single file and seems to assume that keys will 
be ASCII strings. Its format, for example, would not allow NUL to appear in the 
string, as that would terminate the key early and, at best, lead to a reduced 
key size.

I might be mistaken, but wouldn't changing a certificate also require reloading 
the configuration? Do you have some ideas on how this could be done without 
requiring a reload?

I agree on the proxy_ssl_* directives; the same could probably be said for the 
mail and stream SSL modules.

Regards,

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Maxim 
Dounin
Sent: Monday, June 05, 2017 8:09 AM
To: nginx-devel@nginx.org
Subject: Re: PSK Support

Hello!

On Thu, Jun 01, 2017 at 05:20:53PM +, Karstens, Nate wrote:

> Greetings,
>
> I'm about push 3 patches that add support for PSK TLS cipher suites to
> nginx and thought it would be good to discuss the feature itself in a
> separate thread.
>
> First, PSK support is useful in certain environments that are not
> conducive to a full public key infrastructure. The environment I'm
> personally working with is the recreational boating market; we are
> developing a new industry standard that relies on HTTPS, secured by
> PSK, for much of its underlying security protocol. I think this would
> also be useful to the IoT market. A quick search shows that some other
> users have been interested in this feature:
>
> https://forum.nginx.org/read.php?2,272443,272443
> https://stackoverflow.com/questions/22513641/pre-shared-keys-tls-psk-n
> ginx-configuration

I have mixed feelings about PSK.  On the one hand, it is expected to be better 
for constrained devices and also may have various performance benefits for 
internal connections.  On the other hand, using a shared key is a big step 
backwards compared to public-key cryptography.

> After applying the patches, one can enable PSK support by adding a few
> directives to their nginx.conf:
>
> 1) "ssl_nocert" -- This disables checks for a certificate within
> nginx. By default these checks are enabled because most users will
> need a certificate. This is analogous to the "-nocert"
> option in the OpenSSL s_server.
> 2) "ssl_psk_path" -- This is a local folder that contains all of the
> valid PSKs. Each file in the folder is loaded into memory as a PSK,
> and its file name is used as the PSK identity. When the client
> connects it specifies the identity of the PSK it is using for the
> connection. The server looks up the key using hash of the loaded PSKs
> and if the keys match then the TLS handshake is successful. Note that
> the identity of the PSK is made available in the variable
> $ssl_psk_identity.
> 3) Add some PSK ciphers to the "ssl_ciphers" directive.

Some comments, in no particular order:

- the "ssl_nocert" directive looks strange / unneeded, there
  should be a better way to do this (ssl_certificate with a
  special value "off"? just assume a certificate is not needed if
  there are PSK secrets? always require a certificate, and let
  users who don't need specify a dummy one?);

- "ssl_psk_path" seems to be overcomplicated, and yet non-flexible
  as any change to keys requires configuration reloads;

- the only server software with PSK support seems to be stunnel,
  it might be good to be compatible with the form it uses for PSK
  secrets (a file with "IDENTITY:KEY" lines);

- probably eventually there should be a proxy_ssl_* counterpart,
  though it is perfectly ok to don't have it for now.

--
Maxim Dounin
http://nginx.org/
___

[PATCH 3 of 3] PSK: add PSK identity variable

2017-06-01 Thread Karstens, Nate
# HG changeset patch
# User Nate Karstens 
# Date 1496332963 18000
#  Thu Jun 01 11:02:43 2017 -0500
# Node ID cb09937f63834ab74b49a76b9b158dd0a5871309
# Parent  7aa7771191d61ef635478460017446bca1f6db55
PSK: add PSK identity variable

Adds the variable $ssl_psk_identity to get the PSK identity
used in a connnection secured with a PSK cipher suite.

Signed-off-by: Nate Karstens 

diff -r 7aa7771191d6 -r cb09937f6383 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Thu Jun 01 11:01:05 2017 -0500
+++ b/src/event/ngx_event_openssl.c Thu Jun 01 11:02:43 2017 -0500
@@ -4286,6 +4286,33 @@
 }


+ngx_int_t
+ngx_ssl_get_psk_identity(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
+{
+const char *identity;
+size_t  len;
+
+identity = SSL_get_psk_identity(c->ssl->connection);
+
+if (identity == NULL) {
+s->len = 0;
+return NGX_OK;
+}
+
+len = ngx_strlen(identity);
+
+s->data = ngx_pnalloc(pool, len);
+if (s->data == NULL) {
+return NGX_ERROR;
+}
+
+ngx_memcpy(s->data, identity, len);
+s->len = len;
+
+return NGX_OK;
+}
+
+
 static time_t
 ngx_ssl_parse_time(
 #if OPENSSL_VERSION_NUMBER > 0x1010L
diff -r 7aa7771191d6 -r cb09937f6383 src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h Thu Jun 01 11:01:05 2017 -0500
+++ b/src/event/ngx_event_openssl.h Thu Jun 01 11:02:43 2017 -0500
@@ -235,6 +235,8 @@
 ngx_str_t *s);
 ngx_int_t ngx_ssl_get_client_v_remain(ngx_connection_t *c, ngx_pool_t *pool,
 ngx_str_t *s);
+ngx_int_t ngx_ssl_get_psk_identity(ngx_connection_t *c, ngx_pool_t *pool,
+ngx_str_t *s);


 ngx_int_t ngx_ssl_handshake(ngx_connection_t *c);
diff -r 7aa7771191d6 -r cb09937f6383 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.cThu Jun 01 11:01:05 2017 -0500
+++ b/src/http/modules/ngx_http_ssl_module.cThu Jun 01 11:02:43 2017 -0500
@@ -357,6 +357,9 @@
 { ngx_string("ssl_client_v_remain"), NULL, ngx_http_ssl_variable,
   (uintptr_t) ngx_ssl_get_client_v_remain, NGX_HTTP_VAR_CHANGEABLE, 0 },

+{ ngx_string("ssl_psk_identity"), NULL, ngx_http_ssl_variable,
+  (uintptr_t) ngx_ssl_get_psk_identity, NGX_HTTP_VAR_CHANGEABLE, 0 },
+
 { ngx_null_string, NULL, NULL, 0, 0, 0 }
 };



CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[PATCH 2 of 3] PSK: add configuration directives

2017-06-01 Thread Karstens, Nate
# HG changeset patch
# User Nate Karstens 
# Date 1496332865 18000
#  Thu Jun 01 11:01:05 2017 -0500
# Node ID 7aa7771191d61ef635478460017446bca1f6db55
# Parent  a38066b79d71b6ecb62a9f7618afe2cf3ed8a4f9
PSK: add configuration directives

Adds the directive "ssl_psk_dir" to the ngx_http_ssl_module. This
allows the user to specify a folder that contains the set of PSKs
that can be used to form a connection. Each key in the directory
is read into memory and the file name is saved as the key identity.

Also added the "ssl_psk_hash_max_size" and "ssl_psk_hash_bucket_size"
directives to configure the hash that stores PSK data.

This functionality can be easily tested with the OpenSSL s_client
using the "-psk" and "-psk_identity" options.

Signed-off-by: Nate Karstens 

diff -r a38066b79d71 -r 7aa7771191d6 contrib/vim/syntax/nginx.vim
--- a/contrib/vim/syntax/nginx.vim  Thu Jun 01 10:55:04 2017 -0500
+++ b/contrib/vim/syntax/nginx.vim  Thu Jun 01 11:01:05 2017 -0500
@@ -551,6 +551,9 @@
 syn keyword ngxDirective contained ssl_prefer_server_ciphers
 syn keyword ngxDirective contained ssl_preread
 syn keyword ngxDirective contained ssl_protocols
+syn keyword ngxDirective contained ssl_psk_path
+syn keyword ngxDirective contained ssl_psk_hash_bucket_size
+syn keyword ngxDirective contained ssl_psk_hash_max_size
 syn keyword ngxDirective contained ssl_session_cache
 syn keyword ngxDirective contained ssl_session_ticket_key
 syn keyword ngxDirective contained ssl_session_tickets
diff -r a38066b79d71 -r 7aa7771191d6 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Thu Jun 01 10:55:04 2017 -0500
+++ b/src/event/ngx_event_openssl.c Thu Jun 01 11:01:05 2017 -0500
@@ -23,6 +23,8 @@
 static int ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store);
 static void ngx_ssl_info_callback(const ngx_ssl_conn_t *ssl_conn, int where,
 int ret);
+static unsigned int ngx_ssl_psk_callback(SSL *ssl, const char *identity,
+unsigned char *psk, unsigned int max_psk_len);
 static void ngx_ssl_passwords_cleanup(void *data);
 static void ngx_ssl_handshake_handler(ngx_event_t *ev);
 static ngx_int_t ngx_ssl_handle_recv(ngx_connection_t *c, int n);
@@ -114,6 +116,7 @@
 int  ngx_ssl_next_certificate_index;
 int  ngx_ssl_certificate_name_index;
 int  ngx_ssl_stapling_index;
+int  ngx_ssl_psk_index;


 ngx_int_t
@@ -225,6 +228,13 @@
 return NGX_ERROR;
 }

+ngx_ssl_psk_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL);
+
+if (ngx_ssl_psk_index == -1) {
+ngx_ssl_error(NGX_LOG_ALERT, log, 0, "SSL_get_ex_new_index() failed");
+return NGX_ERROR;
+}
+
 return NGX_OK;
 }

@@ -345,6 +355,7 @@
 SSL_CTX_set_read_ahead(ssl->ctx, 1);

 SSL_CTX_set_info_callback(ssl->ctx, ngx_ssl_info_callback);
+SSL_CTX_set_psk_server_callback(ssl->ctx, ngx_ssl_psk_callback);

 return NGX_OK;
 }
@@ -875,6 +886,40 @@
 }


+static unsigned int ngx_ssl_psk_callback(SSL *ssl, const char *identity,
+unsigned char *psk, unsigned int max_psk_len)
+{
+SSL_CTX*ssl_ctx;
+size_t  identity_len;
+ngx_hash_t *psk_hash;
+ngx_uint_t  key;
+ngx_str_t  *psk_str;
+
+ssl_ctx = SSL_get_SSL_CTX(ssl);
+identity_len = strlen(identity);
+
+psk_hash = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_psk_index);
+if (psk_hash == NULL) {
+return 0;
+}
+
+key = ngx_hash_key((u_char *)identity, identity_len);
+
+psk_str = ngx_hash_find(psk_hash, key, (u_char *)identity, identity_len);
+if (psk_str == NULL) {
+return 0;
+}
+
+if (psk_str->len > max_psk_len) {
+return 0;
+}
+
+ngx_memcpy(psk, psk_str->data, psk_str->len);
+
+return psk_str->len;
+}
+
+
 RSA *
 ngx_ssl_rsa512_key_callback(ngx_ssl_conn_t *ssl_conn, int is_export,
 int key_length)
@@ -3137,6 +3182,158 @@
 #endif


+ngx_int_t
+ngx_ssl_psk_path(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *path,
+ ngx_uint_t psk_hash_max_size, ngx_uint_t psk_hash_bucket_size)
+
+{
+ngx_err_t   err;
+ngx_uint_t  level;
+ngx_dir_t   dir;
+ngx_file_t  file;
+ssize_t read;
+ngx_str_t  *psk_str;
+ngx_hash_keys_arrays_t  psk_keys;
+ngx_str_t   key;
+ngx_hash_t *psk_hash;
+ngx_hash_init_t hash;
+
+if (path->len == 0) {
+return NGX_OK;
+}
+
+psk_keys.pool = cf->pool;
+psk_keys.temp_pool = cf->temp_pool;
+
+ngx_hash_keys_array_init(_keys, NGX_HASH_SMALL);
+
+if (ngx_open_dir(path, ) == NGX_ERROR) {
+err = ngx_errno;
+
+level = (err == NGX_ENOENT
+ || err == NGX_ENOTDIR
+ || err == NGX_ENAMETOOLONG
+ || err == NGX_EACCES) ? NGX_LOG_ERR : NGX_LOG_CRIT;
+
+ngx_ssl_error(level, ssl->log, err,
+  ngx_open_dir_n " 

PSK Support

2017-06-01 Thread Karstens, Nate
Greetings,

I'm about push 3 patches that add support for PSK TLS cipher suites to nginx 
and thought it would be good to discuss the feature itself in a separate thread.

First, PSK support is useful in certain environments that are not conducive to 
a full public key infrastructure. The environment I'm personally working with 
is the recreational boating market; we are developing a new industry standard 
that relies on HTTPS, secured by PSK, for much of its underlying security 
protocol. I think this would also be useful to the IoT market. A quick search 
shows that some other users have been interested in this feature:

https://forum.nginx.org/read.php?2,272443,272443
https://stackoverflow.com/questions/22513641/pre-shared-keys-tls-psk-nginx-configuration

After applying the patches, one can enable PSK support by adding a few 
directives to their nginx.conf:

1) "ssl_nocert" -- This disables checks for a certificate within nginx. By 
default these checks are enabled because most users will need a certificate. 
This is analogous to the "-nocert" option in the OpenSSL s_server.
2) "ssl_psk_path" -- This is a local folder that contains all of the valid 
PSKs. Each file in the folder is loaded into memory as a PSK, and its file name 
is used as the PSK identity. When the client connects it specifies the identity 
of the PSK it is using for the connection. The server looks up the key using 
hash of the loaded PSKs and if the keys match then the TLS handshake is 
successful. Note that the identity of the PSK is made available in the variable 
$ssl_psk_identity.
3) Add some PSK ciphers to the "ssl_ciphers" directive.

Thanks,

Nate Karstens
Garmin International, Inc.



CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[PATCH 1 of 3] PSK: make server certificates optional

2017-06-01 Thread Karstens, Nate
# HG changeset patch
# User Nate Karstens 
# Date 1496332504 18000
#  Thu Jun 01 10:55:04 2017 -0500
# Node ID a38066b79d71b6ecb62a9f7618afe2cf3ed8a4f9
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
PSK: make server certificates optional

Adds the directive "ssl_nocert" to the ngx_http_ssl_module to allow the
user to indicate that the absence of a certificate is intentional. Any
cipher suites that rely on certificates will not function properly.

Servers that only use PSK will error out without this change.

Signed-off-by: Nate Karstens 

diff -r 716852cce913 -r a38066b79d71 contrib/vim/syntax/nginx.vim
--- a/contrib/vim/syntax/nginx.vim  Thu Jun 01 15:44:23 2017 +0300
+++ b/contrib/vim/syntax/nginx.vim  Thu Jun 01 10:55:04 2017 -0500
@@ -546,6 +546,7 @@
 syn keyword ngxDirective contained ssl_ecdh_curve
 syn keyword ngxDirective contained ssl_engine
 syn keyword ngxDirective contained ssl_handshake_timeout
+syn keyword ngxDirective contained ssl_nocert
 syn keyword ngxDirective contained ssl_password_file
 syn keyword ngxDirective contained ssl_prefer_server_ciphers
 syn keyword ngxDirective contained ssl_preread
diff -r 716852cce913 -r a38066b79d71 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.cThu Jun 01 15:44:23 2017 +0300
+++ b/src/http/modules/ngx_http_ssl_module.cThu Jun 01 10:55:04 2017 -0500
@@ -101,6 +101,13 @@
   0,
   NULL },

+{ ngx_string("ssl_nocert"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
+  ngx_conf_set_flag_slot,
+  NGX_HTTP_SRV_CONF_OFFSET,
+  offsetof(ngx_http_ssl_srv_conf_t, nocert),
+  NULL },
+
 { ngx_string("ssl_dhparam"),
   NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
   ngx_conf_set_str_slot,
@@ -546,6 +553,7 @@
 sscf->buffer_size = NGX_CONF_UNSET_SIZE;
 sscf->verify = NGX_CONF_UNSET_UINT;
 sscf->verify_depth = NGX_CONF_UNSET_UINT;
+sscf->nocert = NGX_CONF_UNSET;
 sscf->certificates = NGX_CONF_UNSET_PTR;
 sscf->certificate_keys = NGX_CONF_UNSET_PTR;
 sscf->passwords = NGX_CONF_UNSET_PTR;
@@ -595,6 +603,7 @@
 ngx_conf_merge_uint_value(conf->verify, prev->verify, 0);
 ngx_conf_merge_uint_value(conf->verify_depth, prev->verify_depth, 1);

+ngx_conf_merge_value(conf->nocert, prev->nocert, 0);
 ngx_conf_merge_ptr_value(conf->certificates, prev->certificates, NULL);
 ngx_conf_merge_ptr_value(conf->certificate_keys, prev->certificate_keys,
  NULL);
@@ -622,50 +631,52 @@

 conf->ssl.log = cf->log;

-if (conf->enable) {
+   if (!conf->nocert) {
+if (conf->enable) {

-if (conf->certificates == NULL) {
-ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-  "no \"ssl_certificate\" is defined for "
-  "the \"ssl\" directive in %s:%ui",
-  conf->file, conf->line);
-return NGX_CONF_ERROR;
-}
+if (conf->certificates == NULL) {
+ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+  "no \"ssl_certificate\" is defined for "
+  "the \"ssl\" directive in %s:%ui",
+  conf->file, conf->line);
+return NGX_CONF_ERROR;
+}

-if (conf->certificate_keys == NULL) {
-ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-  "no \"ssl_certificate_key\" is defined for "
-  "the \"ssl\" directive in %s:%ui",
-  conf->file, conf->line);
-return NGX_CONF_ERROR;
-}
+if (conf->certificate_keys == NULL) {
+ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+  "no \"ssl_certificate_key\" is defined for "
+  "the \"ssl\" directive in %s:%ui",
+  conf->file, conf->line);
+return NGX_CONF_ERROR;
+}

-if (conf->certificate_keys->nelts < conf->certificates->nelts) {
-ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-  "no \"ssl_certificate_key\" is defined "
-  "for certificate \"%V\" and "
-  "the \"ssl\" directive in %s:%ui",
-  ((ngx_str_t *) conf->certificates->elts)
-  + conf->certificates->nelts - 1,
-  conf->file, conf->line);
-return NGX_CONF_ERROR;
-}
+if (conf->certificate_keys->nelts < conf->certificates->nelts) {
+ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+  "no \"ssl_certificate_key\" is defined "
+  "for certificate \"%V\" and "
+  "the \"ssl\" directive in %s:%ui",
+  ((ngx_str_t 

RE: [PATCH] Proxy: support configuration of socket buffer sizes

2017-05-18 Thread Karstens, Nate
Greetings,

I just wanted to follow up on this patch and make sure that the fraud detection 
notice or confidentiality notice added by my company wasn't precluding it from 
consideration.

Thanks!

Nate

-Original Message-
From: nginx-devel [mailto:nginx-devel-boun...@nginx.org] On Behalf Of Karstens, 
Nate
Sent: Sunday, April 30, 2017 12:19 PM
To: nginx-devel@nginx.org
Subject: [PATCH] Proxy: support configuration of socket buffer sizes

[This sender failed our fraud detection checks and may not be who they appear 
to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]

# HG changeset patch
# User Nate Karstens  # Date 1493467011 18000
#  Sat Apr 29 06:56:51 2017 -0500
# Node ID 1251a543804b17941b2c96b84bd1f4e58a37bc15
# Parent  8801ff7d58e1650c9d1abb50e09f5979e4f9ffbf
Proxy: support configuration of socket buffer sizes

Allows the size of the buffers used by the TCP sockets for HTTP proxy 
connections to be configured. The new configuration directives are:

 * proxy_socket_rcvbuf
 * proxy_socket_sndbuf

These correspond with the SO_RCVBUF and SO_SNDBUF socket options, respectively.

This is be useful in cases where the proxy processes received data slowly. Data 
was being buffered in three separate TCP buffers (nginx-from-client receive, 
nginx- to-proxy send, and proxy-from-nginx receive). The cumulative effect is 
that the client thinks it has sent all of the data, but times out waiting for a 
reply from the proxy, which cannot reply because it is still processing the 
data in its buffers.

Signed-off-by: Nate Karstens 

diff -r 8801ff7d58e1 -r 1251a543804b src/event/ngx_event_connect.c
--- a/src/event/ngx_event_connect.c Tue Apr 25 23:39:13 2017 +0300
+++ b/src/event/ngx_event_connect.c Sat Apr 29 06:56:51 2017 -0500
@@ -73,6 +73,16 @@
 }
 }

+if (pc->sndbuf) {
+if (setsockopt(s, SOL_SOCKET, SO_SNDBUF,
+   (const void *) >sndbuf, sizeof(int)) == -1)
+{
+ngx_log_error(NGX_LOG_ALERT, pc->log, ngx_socket_errno,
+  "setsockopt(SO_SNDBUF) failed");
+goto failed;
+}
+}
+
 if (ngx_nonblocking(s) == -1) {
 ngx_log_error(NGX_LOG_ALERT, pc->log, ngx_socket_errno,
   ngx_nonblocking_n " failed"); diff -r 8801ff7d58e1 -r 
1251a543804b src/event/ngx_event_connect.h
--- a/src/event/ngx_event_connect.h Tue Apr 25 23:39:13 2017 +0300
+++ b/src/event/ngx_event_connect.h Sat Apr 29 06:56:51 2017 -0500
@@ -57,6 +57,7 @@

 int  type;
 int  rcvbuf;
+int  sndbuf;

 ngx_log_t   *log;

diff -r 8801ff7d58e1 -r 1251a543804b src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c  Tue Apr 25 23:39:13 2017 +0300
+++ b/src/http/modules/ngx_http_proxy_module.c  Sat Apr 29 06:56:51 2017 
+++ -0500
@@ -90,6 +90,9 @@
 ngx_uint_t headers_hash_max_size;
 ngx_uint_t headers_hash_bucket_size;

+ngx_int_t  rcvbuf;
+ngx_int_t  sndbuf;
+
 #if (NGX_HTTP_SSL)
 ngx_uint_t ssl;
 ngx_uint_t ssl_protocols;
@@ -629,6 +632,20 @@
   offsetof(ngx_http_proxy_loc_conf_t, http_version),
   _http_proxy_http_version },

+{ ngx_string("proxy_socket_rcvbuf"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+  ngx_conf_set_num_slot,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  offsetof(ngx_http_proxy_loc_conf_t, rcvbuf),
+  NULL },
+
+{ ngx_string("proxy_socket_sndbuf"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+  ngx_conf_set_num_slot,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  offsetof(ngx_http_proxy_loc_conf_t, sndbuf),
+  NULL },
+
 #if (NGX_HTTP_SSL)

 { ngx_string("proxy_ssl_session_reuse"),
@@ -905,6 +922,13 @@

 u->buffering = plcf->upstream.buffering;

+if (plcf->rcvbuf != NGX_CONF_UNSET) {
+u->peer.rcvbuf = plcf->rcvbuf;
+}
+if (plcf->sndbuf != NGX_CONF_UNSET) {
+u->peer.sndbuf = plcf->sndbuf;
+}
+
 u->pipe = ngx_pcalloc(r->pool, sizeof(ngx_event_pipe_t));
 if (u->pipe == NULL) {
 return NGX_HTTP_INTERNAL_SERVER_ERROR; @@ -2902,6 +2926,9 @@
 conf->headers_hash_max_size = NGX_CONF_UNSET_UINT;
 conf->headers_hash_bucket_size = NGX_CONF_UNSET_UINT;

+conf->rcvbuf = NGX_CONF_UNSET;
+conf->sndbuf = NGX_CONF_UNSET;
+
 ngx_str_set(>upstream.module, "proxy");

 return conf;
@@ -3297,6 +3324,12 @@
 ngx_conf_merge_uint_value(conf->headers_hash_bucket_size,
   prev->headers_hash_bucket_size, 64);

+ngx_conf_merge_value(conf->rcvbuf,
+