openbmc / phosphor-certificate-manager

Apache License 2.0
4 stars 4 forks source link

bmcweb crashes on loading self-signed certificate with key-length 512 #17

Closed prkatti1 closed 2 years ago

prkatti1 commented 3 years ago

Describe the bug bmcweb crashes on loading self-signed certificate with key-length 512s.

To Reproduce Steps to reproduce the behavior:

  1. Run the command to generate 512 bit pvt key and generate certificate
  2. use the following command ```openssl req -newkey rsa:512 \ -x509 \ -sha256 \ -days 3650 \ -nodes \ -out example.crt \ -keyout example.key
  3. load the certificate+pvt key via GUI, bmcweb is crashing and dead permanently and not recovering

logs for the same is:


* Apr 16 08:30:48 witherspoon systemd[1]: Reloaded Start bmcweb server.
Apr 16 08:33:30 witherspoon phosphor-certificate-manager[150]: Certificate install
Apr 16 08:33:30 witherspoon phosphor-certificate-manager[150]: Certificate loadCert
Apr 16 08:33:30 witherspoon phosphor-certificate-manager[150]: Certificate verification failed
Apr 16 08:33:30 witherspoon phosphor-certificate-manager[150]: Certificate compareKeys
Apr 16 08:33:30 witherspoon phosphor-certificate-manager[150]: Certificate loadCert
Apr 16 08:33:30 witherspoon systemd[1]: Reloading Start bmcweb server.
Apr 16 08:33:30 witherspoon systemd[1]: Reloaded Start bmcweb server.
Apr 16 08:33:31 witherspoon bmcweb[405]: terminate called after throwing an instance of 'boost::wrapexcept<boost::system::system_error>'
Apr 16 08:33:31 witherspoon bmcweb[405]:   what():  use_certificate_file: asio.ssl error
Apr 16 08:33:31 witherspoon systemd[1]: bmcweb.service: Main process exited, code=killed, status=6/ABRT
Apr 16 08:33:31 witherspoon systemd[1]: bmcweb.service: Failed with result 'signal'.
Apr 16 08:33:31 witherspoon systemd[1]: Started Start bmcweb server.
Apr 16 08:33:32 witherspoon bmcweb[5471]: terminate called after throwing an instance of 'boost::wrapexcept<boost::system::system_error>'
Apr 16 08:33:32 witherspoon bmcweb[5471]:   what():  use_certificate_file: asio.ssl error
Apr 16 08:33:32 witherspoon systemd[1]: bmcweb.service: Main process exited, code=killed, status=6/ABRT
Apr 16 08:33:33 witherspoon systemd[1]: bmcweb.service: Failed with result 'signal'.
Apr 16 08:33:33 witherspoon systemd[1]: Started Start bmcweb server.
Apr 16 08:33:34 witherspoon bmcweb[5473]: terminate called after throwing an instance of 'boost::wrapexcept<boost::system::system_error>'
Apr 16 08:33:34 witherspoon bmcweb[5473]:   what():  use_certificate_file: asio.ssl error
Apr 16 08:33:34 witherspoon systemd[1]: bmcweb.service: Main process exited, code=killed, status=6/ABRT
Apr 16 08:33:34 witherspoon systemd[1]: bmcweb.service: Failed with result 'signal'.
Apr 16 08:33:34 witherspoon systemd[1]: bmcweb.service: Start request repeated too quickly.
Apr 16 08:33:34 witherspoon systemd[1]: bmcweb.service: Failed with result 'signal'.
Apr 16 08:33:34 witherspoon systemd[1]: Failed to start Start bmcweb server.
Apr 16 08:33:34 witherspoon systemd[1]: bmcweb.socket: Failed with result 'service-start-limit-hit'.
edtanous commented 3 years ago

First off, this seems to be an instance of common error number 5.

use_certificate_file has an EC based overload that we should be using here. The question then becomes, should bmcweb do with the error? We've been presented with a certificate that phosphor-certificate-manager accepted, but is not able to be used to open a TLS session. I suspect we're doing the right thing by crashing. The changes I suspect need to happen here are:

  1. Move bmcweb to the ec version of the use_certificate_file method. It will still result in a crash (because we can no longer fulfill our goal of providing a webserver) but hopefully we can print something more useful.
  2. phosphor-certificate-manager needs more appropriate checks to ensure the validity of the SSL certificate on upload, to ensure the certificates have the correct properties to be used within the websever.

Considering 2 is the bulk of the work, and bmcweb is arguably doing the right thing here (despite the coding style issue) I'm going to transfer this to phosphor-certificate-manager. @ojayanth @devenrao Would you two please look into this?

ojayanth commented 3 years ago

@edtanous We have pretty good discussion on the phosphor-certificate-manager scope on certificate validation during initial development time . https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-certificate-manager/+/13364 provides details on this . The design direction team agreed was to minimize the certificate fields validation during upload process and based on the application settings/security level appropriate additional validation can handle.

In this case BMCweb crashes without providing valid reason seems to be not correct. If the error is related ec_version, BMC web should report error and handling as normal BMC web certificate validation?

edtanous commented 3 years ago

@edtanous We have pretty good discussion on the phosphor-certificate-manager scope on certificate validation during initial development time . https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-certificate-manager/+/13364 provides details on this .

Given that this bug exists, clearly not enough validation was done. Please re-assess your design.

The design direction team agreed was to minimize the certificate fields validation during upload process and based on the application settings/security level appropriate additional validation can handle.

I'm not really following, are you saying that we should just allow the crash?

In this case BMCweb crashes without providing valid reason seems to be not correct.

What would you suggest doing instead? A valid certificate is not available, so there's nothing bmcweb can do to start up.

If the error is related ec_version,

I'm not sure what "ec version" means. The error is related to the fact that the certificate cannot be used to start an ssl context, yet phosphor-certificate-manager accepted it for SSL related activites.

BMC web should report error and handling as normal BMC web certificate validation?

Not following what you mean here. bmcweb did "report the error" in the form of an exception, but because it's an unrecoverable error, there's really nothing bmcweb can do at that point.

ojayanth commented 3 years ago

@edtanous We have pretty good discussion on the phosphor-certificate-manager scope on certificate validation during initial development time . https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-certificate-manager/+/13364 provides details on this .

Given that this bug exists, clearly not enough validation was done. Please re-assess your design.

The design direction team agreed was to minimize the certificate fields validation during upload process and based on the application settings/security level appropriate additional validation can handle.

I'm not really following, are you saying that we should just allow the crash?

In this case BMCweb crashes without providing valid reason seems to be not correct.

What would you suggest doing instead? A valid certificate isn't not available, so there's nothing bmcweb can do to start up.

If the error is related ec_version,

I'm not sure what "ec version" means. The error is related to the fact that the certificate cannot be used to start an ssl context, yet phosphor-certificate-manager accepted it for SSL related activites.

BMC web should report error and handling as normal BMC web certificate validation?

Not following what you mean here. bmcweb did "report the error" in the form of an exception, but because it's an unrecoverable error, there's really nothing bmcweb can do at that point.

Referring https://github.ibm.com/openbmc/bmcweb/blob/1020/include/ssl_key_handler.hpp#L347. Is this check happens prior to loading the certificate?

edtanous commented 3 years ago

The link you posted is to a private IBM instance. Can you link to the open source github please?

ojayanth commented 3 years ago

https://github.com/openbmc/bmcweb/blob/master/include/ssl_key_handler.hpp#L348

edtanous commented 3 years ago

Those do some basic sanity checks. The actual check that's failing is this one:

https://github.com/openbmc/bmcweb/blob/b295bf951f391380e60234d0fe6df7ad4f5b00c9/include/ssl_key_handler.hpp#L379

Which I suspect is because openssl rejects keys below a certain length?

ojayanth commented 3 years ago

Openssl command line working good.

openssl verify server.pem xxxxxx error 18 at 0 depth lookup:self signed certificate OK

Error 18 “18 X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT: self signed certificate the passed certificate is self signed and the same certificate cannot be found in the list of trusted certificates.”

openssl x509 -in server.pem -text -noout | grep "Public-Key" Public-Key: (512 bit)

ojayanth commented 2 years ago

@msnidhin patch https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-certificate-manager/+/43189 fixed this issue.