unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
						download: 
* [PATCH] Add some tolerance (RFC2616 sec. 19.3)
@ 2016-10-20  9:05 Mishael A Sibiryakov
  2016-10-20 17:55 ` Eric Wong
  0 siblings, 1 reply; 6+ results
From: Mishael A Sibiryakov @ 2016-10-20  9:05 UTC (permalink / raw)
  To: unicorn-public

Hi all.

We're implementing client certificate authentication with nginx and
unicorn. 

Nginx configured in the following way:

proxy_set_header X-SSL-Client-Cert $ssl_client_cert;

When client submits certificate and nginx passes it to the unicorn,
unicorn responds with 400 (Bad Request). This caused because nginx
doesn't use "\r\n" they using just "\n" and multilne headers is failed
to parse (I've added test).

Accorording to RFC2616 section 19.3:
https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.3

"The line terminator for message-header fields is the sequence CRLF.
However, we recommend that applications, when parsing such headers,
recognize a single LF as a line terminator and ignore the leading CR."

CRLF changed to ("\r\n" | "\n")

Github commit https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf1
76de05720f6be758f8b41b1f


PS: Googling "nginx unicorn ssl_client_cert" shows the problem. 

---
 ext/unicorn_http/unicorn_http_common.rl |  2 +-
 test/unit/test_http_parser.rb           | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/ext/unicorn_http/unicorn_http_common.rl
b/ext/unicorn_http/unicorn_http_common.rl
index cc1d455..507e570 100644
--- a/ext/unicorn_http/unicorn_http_common.rl
+++ b/ext/unicorn_http/unicorn_http_common.rl
@@ -4,7 +4,7 @@
 
 #### HTTP PROTOCOL GRAMMAR
 # line endings
-  CRLF = "\r\n";
+  CRLF = ("\r\n" | "\n");
 
 # character types
   CTL = (cntrl | 127);
diff --git a/test/unit/test_http_parser.rb
b/test/unit/test_http_parser.rb
index c72f7f2..4b1a16e 100644
--- a/test/unit/test_http_parser.rb
+++ b/test/unit/test_http_parser.rb
@@ -230,6 +230,22 @@ def test_nasty_pound_header
     assert_equal expect, req['HTTP_X_SSL_BULLSHIT']
   end
 
+  def test_multiline_header_0d0a
+    parser = HttpParser.new
+    parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
+    req = parser.env
+    assert_equal req, parser.parse
+    assert_equal 'foo bar cha cha zha zha',
req['HTTP_X_MULTILINE_HEADER']
+  end
+
+  def test_multiline_header_0a
+    parser = HttpParser.new
+    parser.buf << "GET / HTTP/1.0\nX-Multiline-Header: foo bar\n\tcha
cha\n\tzha zha\n\n"
+    req = parser.env
+    assert_equal req, parser.parse
+    assert_equal 'foo bar cha cha zha zha',
req['HTTP_X_MULTILINE_HEADER']
+  end
+
   def test_continuation_eats_leading_spaces
     parser = HttpParser.new
     header = "GET / HTTP/1.1\r\n" \
-- 
2.10.1

^ permalink raw reply	[relevance 5%]

* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
  2016-10-20  9:05 [PATCH] Add some tolerance (RFC2616 sec. 19.3) Mishael A Sibiryakov
@ 2016-10-20 17:55 ` Eric Wong
  2016-10-20 20:25   ` Mishael A Sibiryakov
  0 siblings, 1 reply; 6+ results
From: Eric Wong @ 2016-10-20 17:55 UTC (permalink / raw)
  To: Mishael A Sibiryakov; +Cc: unicorn-public

Mishael A Sibiryakov <death@junki.org> wrote:
> Hi all.
> 
> We're implementing client certificate authentication with nginx and
> unicorn. 
> 
> Nginx configured in the following way:
> 
> proxy_set_header X-SSL-Client-Cert $ssl_client_cert;
> 
> When client submits certificate and nginx passes it to the unicorn,
> unicorn responds with 400 (Bad Request). This caused because nginx
> doesn't use "\r\n" they using just "\n" and multilne headers is failed
> to parse (I've added test).
> 
> Accorording to RFC2616 section 19.3:
> https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.3
> 
> "The line terminator for message-header fields is the sequence CRLF.
> However, we recommend that applications, when parsing such headers,
> recognize a single LF as a line terminator and ignore the leading CR."
> 
> CRLF changed to ("\r\n" | "\n")

Thanks for that useful explanation.  Aside from the unnecessary,
"Hi all,", that is an informative commit message which justifies
the usefulness of that patch.

> Github commit https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf1
> 76de05720f6be758f8b41b1f

Unfortunately, the commit message in your git repo is lacking.
I've used the text at the top of your email.

> PS: Googling "nginx unicorn ssl_client_cert" shows the problem. 
> 
> ---
>  ext/unicorn_http/unicorn_http_common.rl |  2 +-
>  test/unit/test_http_parser.rb           | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)

Eeep, Evolution does some strange things with formatting
whitespaces.  It looks like instructions for making it nicer are
in the Linux kernel:

https://bogomips.org/mirrors/linux.git/plain/Documentation/email-clients.txt?h=v4.8

<snip>

> +  def test_multiline_header_0d0a
> +    parser = HttpParser.new
> +    parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"

I expect code to be wrapped at 80 lines or less.  Fixed locally.
(I need big fonts, even 80 is a compromise, I really prefer 64)

Anyways, pushed to the "rfc2616-sec19.3" branch.

I've also uploaded a prerelease 5.1.0.4.gd5fbb to RubyGems
for folks without Ragel.

	gem install --pre unicorn -v 5.1.0.4.gd5fbb

Anything else?  Expect a 5.2.0 release in a few days or so.
Thanks.

^ permalink raw reply	[relevance 6%]

* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
  2016-10-20 17:55 ` Eric Wong
