<div dir="ltr">Got it, thank you Matt.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 28, 2022 at 6:29 PM Matt Caswell <<a href="mailto:matt@openssl.org">matt@openssl.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 28/03/2022 13:11, Brahmaji K wrote:<br>
> Hi Team,<br>
> <br>
> I'm trying to store the invalid EC certificate as a negative test for my <br>
> application. My application calls the X509_STORE_load_locations() to <br>
> load the certificate from a specific path. For invalid EC certificate it <br>
> is expected to FAIL but it is returning the SUCCESS.<br>
> <br>
> I have done some debugging and found the following:<br>
> <br>
> pubkey_cb() calls x509_pubkey_decode(), the x509_pubkey_decode() has the <br>
> check only for -1 as shown below:<br>
> <br>
> 46 if (x509_pubkey_decode(&pubkey->pkey, pubkey) == -1)<br>
> 47 return 0;<br>
> <br>
> But x509_pubkey_decode() can return zero ("0") also in the failure case. <br>
> Is there any intention to have the above check? or is it a known issue?<br>
<br>
The fuller context around these lines gives the answer:<br>
<br>
/*<br>
* Opportunistically decode the key but remove any non fatal errors<br>
* from the queue. Subsequent explicit attempts to decode/use <br>
the key<br>
* will return an appropriate error.<br>
*/<br>
ERR_set_mark();<br>
if (x509_pubkey_decode(&pubkey->pkey, pubkey) == -1)<br>
return 0;<br>
ERR_pop_to_mark();<br>
<br>
The -1 return from x509_pubkey_decode() indicates a fatal error (e.g. a <br>
malloc failure). A 0 error return is considered non-fatal (e.g. an <br>
unrecognised key) and the code is deliberately written to continue in <br>
the case of non-fatal errors.<br>
<br>
Matt<br>
<br>
<br>
<br>
> <br>
> Call trace for more information:<br>
> #0 x509_pubkey_decode (ppkey=ppkey@entry=0x123d5ffd0,<br>
> key=key@entry=0x123d5ffc0) at crypto/x509/x_pubkey.c:125<br>
> #1 0x00000001201f5888 in pubkey_cb (operation=operation@entry=5,<br>
> pval=pval@entry=0x123d5fe40, it=it@entry=0x1206026c8,<br>
> exarg=exarg@entry=0x0) at crypto/x509/x_pubkey.c:46<br>
> #2 0x0000000120152bac in asn1_item_embed_d2i (pval=pval@entry=0x123d5fe40,<br>
> in=in@entry=0xffff654288, len=0, it=0x1206026c8, tag=<optimized out>,<br>
> tag@entry=-1, aclass=<optimized out>, aclass@entry=0,<br>
> opt=<optimized out>, ctx=ctx@entry=0xffff6546a0, depth=<optimized out>,<br>
> depth@entry=2) at crypto/asn1/tasn_dec.c:413<br>
> #3 0x0000000120153660 in asn1_template_noexp_d2i (val=0x123d5fe40,<br>
> in=0xffff6543a0, len=322, tt=0x120618ad8, opt=<optimized out>,<br>
> ctx=0xffff6546a0, depth=<optimized out>) at crypto/asn1/tasn_dec.c:624<br>
> #4 0x0000000120153968 in asn1_template_ex_d2i (val=0x123d5fe40,<br>
> in=in@entry=0xffff6543a0, inlen=<optimized out>, <br>
> tt=tt@entry=0x120618ad8,<br>
> opt=<optimized out>, ctx=ctx@entry=0xffff6546a0, depth=depth@entry=2)<br>
> at crypto/asn1/tasn_dec.c:499<br>
> #5 0x0000000120153064 in asn1_item_embed_d2i (pval=pval@entry=0xffff654490,<br>
> in=in@entry=0xffff654488, len=322, it=0x1206027f8, tag=<optimized out>,<br>
> tag@entry=-1, aclass=<optimized out>, aclass@entry=0,<br>
> opt=<optimized out>, ctx=ctx@entry=0xffff6546a0, depth=2, <br>
> depth@entry=1)<br>
> at crypto/asn1/tasn_dec.c:363<br>
> #6 0x0000000120153660 in asn1_template_noexp_d2i (val=0xffff654490,<br>
> in=0xffff6545a0, len=507, tt=0x120618970, opt=<optimized out>,<br>
> ctx=0xffff6546a0, depth=<optimized out>) at crypto/asn1/tasn_dec.c:624<br>
> #7 0x0000000120153968 in asn1_template_ex_d2i (val=0x123d5fdf0,<br>
> in=in@entry=0xffff6545a0, inlen=<optimized out>, <br>
> tt=tt@entry=0x120618970,<br>
> opt=<optimized out>, ctx=ctx@entry=0xffff6546a0, depth=depth@entry=1)<br>
> at crypto/asn1/tasn_dec.c:499<br>
> #8 0x0000000120153064 in asn1_item_embed_d2i (pval=pval@entry=0x123d5ef40,<br>
> in=0xffff654710, len=507, it=it@entry=0x1206027c0, tag=<optimized out>,<br>
> aclass=<optimized out>, opt=<optimized out>, ctx=0xffff6546a0, depth=1,<br>
> depth@entry=0) at crypto/asn1/tasn_dec.c:363<br>
> #9 0x0000000120153ac8 in ASN1_item_ex_d2i (pval=0x123d5ef40,<br>
> in=<optimized out>, len=<optimized out>, it=0x1206027c0,<br>
> tag=<optimized out>, aclass=<optimized out>, opt=<optimized out>,<br>
> ctx=<optimized out>) at crypto/asn1/tasn_dec.c:124<br>
> #10 0x0000000120153b60 in ASN1_item_d2i (pval=0x123d5ef40,<br>
> in=<optimized out>, len=<optimized out>, it=<optimized out>)<br>
> at crypto/asn1/tasn_dec.c:114<br>
> #11 0x00000001202cd744 in PEM_X509_INFO_read_bio (bp=0x123d5ee50, sk=0x0,<br>
> cb=0, u=0x1204ca0c0) at crypto/pem/pem_info.c:195<br>
> #12 0x0000000120285fe8 in X509_load_cert_crl_file (file=<optimized out>,<br>
> ctx=0x123d5f3c0, type=<optimized out>) at crypto/x509/by_file.c:202<br>
> #13 X509_load_cert_crl_file (ctx=0x123d5f3c0, file=<optimized out>,<br>
> type=<optimized out>) at crypto/x509/by_file.c:188<br>
> #14 0x00000001202861c8 in by_file_ctrl (ctx=<optimized out>,<br>
> cmd=<optimized out>, argp=<optimized out>, argl=<optimized out>,<br>
> ret=<optimized out>) at crypto/x509/by_file.c:64<br>
> #15 0x00000001201e60ec in X509_STORE_load_locations (ctx=0x123d5f2c0,<br>
> file=0xffff654868 "/certs/test.crt", path=0x0)<br>
> at crypto/x509/x509_d2.c:44<br>
</blockquote></div>