Thanks for the changes. I reviewed the git commits and this is exactly what you want to be doing. I do have some comments: crypto_cert_subject_common_name(): * Need to verify subject_name != NULL * Need to verify index != -1 * Need to verify entry != NULL * Need to verify entry_data != NULL * Need to verify length is not < 0 * Need to verify common_name does not have embedded '\0' characters be verifying the length of common_name with ASN1_STRING_length
Similar checks need to be done in crypto_cert_subject_alt_name() (ie, check return codes for *everything*). >From man ASN1_STRING_data: "In general it cannot be assumed that the data returned by ASN1_STRING_data() is null terminated or does not contain embedded nulls." As such you should: a) make sure that the strings you are returning in crypto_cert_subject_common_name() and crypto_cert_subject_alt_name() are NULL terminated, otherwise the strcmp()s you do elsewhere on these can cause you trouble. b) leverage the fact that differences between ASN1_STRING_length and a (properly terminated) strlen means that you have embedded NULLs. Why are embedded NULLs bad? Consider if an attacker used this for the common name: CN=rdp.foo.com\0mitm.attacker.org\0 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/925657 Title: [precise] freerdp does not check the server's hostname when verifying ssl certificates To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/freerdp/+bug/925657/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