@ 2016-10-20 20:25   ` Mishael A Sibiryakov
  2016-10-20 20:50     ` Eric Wong
  0 siblings, 1 reply; 6+ results
From: Mishael A Sibiryakov @ 2016-10-20 20:25 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn-public

On Thu, 2016-10-20 at 17:55 +0000, Eric Wong wrote:

> Thanks for that useful explanation.  Aside from the unnecessary,
> "Hi all,", that is an informative commit message which justifies
> the usefulness of that patch.
> 
> > 
> > Github commit https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf1
> > 76de05720f6be758f8b41b1f
> 
> Unfortunately, the commit message in your git repo is lacking.
> I've used the text at the top of your email.

It's just a temporary fork and I'm too lazy ))

> Eeep, Evolution does some strange things with formatting
> whitespaces.  It looks like instructions for making it nicer are
> in the Linux kernel:
> 
> https://bogomips.org/mirrors/linux.git/plain/Documentation/email-clients.txt?h=v4.8

Whoops, my fault. 

> > +  def test_multiline_header_0d0a
> > +    parser = HttpParser.new
> > +    parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> > bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
> 
> I expect code to be wrapped at 80 lines or less.  Fixed locally.
> (I need big fonts, even 80 is a compromise, I really prefer 64)

Line 221 with "ssl bullshit" guided me to this.

> Anyways, pushed to the "rfc2616-sec19.3" branch.
> 
> I've also uploaded a prerelease 5.1.0.4.gd5fbb to RubyGems
> for folks without Ragel.
> 
> 	gem install --pre unicorn -v 5.1.0.4.gd5fbb
> 
> Anything else?  Expect a 5.2.0 release in a few days or so.
> Thanks.

Thanks for the quick acceptance.

^ permalink raw reply	[relevance 7%]

* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
  2016-10-20 20:25   ` Mishael A Sibiryakov
@ 2016-10-20 20:50     ` Eric Wong
  2016-10-20 21:03       ` Mishael A Sibiryakov
  0 siblings, 1 reply; 6+ results
From: Eric Wong @ 2016-10-20 20:50 UTC (permalink / raw)
  To: Mishael A Sibiryakov; +Cc: unicorn-public

Mishael A Sibiryakov <death@junki.org> wrote:
> It's just a temporary fork and I'm too lazy ))

<snip>

> Whoops, my fault. 

No worries :)

> > > +  def test_multiline_header_0d0a
> > > +    parser = HttpParser.new
> > > +    parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> > > bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
> > 
> > I expect code to be wrapped at 80 lines or less.  Fixed locally.
> > (I need big fonts, even 80 is a compromise, I really prefer 64)
> 
> Line 221 with "ssl bullshit" guided me to this.

Ah, that's an old test from Mongrel, probably not worth fixing;
just trying to keep new code cleaner.

^ permalink raw reply	[relevance 7%]

* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
  2016-10-20 20:50     ` Eric Wong
