You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2021/03/05 17:56:01 UTC

[Bug 65169] New: Add escaped cert env variables

https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

            Bug ID: 65169
           Summary: Add escaped cert env variables
           Product: Apache httpd-2
           Version: 2.4.46
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: mod_ssl
          Assignee: bugs@httpd.apache.org
          Reporter: michaelo@apache.org
  Target Milestone: ---

A recent Tomcat change rejects all headers with a LF in the value. nginx has
added a ssl_client_escaped_cert which does simple URI encoding. Add HTTPd
equivalents of all variables which contain LFs. Here is the support for Tomcat:
https://github.com/apache/tomcat/pull/406

I consider at least these are affected:
* SSL_CLIENT_CERT
* SSL_CLIENT_CERT_CHAIN_n
* SSL_SERVER_CERT

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #15 from Michael Osipov <mi...@apache.org> ---
(In reply to Joe Orton from comment #13)
> Interesting note from testing: when looking at SSL_CLIENT_CERT_CHAIN_* these
> are really the "set of certificates in the cert chain sent by the client"
> and *not* the set of certificates in the verified cert chain as built up by
> OpenSSL/mod_ssl.
> 
> So the client could send (client cert A, unrelated CA cert B, unrelated CA
> cert C), and certs B&C show up in _CERT_CHAIN_0&1 even if (A, CA X, CA Y, CA
> Z) was the actual client cert chain built and verified by OpenSSL.

This is definitively an issue. Never trust the client.

> The mod_ssl docs merely describe this _CHAIN_n as:
> 
> "PEM-encoded certificates in client certificate chain"
> 
> which is ambiguous.  Does Tomcat present/expect anything in particular here?

Tomcat does not even read the chain:
https://github.com/apache/tomcat/blob/681f2afccc2f22ff5fc3d80ad7e77dbeecd083b2/java/org/apache/catalina/valves/SSLValve.java#L159-L162
But this does not mean that people don't read it in custom valves or other
upstream servers.

> We could switch to the alternate OpenSSL 1.1.0 API SSL_get0_verified_chain():
> 
> https://www.openssl.org/docs/man1.1.0/man3/SSL_get0_verified_chain.html
> 
> which seems a more sensible/useful way to do it, except there is a possible
> behaviour difference here in the case where the client cert is *not*
> verified successfully, in which case we may not get a complete chain.

The proposal would be to 

* leave the chain as-is in 2.4
* Introduce a new config option for mod_ssl which lets the admin choose which
chain to obtain (enum value)
* Default value for 2.4: current behavior, future value:
SSL_get0_verified_chain()

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #11 from Joe Orton <jo...@redhat.com> ---
I've updated https://github.com/apache/httpd/pull/177 to use plain base64 and
to allow "SSLOptions +ExportBase64CertData" to export both server & client
cert.

What about the CERT_CHAIN_n? Are these actually useful/used/needed by Tomcat??

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #4 from Michael Osipov <mi...@apache.org> ---
(In reply to Mark Thomas from comment #3)
> Remove myself from CC.

I intentionally added you to discuss how tomcat should handle it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #8 from Michael Osipov <mi...@apache.org> ---
(In reply to Michael Osipov from comment #6)
> (In reply to Joe Orton from comment #5)
> > Adding SSL_{SERVER,CLIENT}_B64CERT variables (or whatever the name) is
> > trivial, I  would not want to add a config option for this definitely.
> 
> One thing needs to be considered since +ExportCertData would provide both it
> would consume the double amount of memory even if the admin needs only one
> format.  What us your opinion on it?

Having a Base64 version of the export option makes the system consistent. For
the same reason I would export this chain too, this isn't much Tomcat specific,
but for all upstream servers which want to inspect certs.

Why do you want to use Base64 URL instead of plain Base64? Other headers work
with that too and Tomcat would need to add support first.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #5 from Joe Orton <jo...@redhat.com> ---
Adding SSL_{SERVER,CLIENT}_B64CERT variables (or whatever the name) is trivial,
I  would not want to add a config option for this definitely.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #9 from Joe Orton <jo...@redhat.com> ---
(In reply to Michael Osipov from comment #8)
> (In reply to Michael Osipov from comment #6)
> > (In reply to Joe Orton from comment #5)
> > > Adding SSL_{SERVER,CLIENT}_B64CERT variables (or whatever the name) is
> > > trivial, I  would not want to add a config option for this definitely.
> > 
> > One thing needs to be considered since +ExportCertData would provide both it
> > would consume the double amount of memory even if the admin needs only one
> > format.  What us your opinion on it?
> 
> Having a Base64 version of the export option makes the system consistent.
> For the same reason I would export this chain too, this isn't much Tomcat
> specific, but for all upstream servers which want to inspect certs.

Makes sense.

> Why do you want to use Base64 URL instead of plain Base64? Other headers
> work with that too and Tomcat would need to add support first.

I've a mild preference for base64url since it would allow safe use in more
contexts, but plain base64 would work too if that's difficult for Tomcat.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #16 from Michael Osipov <mi...@apache.org> ---
(In reply to Joe Orton from comment #14)
> Will consider that issue separately, I think it's worth addressing too.

It is!

> Fixed this in: https://svn.apache.org/viewvc?view=revision&revision=1887811
> 
> Thanks so much Michael for all the review here, appreciate it.

Thanks for the quick fix!

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|markt@apache.org            |

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
Remove myself from CC.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

Michael Osipov <mi...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |markt@apache.org

--- Comment #2 from Michael Osipov <mi...@apache.org> ---
(In reply to Joe Orton from comment #1)
> The obvious way to do this would be to use the base64url encoding of the
> DER, rather than URL-encoding the PEM, which seems kind of daft.  Can Tomcat
> decode the former?

Not without additional code. would you prefer to introduce new variables or
rather a format co fig option?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #1 from Joe Orton <jo...@redhat.com> ---
The obvious way to do this would be to use the base64url encoding of the DER,
rather than URL-encoding the PEM, which seems kind of daft.  Can Tomcat decode
the former?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #12 from Michael Osipov <mi...@apache.org> ---
(In reply to Joe Orton from comment #11)
> I've updated https://github.com/apache/httpd/pull/177 to use plain base64
> and to allow "SSLOptions +ExportBase64CertData" to export both server &
> client cert.

Looks better now, added a few comments

> What about the CERT_CHAIN_n? Are these actually useful/used/needed by
> Tomcat??

I would retain them too, Tomcat exposes the as an array of X509Certificate
objects. It might be up to the developer to do anything fruitful with that
chain. It should be there for the sake of completeness and consistency.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #10 from Michael Osipov <mi...@apache.org> ---
(In reply to Joe Orton from comment #9)
> (In reply to Michael Osipov from comment #8)
> > (In reply to Michael Osipov from comment #6)
> > > (In reply to Joe Orton from comment #5)
> > > > Adding SSL_{SERVER,CLIENT}_B64CERT variables (or whatever the name) is
> > > > trivial, I  would not want to add a config option for this definitely.
> > > 
> > > One thing needs to be considered since +ExportCertData would provide both it
> > > would consume the double amount of memory even if the admin needs only one
> > > format.  What us your opinion on it?
> > 
> > Having a Base64 version of the export option makes the system consistent.
> > For the same reason I would export this chain too, this isn't much Tomcat
> > specific, but for all upstream servers which want to inspect certs.
> 
> Makes sense.
> 
> > Why do you want to use Base64 URL instead of plain Base64? Other headers
> > work with that too and Tomcat would need to add support first.
> 
> I've a mild preference for base64url since it would allow safe use in more
> contexts, but plain base64 would work too if that's difficult for Tomcat.

I'd prefer plain Base64 for the sake of simplicity.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

Michael Osipov <mi...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |michaelo@apache.org

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #13 from Joe Orton <jo...@redhat.com> ---
Interesting note from testing: when looking at SSL_CLIENT_CERT_CHAIN_* these
are really the "set of certificates in the cert chain sent by the client" and
*not* the set of certificates in the verified cert chain as built up by
OpenSSL/mod_ssl.

So the client could send (client cert A, unrelated CA cert B, unrelated CA cert
C), and certs B&C show up in _CERT_CHAIN_0&1 even if (A, CA X, CA Y, CA Z) was
the actual client cert chain built and verified by OpenSSL.

The mod_ssl docs merely describe this _CHAIN_n as:

"PEM-encoded certificates in client certificate chain"

which is ambiguous.  Does Tomcat present/expect anything in particular here?

We could switch to the alternate OpenSSL 1.1.0 API SSL_get0_verified_chain():

https://www.openssl.org/docs/man1.1.0/man3/SSL_get0_verified_chain.html

which seems a more sensible/useful way to do it, except there is a possible
behaviour difference here in the case where the client cert is *not* verified
successfully, in which case we may not get a complete chain.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

Joe Orton <jo...@redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #14 from Joe Orton <jo...@redhat.com> ---
Will consider that issue separately, I think it's worth addressing too.

Fixed this in: https://svn.apache.org/viewvc?view=revision&revision=1887811

Thanks so much Michael for all the review here, appreciate it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #7 from Joe Orton <jo...@redhat.com> ---
(In reply to Michael Osipov from comment #6)
> (In reply to Joe Orton from comment #5)
> > Adding SSL_{SERVER,CLIENT}_B64CERT variables (or whatever the name) is
> > trivial, I  would not want to add a config option for this definitely.
> 
> One thing needs to be considered since +ExportCertData would provide both it
> would consume the double amount of memory even if the admin needs only one
> format.  What us your opinion on it?

I don't know what the usually recommended configuration is here but it
shouldn't use ExportCertData exactly because it's so expensive.  Better to use
one of the configs using e.g. mod_headers which just extracts the variables
which are required.

If really required, we could add a +ExportBase64CertData or similar which just
does the base64url(der), that is not as painful as adding a new config options
at least.

BTW, does Tomcat really want/need the CLIENT_CERT_CHAIN_n as well here?

Proof of concept here - https://github.com/apache/httpd/pull/177

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65169] Add escaped cert env variables

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65169

--- Comment #6 from Michael Osipov <mi...@apache.org> ---
(In reply to Joe Orton from comment #5)
> Adding SSL_{SERVER,CLIENT}_B64CERT variables (or whatever the name) is
> trivial, I  would not want to add a config option for this definitely.

One thing needs to be considered since +ExportCertData would provide both it
would consume the double amount of memory even if the admin needs only one
format.  What us your opinion on it?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org