> On March 28, 2017, 11:57 p.m., Robert Levas wrote: > > ambari-common/src/main/python/ambari_commons/network.py > > Lines 65-66 (original), 64-65 (patched) > > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66> > > > > Maybe there should be a flag so that the caller of this can indicate > > whether they want this to fail or not. It may be desirable to not allow > > the connection if the server is not trusted. > > > > Also, this function will return `ssl.PROTOCOL_SSLv23` > > no matter what, so the caller has no indication that an error may have > > occurred. > > Aravindan Vijayan wrote: > I will make the check certificate step configurable. > > Sid Wagle wrote: > I am not sure about making cert check failure/sucess configurable. It > just sounds like an overkill. Why not check and warn if there is an issue > with the check. The daemon launching with sucess or failure will in any case > determine if the cert is valid and configured correctly. There is effectively > no stack script that does cert checks like this :-) > > Robert Levas wrote: > The daemon (serve-side of this equation?) cannot be trusted to to verify > it's own cert, this is why the client should do it. I think (basically) > silently warning the user the server cannot be trusted can be a problem in > some situations, depending on the level of paranoia a user may have. And if a > user has gone through the trouble of getting a _real_ SSL certificate (as > opposed to a self-signed SSL certificate), then they probably want to know if > the server's cert is not valid.
IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use. - Aravindan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58006/#review170350 ----------------------------------------------------------- On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58006/ > ----------------------------------------------------------- > > (Updated March 28, 2017, 10:03 p.m.) > > > Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid > Wagle. > > > Bugs: AMBARI-20600 > https://issues.apache.org/jira/browse/AMBARI-20600 > > > Repository: ambari > > > Description > ------- > > EXCEPTION TRACE > > File > "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", > line 235, in create_grafana_admin_pwd > response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1) > File > "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", > line 59, in perform_grafana_get_call > grafana_https_enabled, ca_certs) > File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, > in get_http_connection > ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, > ca_certs) > File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, > in check_ssl_certificate_and_return_ssl_version > .format(host, port, ca_certs, str(ssl_error))) > resource_management.core.exceptions.Fail: Failed to verify the SSL > certificate for https://<host>:3000 with CA certificate in > /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL > routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed > > > PROBLEM > The Grafana util script makes HTTPS calls with the server endpoint to create > datasource, dashboards etc. For this call, it validates the server's > certificate with the CA certificate using the > https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. > This call checks the certificate validity against a root certificate list. > The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used > both by the Grafana server to start up in HTTPS as well as in this validation > step if the cert file is not a leaf certificate (for example a self signed > certificate). If there is a CA which issued the certificate for Grafana > HTTPS, then the ca bundle must be used to validate the server's certificate. > > FIX > Added a new parameter that takes in the ca_cert, defaulting to the cert file. > Grafana start should not fail if we are not able to validate the certificate, > but able to make HTTPS calls to the server. We will print out a warning > statement instead. > > > Diffs > ----- > > ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml > b4570b7 > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py > a6a9779 > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py > 3276cc1 > > > Diff: https://reviews.apache.org/r/58006/diff/1/ > > > Testing > ------- > > Manually tested. > Python unit tests passed. > > > Thanks, > > Aravindan Vijayan > >