@ 2016-10-20 21:03       ` Mishael A Sibiryakov
  0 siblings, 0 replies; 6+ results
From: Mishael A Sibiryakov @ 2016-10-20 21:03 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn-public

On Thu, 2016-10-20 at 20:50 +0000, Eric Wong wrote:

> > > > +  def test_multiline_header_0d0a
> > > > +    parser = HttpParser.new
> > > > +    parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> > > > bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
> > > 
> > > I expect code to be wrapped at 80 lines or less.  Fixed locally.
> > > (I need big fonts, even 80 is a compromise, I really prefer 64)
> > 
> > Line 221 with "ssl bullshit" guided me to this.
> 
> Ah, that's an old test from Mongrel, probably not worth fixing;
> just trying to keep new code cleaner.

It's a good desire :) I'll comply next time.

^ permalink raw reply	[relevance 7%]

* [ANN] unicorn 5.2.0 - Rack HTTP server for fast clients and *nix
      [irrelevant] <20161031-unicorn-5.2.0-released@bogomips.org>
@ 2016-10-31 20:04 ` Eric Wong
  0 siblings, 0 replies; 6+ results
From: Eric Wong @ 2016-10-31 20:04 UTC (permalink / raw)
  To: ruby-talk, unicorn-public; +Cc: Mishael A Sibiryakov

unicorn is an HTTP server for Rack applications designed to only serve
fast clients on low-latency, high-bandwidth connections and take
advantage of features in Unix/Unix-like kernels.  Slow clients should
only be served by placing a reverse proxy capable of fully buffering
both the the request and response in between unicorn and slow clients.

* https://bogomips.org/unicorn/
* public list: unicorn-public@bogomips.org
* mail archives: https://bogomips.org/unicorn-public/
* git clone git://bogomips.org/unicorn.git
* https://bogomips.org/unicorn/NEWS.atom.xml
* nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn

Changes:

    Most notably, this release allows us to support requests with
    lines delimited by LF-only, as opposed to the standard CRLF
    pair and allowed by RFC 2616 sec 19.3.

    Thanks to Mishael A Sibiryakov for the explanation and change:

      https://bogomips.org/unicorn-public/1476954332.1736.156.camel@junki.org/

    Thanks to Let's Encrypt, the website also moves to HTTPS
    <https://bogomips.org/unicorn/> to improve reader privacy.  The
    "unicorn.bogomips.org" subdomain will be retired soon to reduce
    subjectAltName bloat and speed up certificate renewals.

    There's also the usual round of documentation and example
    updates, too.

    Eric Wong (7):
          examples/init.sh: update to reduce upgrade raciness
          doc: systemd should only kill master in example
          examples/logrotate.conf: update example for systemd
          doc: update gmane URLs to point to our own archives
          relocate website to https://bogomips.org/unicorn/
          TODO: remove Rack 2.x item
          build: "install-gem" target avoids network

    Mishael A Sibiryakov (1):
          Add some tolerance (RFC2616 sec. 19.3)

^ permalink raw reply	[relevance 6%]

Results 1-6 of 6 | reverse results
2016-10-20  9:05 [PATCH] Add some tolerance (RFC2616 sec. 19.3) Mishael A Sibiryakov
2016-10-20 17:55 ` Eric Wong
2016-10-20 20:25   ` Mishael A Sibiryakov
2016-10-20 20:50     ` Eric Wong
2016-10-20 21:03       ` Mishael A Sibiryakov
    [irrelevant] <20161031-unicorn-5.2.0-released@bogomips.org>
2016-10-31 20:04 ` [ANN] unicorn 5.2.0 - Rack HTTP server for fast clients and *nix Eric Wong


unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/unicorn-public
	git clone --mirror http://ou63pmih66umazou.onion/unicorn-public

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git