Replace BIO_printf with ASN1_STRING_print in GENERAL_NAME_print#9101
Replace BIO_printf with ASN1_STRING_print in GENERAL_NAME_print#9101acheevbhagat wants to merge 1 commit intoopenssl:masterfrom acheevbhagat:master
Conversation
| case GEN_EMAIL: | ||
| BIO_printf(out, "email:%s", gen->d.ia5->data); | ||
| BIO_printf(out, "email:"); | ||
| ASN1_STRING_print(out, gen->d.ia5); |
There was a problem hiding this comment.
whitespace error: replace tab by four spaces
There was a problem hiding this comment.
correction: replace tab bei eight spaces (three locations)
| case GEN_DNS: | ||
| BIO_printf(out, "DNS:%s", gen->d.ia5->data); | ||
| BIO_printf(out, "DNS:"); | ||
| ASN1_STRING_print(out, gen->d.ia5); |
There was a problem hiding this comment.
whitespace error: replace tab by four spaces
| case GEN_URI: | ||
| BIO_printf(out, "URI:%s", gen->d.ia5->data); | ||
| BIO_printf(out, "URI:"); | ||
| ASN1_STRING_print(out, gen->d.ia5); |
There was a problem hiding this comment.
whitespace error: replace tab by four spaces
|
@acheevbhagat To give a hint how to do the fixups - just use |
t-j-h
left a comment
There was a problem hiding this comment.
Approved with white space fixes to be handled during commit (or not as your choice).
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from #9101)
|
Merged. Thank you @acheevbhagat for your contribution! bab6046 Replace BIO_printf with ASN1_STRING_print in GENERAL_NAME_print [master] |
|
@mspncp I think this should also go into 1.0.2 - it is a security related issue - and it applies there as well. |
Ok, I can do it. I already verified that commit bab6046 cherry-picks to 1.0.2 without conflicts. But before pushing it, I'd like to wait 24 hours to give other @openssl/omc members the opportunity to express their opinions about whether the change qualifies as security fix or not. |
|
Why is this a security fix? |
|
Any application using the print function to compare names in certificates will match incorrectly if there is an embedded 0x00 in the ia5string. And human looking at the values will see the wrong information. |
|
Ok then. |
|
Given two OMC approvals for the cherry-pick, I'll reduce the waiting time to 6 hours, i.e., I'll do it this evening. |
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from #9101)
|
And finally, merged to 1.0.2 8479e9e Replace BIO_printf with ASN1_STRING_print in GENERAL_NAME_print [1.0.2] |
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from openssl#9101) (cherry picked from commit bab6046)
Fixes #8997
Checklist