)]}'
{"ironic_python_agent/agent.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        )"},{"line_number":168,"context_line":"        self.api_url \u003d api_url"},{"line_number":169,"context_line":"        self.driver_name \u003d driver_name"},{"line_number":170,"context_line":"        self.certificate \u003d None"},{"line_number":171,"context_line":"        if self.api_url.startswith(\u0027https\u0027) and cert_fingerprint:"},{"line_number":172,"context_line":"            self.certificate \u003d self._get_certificate_if_fingerprint_matches("},{"line_number":173,"context_line":"                self.api_url, cert_fingerprint)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_fc53f414","line":170,"updated":"2016-08-30 03:59:39.000000000","message":"Need unit tests to show this is working.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        self.api_url \u003d api_url"},{"line_number":169,"context_line":"        self.driver_name \u003d driver_name"},{"line_number":170,"context_line":"        self.certificate \u003d None"},{"line_number":171,"context_line":"        if self.api_url.startswith(\u0027https\u0027) and cert_fingerprint:"},{"line_number":172,"context_line":"            self.certificate \u003d self._get_certificate_if_fingerprint_matches("},{"line_number":173,"context_line":"                self.api_url, cert_fingerprint)"},{"line_number":174,"context_line":"        self.api_client \u003d ironic_api_client.APIClient(self.api_url,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_bc8b2c67","line":171,"updated":"2016-08-30 03:59:39.000000000","message":"What if someone sets the cert_fingerprint with \u0027http\u0027? Seems like no error is raised and the user might think they are secure.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        self.standalone \u003d standalone"},{"line_number":193,"context_line":"        self.hardware_initialization_delay \u003d hardware_initialization_delay"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _get_certificate_if_fingerprint_matches(self, api_url, fingerprint):"},{"line_number":196,"context_line":"        # This will throw an exception if we can\u0027t access the host or fetch the"},{"line_number":197,"context_line":"        # certificate, which means that the agent has been misconfigured."},{"line_number":198,"context_line":"        certificate \u003d utils.fetch_certificate(api_url)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_1910aa58","line":195,"updated":"2016-08-30 03:59:39.000000000","message":"Need unit tests\n\nShould probably document that this raises RuntimeError()\n\nAlso, this seems like it could be in utils.py. As I don\u0027t think it needs to be a class/object method.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        # certificate, which means that the agent has been misconfigured."},{"line_number":198,"context_line":"        certificate \u003d utils.fetch_certificate(api_url)"},{"line_number":199,"context_line":"        utils.verify_fingerprint(certificate, fingerprint)"},{"line_number":200,"context_line":"        return self.dump_cert_in_temp_file(certificate)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def get_status(self):"},{"line_number":203,"context_line":"        \"\"\"Retrieve a serializable status."}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_7c128458","line":200,"updated":"2016-08-30 03:59:39.000000000","message":"Does this work? This function doesn\u0027t seem to be part of the class. It is in the utils.py file.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"}],"ironic_python_agent/config.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"44c2227dbf9e69baee5f8c44a811ff7dda94cc2f","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    \u0027to check if at least one suitable disk has appeared \u0027"},{"line_number":148,"context_line":"                    \u0027in inventory.\u0027),"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    cfg.StrOpt(\u0027ipa_server_cert_fingerprint\u0027,"},{"line_number":151,"context_line":"               default\u003dAPARAMS.get(\u0027ipa-cert-fingerprint\u0027),"},{"line_number":152,"context_line":"               help\u003d(\"Fingerprint of the IPA server\u0027s certificate as a hex \""},{"line_number":153,"context_line":"                     \"string\")),"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_9a50f40c","line":150,"range":{"start_line":150,"start_character":16,"end_line":150,"end_character":27},"updated":"2016-08-23 16:40:49.000000000","message":"s/ipa_server_// for consistency (we only add ipa- prefix to kernel arguments)","commit_id":"6ad7db475d75281ee5a4f46e516c24e3d8765749"}],"ironic_python_agent/ironic_api_client.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            \u0027Accept\u0027: \u0027application/json\u0027,"},{"line_number":66,"context_line":"        })"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if self.certificate:"},{"line_number":69,"context_line":"            kwargs[\u0027verify\u0027] \u003d self.certificate"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        return self.session.request(method,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_7c67e4e8","line":68,"updated":"2016-08-30 03:59:39.000000000","message":"Unit tests please.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"}],"ironic_python_agent/utils.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"44c2227dbf9e69baee5f8c44a811ff7dda94cc2f","unresolved":false,"context_lines":[{"line_number":440,"context_line":"def is_equal(a, b):"},{"line_number":441,"context_line":"    \"\"\"Check if two strings are equal."},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    checking for the certificate hash using \u003d\u003d is a security vulnerability due"},{"line_number":444,"context_line":"    to the possibility of timing attacks. We instead use a time-constant"},{"line_number":445,"context_line":"    function to check the length."},{"line_number":446,"context_line":"    \"\"\""},{"line_number":447,"context_line":"    if len(a) !\u003d len(b):"},{"line_number":448,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_67064521","line":445,"range":{"start_line":443,"start_character":0,"end_line":445,"end_character":33},"updated":"2016-08-23 16:40:49.000000000","message":"Do we really need to care, provided that both fingerprints are publicly available?","commit_id":"6ad7db475d75281ee5a4f46e516c24e3d8765749"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"f89fa3c8bbd5098931a9fb9ddf4cf01cf3d40991","unresolved":false,"context_lines":[{"line_number":440,"context_line":"def is_equal(a, b):"},{"line_number":441,"context_line":"    \"\"\"Check if two strings are equal."},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    checking for the certificate hash using \u003d\u003d is a security vulnerability due"},{"line_number":444,"context_line":"    to the possibility of timing attacks. We instead use a time-constant"},{"line_number":445,"context_line":"    function to check the length."},{"line_number":446,"context_line":"    \"\"\""},{"line_number":447,"context_line":"    if len(a) !\u003d len(b):"},{"line_number":448,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_81fb4d78","line":445,"range":{"start_line":443,"start_character":0,"end_line":445,"end_character":33},"in_reply_to":"1ac06dbe_67064521","updated":"2016-08-24 05:26:18.000000000","message":"It\u0027s pretty standard to compare fingerprints this way to prevent an attacker guessing which would be a valid fingerprint based on timing. But in this case, being that it would probably be in a private network and it may not be that necessary and kind of overkill.","commit_id":"6ad7db475d75281ee5a4f46e516c24e3d8765749"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"44c2227dbf9e69baee5f8c44a811ff7dda94cc2f","unresolved":false,"context_lines":[{"line_number":468,"context_line":"    h \u003d hashlib.sha256()"},{"line_number":469,"context_line":"    cert_der \u003d ssl.PEM_cert_to_DER_cert(certificate)"},{"line_number":470,"context_line":"    h.update(cert_der)"},{"line_number":471,"context_line":"    return is_equal(h.hexdigest(), fingerprint)"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"def dump_cert_in_temp_file(certificate):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_a7667d27","line":471,"range":{"start_line":471,"start_character":4,"end_line":471,"end_character":47},"updated":"2016-08-23 16:40:49.000000000","message":"I think the exception should be here, and please include both fingerprints in it","commit_id":"6ad7db475d75281ee5a4f46e516c24e3d8765749"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"f89fa3c8bbd5098931a9fb9ddf4cf01cf3d40991","unresolved":false,"context_lines":[{"line_number":468,"context_line":"    h \u003d hashlib.sha256()"},{"line_number":469,"context_line":"    cert_der \u003d ssl.PEM_cert_to_DER_cert(certificate)"},{"line_number":470,"context_line":"    h.update(cert_der)"},{"line_number":471,"context_line":"    return is_equal(h.hexdigest(), fingerprint)"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"def dump_cert_in_temp_file(certificate):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_c148a5ce","line":471,"range":{"start_line":471,"start_character":4,"end_line":471,"end_character":47},"in_reply_to":"1ac06dbe_a7667d27","updated":"2016-08-24 05:26:18.000000000","message":"So you mean that I should raise the exception here and include both fingerprints in the exception\u0027s message? why?","commit_id":"6ad7db475d75281ee5a4f46e516c24e3d8765749"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":429,"context_line":"    :param url: The url to the server whose certificate will be fetched."},{"line_number":430,"context_line":"    :returns: a string containing the certificate in PEM format."},{"line_number":431,"context_line":"    \"\"\""},{"line_number":432,"context_line":"    parsed_url \u003d parse.urlparse(url)"},{"line_number":433,"context_line":"    if not parsed_url.port:"},{"line_number":434,"context_line":"        port \u003d 443"},{"line_number":435,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_5904f248","line":432,"updated":"2016-08-30 03:59:39.000000000","message":"I wonder if we care if they pass \u0027http\u0027/\u0027ftp\u0027/\u0027foo\u0027 ? Is it okay that it gets automatically converted to \u0027https\u0027.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":431,"context_line":"    \"\"\""},{"line_number":432,"context_line":"    parsed_url \u003d parse.urlparse(url)"},{"line_number":433,"context_line":"    if not parsed_url.port:"},{"line_number":434,"context_line":"        port \u003d 443"},{"line_number":435,"context_line":"    else:"},{"line_number":436,"context_line":"        port \u003d parsed_url.port"},{"line_number":437,"context_line":"    return ssl.get_server_certificate((parsed_url.hostname, port))"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_59ddd2de","line":434,"updated":"2016-08-30 03:59:39.000000000","message":"Nit, magic number, though most people know it.\n\n port \u003d httplib.HTTPS_PORT\n\nProbably have to use the six library to get this value.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    return ssl.get_server_certificate((parsed_url.hostname, port))"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"def is_equal(a, b):"},{"line_number":441,"context_line":"    \"\"\"Check if two strings are equal."},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    checking for the certificate hash using \u003d\u003d is a security vulnerability due"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_99475abb","line":440,"updated":"2016-08-30 03:59:39.000000000","message":"If this function remains, it needs some unit tests.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":7080,"name":"Mathieu Mitchell","email":"mat128@gmail.com","username":"mat128"},"change_message_id":"3751c1891c46d34579f87d83d0b213754ef46a74","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    return ssl.get_server_certificate((parsed_url.hostname, port))"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"def is_equal(a, b):"},{"line_number":441,"context_line":"    \"\"\"Check if two strings are equal."},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    checking for the certificate hash using \u003d\u003d is a security vulnerability due"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a98d1f4_5b73634d","line":440,"in_reply_to":"fa7ab95a_99475abb","updated":"2016-09-29 20:03:12.000000000","message":"+1. It also needs a different name if we are comparing equality by doing something else. Right now, this feels like a wrapper around `return a \u003d\u003d b`.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"8655b2e8b1983c05135b760596283a4770686c61","unresolved":false,"context_lines":[{"line_number":440,"context_line":"def is_equal(a, b):"},{"line_number":441,"context_line":"    \"\"\"Check if two strings are equal."},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    checking for the certificate hash using \u003d\u003d is a security vulnerability due"},{"line_number":444,"context_line":"    to the possibility of timing attacks. We instead use a time-constant"},{"line_number":445,"context_line":"    function to check the length."},{"line_number":446,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_39f64ef9","line":443,"updated":"2016-08-30 03:59:39.000000000","message":"I\u0027m failing to understand how in the case of Ironic Python Agent using \u0027\u003d\u003d\u0027 would be a security vulnerability.\n\nCan you explain why?\n\nIsn\u0027t the certificate public information?","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":7080,"name":"Mathieu Mitchell","email":"mat128@gmail.com","username":"mat128"},"change_message_id":"3751c1891c46d34579f87d83d0b213754ef46a74","unresolved":false,"context_lines":[{"line_number":463,"context_line":"    :param certificate: a string containing the certificate in PEM format."},{"line_number":464,"context_line":"    :param fingerprint: a string containing the SHA256 fingerprint we want to"},{"line_number":465,"context_line":"                        verify against."},{"line_number":466,"context_line":"    :returns: True if the verification passed or False if it didn\u0027t."},{"line_number":467,"context_line":"    \"\"\""},{"line_number":468,"context_line":"    h \u003d hashlib.sha256()"},{"line_number":469,"context_line":"    cert_der \u003d ssl.PEM_cert_to_DER_cert(certificate)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a98d1f4_1b92bb57","line":466,"updated":"2016-09-29 20:03:12.000000000","message":"This is not true. It will raise if the fingerprint does not match.","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"},{"author":{"_account_id":7080,"name":"Mathieu Mitchell","email":"mat128@gmail.com","username":"mat128"},"change_message_id":"3751c1891c46d34579f87d83d0b213754ef46a74","unresolved":false,"context_lines":[{"line_number":480,"context_line":"    \"\"\"Dumps the given certificate in a temporary file."},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    The given certificate string is written into a temporary file."},{"line_number":483,"context_line":"    Note that we set delete\u003dFalse for the NamedTemporaryFile because we will"},{"line_number":484,"context_line":"    read this certificate for validating the SSL connection."},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    :param certificate: a string containing the certificate in PEM format."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a98d1f4_db869312","line":483,"updated":"2016-09-29 20:03:12.000000000","message":"Then who is responsible for deleting it?","commit_id":"3f1c4129bb967900422fe61ff8e91aaadd618418"}]}