public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Beat Bolli <dev+git@drbeat•li>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Oswald Buddenhagen <oswald.buddenhagen@gmx•de>
Subject: Re: [PATCH 3/4] imap-send: remove two string length checks
Date: Wed, 11 Mar 2026 23:00:25 +0100	[thread overview]
Message-ID: <d9338d30-3e73-4cb7-9f8f-cc1ce21c9de4@drbeat.li> (raw)
In-Reply-To: <xmqq4immp56h.fsf@gitster.g>

Hi Junio

On 11.03.2026 19:55, Junio C Hamano wrote:
> Beat Bolli <dev+git@drbeat•li> writes:
> 
>> At this point, these two checks verify that the ASN1_STRINGs are
>> internally consistent. This may have been ok when the fields were
>> accessed directly, but now that the API is used, is unnecessary.
>>
>> Remove the two checks.
> 
> Oswald already gave a similar comment, but
> 
>   * I am not sure what you meant by "ok" in "may have been ok".  Do
>     you mean "with raw access to the fields, it may have been made
>     send to ensure validity of ASN1_STRING"?
> 
>   * I am also not sure what you meant by "now that the API is used".
>     Who in the code uses which API function so that we do not have to
>     do our sanity checking?
> 
>     The call to host_matches() that these extra checks protect are
>     still passing raw "const char *" in this step, and the change to
>     pass ASN1_STRING does not happen until [4/4], so you did not mean
>     host_matches().  I am not sure what it is.
>     
> Thanks.

I didn't realize that the strlen() comparison was meant to check for 
embedded NULs. I'll send v2 shortly that keeps this check.

Cheers, Beat


>> Signed-off-by: Beat Bolli <dev+git@drbeat•li>
>> ---
>>   imap-send.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/imap-send.c b/imap-send.c
>> index 2a904314dd..2bb0003f08 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -253,8 +253,6 @@ static int verify_hostname(X509 *cert, const char *hostname)
>>   			ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype);
>>   
>>   			if (ntype == GEN_DNS &&
>> -			    strlen((const char *)ASN1_STRING_get0_data(subj_alt_str)) ==
>> -				    ASN1_STRING_length(subj_alt_str) &&
>>   			    host_matches(hostname, (const char *)ASN1_STRING_get0_data(subj_alt_str)))
>>   				found = 1;
>>   		}
>> @@ -270,8 +268,7 @@ static int verify_hostname(X509 *cert, const char *hostname)
>>   	    (cname_entry = X509_NAME_get_entry(subj, i)) == NULL ||
>>   	    (cname = X509_NAME_ENTRY_get_data(cname_entry)) == NULL)
>>   		return error("cannot get certificate common name");
>> -	if (strlen((const char *)ASN1_STRING_get0_data(cname)) == ASN1_STRING_length(cname) &&
>> -	    host_matches(hostname, (const char *)ASN1_STRING_get0_data(cname)))
>> +	if (host_matches(hostname, (const char *)ASN1_STRING_get0_data(cname)))
>>   		return 0;
>>   	return error("certificate owner '%s' does not match hostname '%s'",
>>   		     ASN1_STRING_get0_data(cname), hostname);


  reply	other threads:[~2026-03-11 22:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 12:11 [PATCH 0/4] imap-send: modernize the OpenSSL API Beat Bolli
2026-03-11 12:11 ` [PATCH 1/4] imap-send: use the OpenSSL API to access the subject alternative names Beat Bolli
2026-03-11 12:11 ` [PATCH 2/4] imap-send: use the OpenSSL API to access the subject common name Beat Bolli
2026-03-11 12:11 ` [PATCH 3/4] imap-send: remove two string length checks Beat Bolli
2026-03-11 13:41   ` Oswald Buddenhagen
2026-03-11 21:49     ` Beat Bolli
2026-03-11 18:55   ` Junio C Hamano
2026-03-11 22:00     ` Beat Bolli [this message]
2026-03-11 12:11 ` [PATCH 4/4] imap-send: refactor function host_matches() Beat Bolli
2026-03-11 22:10 ` [PATCH v2 0/3] imap-send: modernize the OpenSSL API Beat Bolli
2026-03-12  0:25   ` Junio C Hamano
2026-03-11 22:10 ` [PATCH v2 1/3] imap-send: use the OpenSSL API to access the subject alternative names Beat Bolli
2026-03-11 22:10 ` [PATCH v2 2/3] imap-send: use the OpenSSL API to access the subject common name Beat Bolli
2026-03-11 22:10 ` [PATCH v2 3/3] imap-send: move common code into function host_matches() Beat Bolli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9338d30-3e73-4cb7-9f8f-cc1ce21c9de4@drbeat.li \
    --to=dev+git@drbeat$(echo .)li \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=oswald.buddenhagen@gmx$(echo .)de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox