erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.32k stars 2.94k forks source link

httpc fails connection with selfsigned_peer even though peer cert is provided as trusted in cacertfile #8057

Closed csrl closed 1 month ago

csrl commented 8 months ago

Describe the bug When providing the server's self signed cert in cacertfile, the ssl connection fails with selfsigned_peer. Because 3rd party libraries only provide the ability to customize cacertfile, and without the ability to customize verify_fun, self signed certificates are generally not usable under erlang http clients. This is pronounced in otp26, as validating certificates is now correctly enforced by default. I consider this a bug, as it forces users of 3rd party libraries to disable peer validation as pre otp26 nullifying the value of otp26 enforcing peer validation by default.

To Reproduce Steps to reproduce the behavior.

echo | openssl s_client -connect self-signed.badssl.com:443 2>&1 | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > cert.pem
touch empty.pem
erl

Expect this should work, however it fails.

Erlang/OTP 26 [erts-14.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Eshell V14.1 (press Ctrl+G to abort, type help(). for help)
1> inets:start().
ok
2> ssl:start().
ok
3> httpc:request(get, {"https://self-signed.badssl.com/",[]}, [{ssl, [{verify, verify_peer}, {cacertfile, "./cert.pem"}]}],[]).
=NOTICE REPORT==== 26-Jan-2024::13:27:09.730517 ===
TLS client: In state certify at ssl_handshake.erl:2136 generated CLIENT ALERT: Fatal - Bad Certificate

{error,{failed_connect,[{to_address,{"self-signed.badssl.com",
                                     443}},
                        {inet,[inet],
                              {tls_alert,{bad_certificate,"TLS client: In state certify at ssl_handshake.erl:2136 generated CLIENT ALERT: Fatal - Bad Certificate\n"}}}]}}

This next example shows how cacertfile is required to be provided, but providing an empty file with a stub verify_fun allowing self signed certificates is not sufficient to validate the self signed certificate is actually trusted. A more complex verify_fun is required.

Erlang/OTP 26 [erts-14.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Eshell V14.1 (press Ctrl+G to abort, type help(). for help)
1> inets:start().
ok
2> ssl:start().
ok
3> httpc:request(get, {"https://self-signed.badssl.com/",[]}, [{ssl, [{verify, verify_peer},{verify_fun, {fun (_,valid,U) -> {valid,U}; (_,valid_peer,U) -> {valid,U}; (_,{bad_cert, selfsigned_peer},U) -> {valid, U}; (_,Reason,U) -> {fail, Reason} end, []}}]}],[]).
{error,{failed_connect,[{to_address,{"self-signed.badssl.com",
                                     443}},
                        {inet,[inet],
                              {options,incompatible,
                                       [{verify,verify_peer},{cacerts,undefined}]}}]}}
4> httpc:request(get, {"https://self-signed.badssl.com/",[]}, [{ssl, [{verify, verify_peer}, {cacertfile, "./empty.pem"},{verify_fun, {fun (_,valid,U) -> {valid,U}; (_,valid_peer,U) -> {valid,U}; (_,{bad_cert, selfsigned_peer},U) -> {valid, U}; (_,Reason,U) -> {fail, Reason} end, []}}]}],[]).
{ok,{{"HTTP/1.1",200,"OK"},
     [{"cache-control","no-store"},
      {"connection","keep-alive"},
      {"date","Fri, 26 Jan 2024 21:19:53 GMT"},
      {"accept-ranges","bytes"},
      {"etag","\"6567bc98-1f6\""},
      {"server","nginx/1.10.3 (Ubuntu)"},
      {"content-length","502"},
      {"content-type","text/html"},
      {"last-modified","Wed, 29 Nov 2023 22:35:04 GMT"}],
     "<!DOCTYPE html>\n<html>\n<head>\n  <meta charset=\"utf-8\">\n  <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">\n  <link rel=\"shortcut icon\" href=\"/icons/favicon-red.ico\"/>\n  <link rel=\"apple-touch-icon\" href=\"/icons/icon-red.png\"/>\n  <title>self-signed.badssl.com</title>\n  <link rel=\"stylesheet\" href=\"/style.css\">\n  <style>body { background: red; }</style>\n</head>\n<body>\n<div id=\"content\">\n  <h1 style=\"font-size: 12vw;\">\n    self-signed.<br>badssl.com\n  </h1>\n</div>\n\n</body>\n</html>\n"}}
5> 

Expected behavior I expect to be able to connect to a remote server providing a self signed peer by providing the cert in cacert or cacertfile. Otherwise, some 3rd party libraries can not connect to servers providing a self signed certificate, as the 3rd party libraries do not support custom verify_fun callbacks, and the complexity to implement proper self signed certificate validation is not reasonable to expect everyone to do so correctly.

So, update httpc (or lower layer) to support proper self signed certificate validation when the remote server self signed certificate is set as cacert or cacertfile in httpc ssl options.

Affected versions OTP 26, likely all others.

Additional context As a side note, I found that once httpc successfully connected to the remote server using the last example, then in the same erl session, the first example started to work. That seems surprising during this naive test run. Surprising behavior I think should be considered a bug. ie. if an explicit profile/session for httpc isn't setup, every invocation should be isolated, in my thinking.

IngelaAndin commented 8 months ago

If you do not want to do certificate validation you could use option verify_none. If server has a selfsigned certificate it makes no sense as there is no point in performing certificate path validation if you do not have a trusted anchor/root.

httpc:request(get, {"https://self-signed.badssl.com/",[]}, [{ssl, [{verify, verify_none}]}],[]).`

If you want to write your own verify fun that accepts only selfsigned_peer error it is quite possible to do that.

httpc:request(get, {"https://self-signed.badssl.com/",[]}, [{ssl, [{verify, verify_peer}, {cacerts, []},{verify_fun, {fun (_,valid,U) -> {valid,U}; (_,valid_peer,U) -> {valid,U}; (_,{bad_cert, selfsigned_peer},U) -> {valid, U}; (_,Reason,U) -> {fail, Reason} end, []}}]}],[]).

csrl commented 8 months ago

I do want to verify that the cert is a trusted cert. And your example does not verify that. It will accept any self signed certificate. The point is one should be able to easily validate that the server presented cert matches the cert stored in the client. There is not a trivial implementation to do so, and no common 3rd party library wrapping httpc provides proper validation for that case.

csrl commented 8 months ago

Here is semi psuedo code example of what would be great to have supported out of the box, by simply allowing cacert to match the self signed cert itself.

extract_public_key(Cert) ->
 TBSCert = Cert#'OTPCertificate'.tbsCertificate,
 PublicKeyInfo = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,
 PublicKeyInfo#'OTPSubjectPublicKeyInfo'.subjectPublicKey.   

verify_fun(Cert ,{bad_cert ,selfsigned_peer}, #{known_cert_pubkey := PK} = U) ->
  case extract_public_key(Cert) =:= PK of
    false -> {fail ,invalid_server_cert};
    true -> {valid,U}
  end;
%snip rest of valid cert clauses for verify_fun
.

% used by passing in something like this to ssl opts
SslOpts = [{verify_fun, {fun verify_fun/3 ,#{known_cert_pubkey => ClientTrustedServerCert}}}]
csrl commented 8 months ago

Also, I realize my original report was a large wall of text, but if you could take a second look, I think your response is fully addressed in it.

IngelaAndin commented 8 months ago

Well, true when I read it again I think I missed your intent. But there is not really such a thing as verifying a chain that is not a chain. Verifying that the sent self-signed certificate has the expected public_key is not really verifying anything at all. Verifying that the signature on the received cert equals the signature of the owned cert could have some value, but then the client need to know about the servers "self-signed" certificate previous to making the connection and then why not let the server have a real root certificate that it lets the client know about previous to making the connection?

IngelaAndin commented 8 months ago

Also note that the old default was {verify, verify_none}.

csrl commented 8 months ago

Yes, understood the the old default was verify_none. I do not view this issue as a regression, but that the new default requiring peer validation does not work for self signed certificates, and many 3rd party libraries that make https client connections have no way to validate a self signed peer.

It would be very convenient with the current library ecosystem, and certainly no less secure, to support direct peer cert matching with a chain of 1 certificate. The only way to connect to a server using a self signed cert is to turn off peer validation, ie going back to the old non secure behavior.

Many IoT and embbeded devices, network routers, local services, test beds are set up with a self signed certificate. Making a connection to these should not require blindly accepting any self signed certificate presented. So one has to extract the cert once, then use that cert ensuring it does not change going forward - ie no mitm situation. To repeat myself, the simplest way to enable this is to support placing the self signed cert in cacertfile option such that the implementation validate single cert chain. I get conceptually that "doesn't make sense", but what is actually wrong /broken about doing so?

IngelaAndin commented 8 months ago

So you are saying first time when we receive a arbitrary self-signed cert lets put it into our trusted store and make sure that we are always talking to this first site, good or bad we do not really know, but at least it the same?!

I think it would be less secure if we by default would accept such connections because we would be pretending we are able to validate something that we are not able to validate. In testing scenarios it can be ok to skip the validation. In local scenarios it easy enough to create your own CA that you can include in your trust store.

It is quite possible for you to do the match that you want using the verify_fun. If you create a verify_fun with four arguments and then you will get the DER-encoded cert that you can match against the DER of what you saved after your first connection.

csrl commented 7 months ago

I think we agree on that "first time when we receive a arbitrary self-signed lets put it into our trusted store" is not a secure approach. Rather, the perspective is, by whatever means I chose to trust a root certificate, the libraries should allow me to indicate that trust. It is impossible for Erlang to enforce how I decide to trust a root certificate.

All root certificates are self signed. That is the definition of a root certificate. Root certificates must be assigned trust and placed in the certificate store. Now, whether a server is presenting a certificate signed by a trusted root certificate or is presenting the trusted certificate itself should not make a difference to the erlang stack. I provide the certificates I trust. The erlang stack should respect that, regardless of the length of the chain, whether length of 1 or many up to the depth length.

Currently the primary way that the erlang stack provides indicating trusted root certificates, as exposed by various 3rd party libraries in the erlang ecosystem, is via the cacerts or cacertfile option. verify_fun is not exposed in any library I've come across.

Is there an explanation as to why the erlang stack thinks that a certificate chain of 1 is not secure when that certificate is stated to be trusted from the cacert(s|file) options?

Thank you for taking the time to discuss this.

IngelaAndin commented 7 months ago

Yes the so called ROOT-certs are self-signed, this is not the problem. The problem is that a ROOT-cert are not end-entity certs. What most people refer to as validation of the certificate chain is validating the certificate path which is done by an algorithm described in RFC 5280.

The trust anchor (normally the Root Cert) is an input to the algorithm and so is a prospective certification path of length n.

So you do not have a path of 1, but a path of 0, and that is the problem.

Also from the RFC:

" The trust anchor information may be provided to the path processing procedure in the form of a self-signed certificate. When the trust anchor information is provided in the form of a certificate, the name in the subject field is used as the trusted issuer name and the contents of the subjectPublicKeyInfo field is used as the source of the trusted public key algorithm and the trusted public key. The trust anchor information is trusted because it was delivered to the path processing procedure by some trustworthy out-of-band procedure. "

httpc will let you configure the verify_fun, I believe that other software will too but maybe not document that so clearly, but sure there can exist software that does not.

csrl commented 7 months ago

From the RFC: "Usually, the last certificate is an end entity certificate, but it can be a CA certificate."

I guess I see your position when basicConstraints=CA:false on a self signed certificate. However, from my searching, I do not find anything that says a CA certificate (basicConstraints=CA:true) can not also be an End Entity certificate, and the note in the RFC seems to agree with that.

Anyway, I certainly do not have a complete understanding of RFC 5280, but my position is coming from a usability perspective. There are many pieces of hardware shipped with a self signed certificate, and those that I've interacted with have basicConstraints=CA:true. So, a CA cert being used as an End Entity certificate. Being able to securely connect to these devices without unnecessary complexity would be great.

Erlang made the right choice to require TLS connections by default. The significant and practical concern here is the high complexity required to use a secure connection for this use case. I presume many will revert to using verify_none because of it, negating the improved baseline security that OTP 26 attempted to bring to the ecosystem.

Perhaps erlang should return self_signed when basicConstraints=CA:false, but otherwise should allow a chain of 0 when a CA cert is used as the End Entity cert.

If this can not convince you to reevaluate, do feel free to close the issue. I think I've made the case as best I can. :) Thanks!

IngelaAndin commented 7 months ago

Well if I try your purposed setup with OpenSSL (to get another perspective) it will report

Verification error: unsupported certificate purpose

So I remain unconvinced that I misread the RFC.

I think the "Usually, the last certificate is an end entity certificate, but it can be a CA certificate." bit suggest you can path_validate(Trusted, [CA3, CA2, CA1]) instead of path_validate(Trusted, [CA3, CA2, CA1, EndEntity]) if there is some reason to do so.

I do understand your perspective but we have been changing defaults to make less secure choices not possible by default, which is a tradeoff between interoperability (easy usability) and security.

Maybe in the future new TLS-1.3 so called raw certificates will be a better alternative. (We have not implement it yet, but it will come sooner or later).

csrl commented 7 months ago

Can you share your openssl validation check?

$ openssl s_client -connect self-signed.badssl.com:443 -verify_return_error -verify_quiet -quiet
Connecting to 104.154.89.105
depth=0 C=US, ST=California, L=San Francisco, O=BadSSL, CN=*.badssl.com
verify error:num=18:self-signed certificate
40E722A2E8720000:error:0A000086:SSL routines:tls_post_process_server_certificate:certificate verify failed:ssl/statem/statem_clnt.c:2091:
$ echo | openssl s_client -connect self-signed.badssl.com:443 2>&1 | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > cert.pem

$ openssl s_client -connect self-signed.badssl.com:443 -verifyCAfile cert.pem -verify_return_error -verify_quiet -quiet
Connecting to 104.154.89.105
^C

So, openssl accepts using the self signed cert in the CAfile option successfully.

For what it is worth, the self-signed.badssl.com:443 certificate is NOT a basicConstraints=CA: true, so it does not completely represent the case I was arguing. But testing with a local self signed certificate with basicConstraints=CA: true has the same result from openssl.

I don't understand your statement that the requested change is "less secure". I agree changes shouldn't be done that are less secure, and I understand being conservative when uncertain. I just do not read/find anything that indicates this would be the case.

Again, without making a change here, the almost certain behavior of users will be to use verify_none when connecting to hardware that is shipped with a self signed certificate. So leaving this as is will drive less secure behavior in that use case.

IngelaAndin commented 7 months ago

openssl s_client ands_server are test programs and they will make connections for some certificate path validation errors and only print a result of the certificate path validation algorithm. Only if it prints verification ok did all checks pass.

csrl commented 7 months ago
$ openssl s_client -connect self-signed.badssl.com:443 -verifyCAfile cert.pem  |grep "Verification: OK"
Connecting to 104.154.89.105
depth=0 C=US, ST=California, L=San Francisco, O=BadSSL, CN=*.badssl.com
verify return:1
Verification: OK
^C
IngelaAndin commented 7 months ago

Humm, there is some difference between this cert and the one I created. I will look into some more later.

IngelaAndin commented 1 month ago

Well seem you are correct about how OpenSSL behaves. Although, this does not convince me that this is a good idea. See: https://www.ssl.com/article/ssl-tls-self-signed-certificates/

Also you can use public_key:pkix_test_data/1 to easily generate certificate test data, with proper chains.

csrl commented 1 month ago

Thank you for the follow up.

I do not think it is necessary to agree that self signed certificate are less secure or not - they exist and are in use. But we should agree that in the case when a self signed certificate is in use, that it should be validated and not ignored. The complexity of validating a self signed certificate is currently greater than validating a CA signed certificate, resulting in implementations that disable validating self signed certificates at all. Resulting in overall reduced security.

Whether validating a self signed certificate or a certificate signed by an authority, the root of trust has to be established, and whether that root of trust is a CA or the certificate itself is not something Erlang needs to have an opinion one. It should just ensure that whatever the consumer says it trusts, that it validates against the stated trust.

IngelaAndin commented 1 month ago

Erlang will continue striving to follow the spec and promote security. The use of self-signed certificates as end entity certificates is a legacy thing that should be moved away from and not encouraged. But Erlang does not have an opinion if you want to accept them, you can, we just do not want to call them trusted we want the user to make that tradeoff consciously and understanding there are implications of doing so. We have added the possibility to use a verify_fun with four argument so that the user will be able to access the encoded certificate directly which makes the check you want to do a simple match or maybe lists:member operation if you want to have several such certificates that you recognize.

csrl commented 1 month ago

I guess I'm having a difficult time understanding your perspective, so I'm not sure how to respond.

In node.js "https" module, which consumes the "tls" module, provides a "ca" option as documented here: https://nodejs.org/api/tls.html#tlscreatesecurecontextoptions relevant excerpt highlighted here:

The peer's certificate must be chainable to a CA trusted by the server for the connection to be authenticated. When using certificates that are not chainable to a well-known CA, the certificate's CA must be explicitly specified as a trusted or the connection will fail to authenticate. If the peer uses a certificate that doesn't match or chain to one of the default CAs, use the ca option to provide a CA certificate that the peer's certificate can match or chain to. For self-signed certificates, the certificate is its own CA, and must be provided.

This is a common concept and behavior across major vendors. And as I mentioned I fail to understand the perspective here that requires every library to implement extra code to support self signed certificates to the end application.

IngelaAndin commented 1 month ago

So my perspective is that I care about security, and following the specifications of the public_key infra structure. Now decades ago the infrastructure was no as strong and security considerations where much looser. And especially many web-sites where not so concerned about security. And as certificates infrastructure was sparse and the whole concept not trivial to understand short-cut defacto standards of allowing self-signed entity certificates emerged. But although they have become wide spread they are not good enough and we need to move away from these solutions. Security is the responsibility of everyone and hard to add afterwards you need to consider it from the start. And of course we can not disallow people from using "weak locks" but it is our responsibility not to encourage them. So if you want ta make the trade-off using end entity self-signed certificates that is up to you but I do not consider it my or OTPs responsibility to make your life easier in doing so due to legacy decisions of other frameworks. Especially not if it creates a false sense of security that later could be blamed on our product.

csrl commented 1 month ago

Self signed certificate has no technical difference in transport security. What is apparently being discussed is PKI and how a user assigns trust. I understand you are asserting that trusting public certificate authorities is "more secure" and trusting self signed certificates is "less secure". I would assert the opposite is true. Pinning a certificate (self signed or otherwise) is orders of magnitude more trustworthy than trusting a 3rd party certificate authority. From an overall ecosystem perspective certificate authorities "improve security" only in that they reduce maintenance and thus reduce the number of people configuring their client to ignore validation errors. Yet what OTP is choosing to do here is to make it more difficult to assign trust in a self signed certificate thus increasing the number of clients ignoring validation errors. I suggest this approach has the opposite of the intended affect.

I 100% agree that the default configuration should give the greatest overall security profile. Currently the default configuration results in users ignoring certificate validation for self signed certificates. That is not the moving the needle in the right direction. Additionally, as much as I think that Erlang/otp should rule the web, having OTP take the solitary stand that it will try to force the ecosystem off of self signed certificates isn't going to practically result in that.

I do disagree with your perspective of "I do not consider it my or OTPs responsibility to make your life easier". The framework should always attempt to make things easier, that is explicitly what a framework exists for, otherwise we can all write our code from scratch. I also do not understand your assertion of "it creates a false sense of security". Self signed certificates are not inherently or technically less secure. It is 100% about trust assignment. Why does having to write extra code prove intended trust? Why can't a configuration option prove intended trust?

Thank you again for taking the time to read and discuss this topic. I appreciate your willingness to do so. I don't expect I'll change your mind, so at this point I'm trying to learn if I'm missing something that is not so obvious, but so far I'm not finding what that might be.

IngelaAndin commented 1 month ago

I feel that we are talking about different things. So let me try and explain:

Lets say we have a self-signed Root CA cert lets call it RCA and we have a Server certificate that is issued by RCA lets call it EntityCert.

So RCA is is self-signed and can be said to be trusted by adding it to cacert/cacertfile option.

And when trying to connect to the server we will be able to verify the chain EntityCert - RCA ( actually when you verify it you reverse the order and call it path, but it is besides the point)

This is the minimum chain. A valid chain can also be longer EntityCert-CAN... CA2 - CA1 - RCA

However if you make a self-signed cert and say that it is your EntityCert you no longer have a certificate chain you only have one single certificate, to which the certificate path validation algorithm (that verifies the chain) is not applicable as it takes two inputs the trusted CA (RCA) that signed the first certificate in the next argument that is a list of certificates in the path starting with the one signed by the RCA and the rest of the path ending with the EntityCert,

When testing and you have control of both client and server it has been, probably still is, quite common to use a a self-signed-entity cert for it is easier for the person setting up the test. But facilitating this also facilitates it for users to download a certificate from the net and putting it in there trust store just to make it work. And this is what I do not like.

Also we provide

public_key:pkix_test_data(#{client_chain =>
   #{root => [], intermediates => [[]], peer => []},
       server_chain =>
   #{root => [], intermediates => [[]], peer => []}
      }).
#{client_config =>
      [{cert,<<48,130,1,237,48,130,1,165,160,3,2,1,2,2,1,6,48,
               16,6,7,42,134,72,206,61,...>>},
       {key,{'ECPrivateKey',<<48,81,2,1,1,4,21,0,194,33,164,91,
                              185,89,65,72,160,188,206,203,166,
                              242,...>>}},
       {cacerts,[<<48,130,1,250,48,130,1,178,160,3,2,1,2,2,1,1,
                   48,16,6,7,42,134,...>>,
                 <<48,130,2,7,48,130,1,192,160,3,2,1,2,2,1,5,48,16,6,7,42,
                   ...>>,
                 <<48,130,1,251,48,130,1,178,160,3,2,1,2,2,1,2,48,16,6,
                   7,...>>]}],
  server_config =>
      [{cert,<<48,130,2,20,48,130,1,203,160,3,2,1,2,2,1,4,48,
               16,6,7,42,134,72,206,61,...>>},
       {key,{'ECPrivateKey',<<48,81,2,1,1,4,21,0,115,200,57,48,
                              38,155,127,61,238,166,2,159,16,
                              211,...>>}},
       {cacerts,[<<48,130,1,251,48,130,1,178,160,3,2,1,2,2,1,2,
                   48,16,6,7,42,134,...>>,
                 <<48,130,2,9,48,130,1,192,160,3,2,1,2,2,1,3,48,16,6,7,42,
                   ...>>,
                 <<48,130,1,250,48,130,1,178,160,3,2,1,2,2,1,1,48,16,6,
                   7,...>>]}]}

That will generate test data that you can directly use as input to the both sides and get valid chains that can be verified as you control both sides. The empty list can be used, that is include elements so it is not empty, to customize the test data a lot if you like to test specific things, but I will not go into that here.

Granted that we could improve this function to also automatically being able to save the data to files facilitating using other implementations than ours on one of the sides. I think we should do that.

And yes it is our job to make it as easy for the users as possible without sacrificing security. I think on that we agree.