[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#8485) [PATCH] Adding support for encrypted server private keys



--001a11447608411b0a054928d3c2
Content-Type: text/plain; charset=UTF-8

I completely agree that this doesn't add any security when running slapd.
However, when I originally wrote this patch, I was more targeting API
usage, and in particular, client-side API usage. My goal was to allow a
user to present an encrypted key (stored on disk) and a password (only held
in memory), and use these to query a secure LDAP server. The issue of how
the password got into memory is certainly beyond the scope of OpenLDAP in
this case, but it certainly could be done securely in a way that wouldn't
leave it exposed to a third-party with file-system access.

For example (in the use case that motivated me originally), an sysadmin
could input the password through an online form, submit said form to a
Python webserver over HTTPS, the Python code could pass the password to
OpenLDAP through a Python interface like python-ldap (
https://www.python-ldap.org/), and then use the LDAPS or STARTTLS
connection to talk securely to some LDAP server elsewhere. All without an
unencrypted keyfile or a password ever existing on disk, hence the added
security.

If you feel that supporting encrypted keyfiles in slapd is a bad design
choice, I'm willing to rework the patch to drop that part and only keep the
API-level support. Personally, I would argue that it doesn't hurt to
support the feature, although perhaps my documentation changes could do a
better job emphasizing the inherent security limitations. Let me know what
you think (or if this discussion should take place on a list, let me know
which/where).


On Wed, Feb 22, 2017 at 1:11 PM, Howard Chu <hyc@symas.com> wrote:

> ahnolds@gmail.com wrote:
>
>> Full_Name: Alec Cooper
>> Version: HEAD of master branch
>> OS: Ubuntu Linux 16.04
>> URL: ftp://ftp.openldap.org/incoming/Alec-Cooper-160827.patch
>> Submission from: (NULL) (73.134.243.211)
>>
>>
>> Adding support for encrypted server private keys.
>>
>
> Thank you for your submission. I must compliment you on providing such a
> comprehensive and well-written contribution. Unfortunately the feature
> itself is clearly not useful. Perhaps you should have raised this topic on
> the openldap-devel list for discussion before spending your time writing it.
>
> All your feature does is embed the encryption key in the filesystem, so an
> encrypted private key is still no more secure than an unencrypted one. It
> still just depends on setting proper file access permissions, and the
> current documentation already makes that point.
>
> Indeed, all it does is give the false illusion of enhancing security, and
> followups like balmerpeak92@gmail.com's shows that people will easily
> fall for such illusions.
>
> I regret having to reject such a well written contribution, but we cannot
> in good conscience accept a security feature that doesn't actually improve
> security.
>
>>
>> The meat of this patch is changes to tls2.c, tls_g.c, tls_m.c, and
>> tls_o.c to
>> send a password to the underlying TLS library.
>>
>> Changes to ldap.h, init.c and ldap-int.h expose the new
>> LDAP_OPT_X_TLS_KEYPASSWORD in the main API.
>>
>> Changes in the contrib/ldapc++ directory expose the corresponding
>> TlsOptions::KEYPASSWORD option in the C++ API.
>>
>> Changes in the servers directory expose equivalent options for servers as
>> configuration file entries or environment variables.
>>
>> Changes in the doc directory add documentation about the new options and
>> remove
>> statements that indicated that encrypted keyfiles are not supported.
>>
>> New files in the test directory are for testing TLS connections, both in
>> general
>> and with encrypted keyfiles. Tests pass for OpenSSL and GnuTLS using PEM
>> formatted certs and keys, and for MozNSS using cert/key databases. The
>> new unit
>> test (test065-tls) has been written to detect when using NSS, and use the
>> cert/key databases in this case. I have been unable to get a working
>> version of
>> libnsspem, so I cannot test MozNSS with (or without) encrypted keyfiles -
>> testing for this case would be welcome!
>>
>> Notice of origin: The attached patch file is derived from OpenLDAP
>> Software. All
>> of the modifications to OpenLDAP Software represented in the following
>> patch
>> were developed by Alec Cooper ahnolds@gmail.com. I have not assigned
>> rights
>> and/or interest in this work to any party.
>> Rights statement: I, Alec Cooper, hereby place the following
>> modifications to
>> OpenLDAP Software (and only these modifications) into the public domain.
>> Hence,
>> these modifications may be freely used and/or redistributed for any
>> purpose with
>> or without attribution and/or other notice.
>>
>> The patch has been uploaded to the OpenLDAP FTP server and can be found at
>> ftp://ftp.openldap.org/incoming/Alec-Cooper-160827.patch
>>
>>
>>
>
> --
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP  http://www.openldap.org/project/
>

--001a11447608411b0a054928d3c2
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">I completely agree that this doesn&#39;t add any security =
when running slapd. However, when I originally wrote this patch, I was more=
 targeting API usage, and in particular, client-side API usage. My goal was=
 to allow a user to present an encrypted key (stored on disk) and a passwor=
d (only held in memory), and use these to query a secure LDAP server. The i=
ssue of how the password got into memory is certainly beyond the scope of O=
penLDAP in this case, but it certainly could be done securely in a way that=
 wouldn&#39;t leave it exposed to a third-party with file-system access.<di=
v><br></div><div>For example (in the use case that motivated me originally)=
, an sysadmin could input the password through an online form, submit said =
form to a Python webserver over HTTPS, the Python code could pass the passw=
ord to OpenLDAP through a Python interface like python-ldap (<a href=3D"htt=
ps://www.python-ldap.org/">https://www.python-ldap.org/</a>), and then use =
the LDAPS or STARTTLS connection to talk securely to some LDAP server elsew=
here. All without an unencrypted keyfile or a password ever existing on dis=
k, hence the added security.<div><br></div><div>If you feel that supporting=
 encrypted keyfiles in slapd is a bad design choice, I&#39;m willing to rew=
ork the patch to drop that part and only keep the API-level support. Person=
ally, I would argue that it doesn&#39;t hurt to support the feature, althou=
gh perhaps my documentation changes could do a better job emphasizing the i=
nherent security limitations. Let me know what you think (or if this discus=
sion should take place on a list, let me know which/where).=C2=A0</div></di=
v><div><br></div></div><div class=3D"gmail_extra"><br><div class=3D"gmail_q=
uote">On Wed, Feb 22, 2017 at 1:11 PM, Howard Chu <span dir=3D"ltr">&lt;<a =
href=3D"mailto:hyc@symas.com"; target=3D"_blank">hyc@symas.com</a>&gt;</span=
> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bo=
rder-left:1px #ccc solid;padding-left:1ex"><a href=3D"mailto:ahnolds@gmail.=
com" target=3D"_blank">ahnolds@gmail.com</a> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
Full_Name: Alec Cooper<br>
Version: HEAD of master branch<br>
OS: Ubuntu Linux 16.04<br>
URL: <a href=3D"ftp://ftp.openldap.org/incoming/Alec-Cooper-160827.patch"; r=
el=3D"noreferrer" target=3D"_blank">ftp://ftp.openldap.org/incomin<wbr>g/Al=
ec-Cooper-160827.patch</a><br>
Submission from: (NULL) (73.134.243.211)<br>
<br>
<br>
Adding support for encrypted server private keys.<br>
</blockquote>
<br>
Thank you for your submission. I must compliment you on providing such a co=
mprehensive and well-written contribution. Unfortunately the feature itself=
 is clearly not useful. Perhaps you should have raised this topic on the op=
enldap-devel list for discussion before spending your time writing it.<br>
<br>
All your feature does is embed the encryption key in the filesystem, so an =
encrypted private key is still no more secure than an unencrypted one. It s=
till just depends on setting proper file access permissions, and the curren=
t documentation already makes that point.<br>
<br>
Indeed, all it does is give the false illusion of enhancing security, and f=
ollowups like <a href=3D"mailto:balmerpeak92@gmail.com"; target=3D"_blank">b=
almerpeak92@gmail.com</a>&#39;s shows that people will easily fall for such=
 illusions.<br>
<br>
I regret having to reject such a well written contribution, but we cannot i=
n good conscience accept a security feature that doesn&#39;t actually impro=
ve security.<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
<br>
The meat of this patch is changes to tls2.c, tls_g.c, tls_m.c, and tls_o.c =
to<br>
send a password to the underlying TLS library.<br>
<br>
Changes to ldap.h, init.c and ldap-int.h expose the new<br>
LDAP_OPT_X_TLS_KEYPASSWORD in the main API.<br>
<br>
Changes in the contrib/ldapc++ directory expose the corresponding<br>
TlsOptions::KEYPASSWORD option in the C++ API.<br>
<br>
Changes in the servers directory expose equivalent options for servers as<b=
r>
configuration file entries or environment variables.<br>
<br>
Changes in the doc directory add documentation about the new options and re=
move<br>
statements that indicated that encrypted keyfiles are not supported.<br>
<br>
New files in the test directory are for testing TLS connections, both in ge=
neral<br>
and with encrypted keyfiles. Tests pass for OpenSSL and GnuTLS using PEM<br=
>
formatted certs and keys, and for MozNSS using cert/key databases. The new =
unit<br>
test (test065-tls) has been written to detect when using NSS, and use the<b=
r>
cert/key databases in this case. I have been unable to get a working versio=
n of<br>
libnsspem, so I cannot test MozNSS with (or without) encrypted keyfiles -<b=
r>
testing for this case would be welcome!<br>
<br>
Notice of origin: The attached patch file is derived from OpenLDAP Software=
. All<br>
of the modifications to OpenLDAP Software represented in the following patc=
h<br>
were developed by Alec Cooper <a href=3D"mailto:ahnolds@gmail.com"; target=
=3D"_blank">ahnolds@gmail.com</a>. I have not assigned rights<br>
and/or interest in this work to any party.<br>
Rights statement: I, Alec Cooper, hereby place the following modifications =
to<br>
OpenLDAP Software (and only these modifications) into the public domain. He=
nce,<br>
these modifications may be freely used and/or redistributed for any purpose=
 with<br>
or without attribution and/or other notice.<br>
<br>
The patch has been uploaded to the OpenLDAP FTP server and can be found at<=
br>
<a href=3D"ftp://ftp.openldap.org/incoming/Alec-Cooper-160827.patch"; rel=3D=
"noreferrer" target=3D"_blank">ftp://ftp.openldap.org/incomin<wbr>g/Alec-Co=
oper-160827.patch</a><br>
<br>
<br><span class=3D"HOEnZb"><font color=3D"#888888">
</font></span></blockquote><span class=3D"HOEnZb"><font color=3D"#888888">
<br>
<br>
-- <br>
=C2=A0 -- Howard Chu<br>
=C2=A0 CTO, Symas Corp.=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0<a href=3D"=
http://www.symas.com"; rel=3D"noreferrer" target=3D"_blank">http://www.symas=
.com</a><br>
=C2=A0 Director, Highland Sun=C2=A0 =C2=A0 =C2=A0<a href=3D"http://highland=
sun.com/hyc/" rel=3D"noreferrer" target=3D"_blank">http://highlandsun.com/h=
yc/</a><br>
=C2=A0 Chief Architect, OpenLDAP=C2=A0 <a href=3D"http://www.openldap.org/p=
roject/" rel=3D"noreferrer" target=3D"_blank">http://www.openldap.org/proje=
c<wbr>t/</a><br>
</font></span></blockquote></div><br></div>

--001a11447608411b0a054928d3c2--