On 4/2/20 1:28 AM, Arne Schwabe wrote:
> Am 01.04.20 um 23:50 schrieb w...@torlan.ru:
>> From: Maxim Plotnikov <w...@torlan.ru>
>>
>> Lack of this led people accepting multiple CAs to use capath,
>> which already supports multiple CRLs. But capath mode itself
>> is somewhat ugly: you have to create new file/symlink every time
>> CRL is updated, and there's no good way to clean them up without
>> restarting OpenVPN, since any gap in the sequence would cause it
>> to lose sync[1].
>>
>> mbedtls crypto backends already loads multiple CRLs as is, so
>> it doesn't need this fix.
>>
>> The patch also includes some logging changes which I think are useful.
>
> Feature-ACK but some changes to the code required.
Thank you for the feedback! How should I send a new patch version? As a reply
(with [PATCH v2]) to this thread, or as an independent message? I'm new to
contributing patches through e-mail.
>> + X509_CRL *crl = PEM_read_bio_X509_CRL(in, NULL, NULL, NULL);
>> + if (crl == NULL)
>> + {
>> + unsigned long err = ERR_get_error();
>
> I think we should use ERR_peek_error here, so we do not drop the error
> message if the error is something else.
backend_tls_ctx_reload_crl doesn't return an error (as it's void), and its
caller never checks OpenSSL error stack. So as this function is, I think it
should handle all errors itself, and leave the error stack clear.
Of course, it would make sense to refactor this part, but that would be a
different patch.
>> + goto end;
>> + }
>>
>> - if (!X509_STORE_add_crl(store, crl))
>> - {
>> - msg(M_WARN, "CRL: cannot add %s to store", crl_file);
>> - goto end;
>> + ERR_error_string_n(err, buf, sizeof(buf));
>> +
>
>> + msg(M_WARN, "CRL: cannot read CRL from file %s: %s", crl_file,
>> buf);
>
> Use crypto_msg that handles the openssl error stack printing.
>
>> + goto end;
>> + }
>> +
>> + if (!X509_STORE_add_crl(store, crl))
>> + {
>> + msg(M_WARN, "CRL: cannot add %s to store", crl_file);
>
> Same here.
I can do this, but at the same time I'd like to note that output
becomes worse:
Thu Apr 2 17:04:24 2020 Diffie-Hellman initialized with 2048 bit key
Thu Apr 2 17:04:24 2020 OpenSSL: error:0909006C:PEM routines:get_name:no
start line
Thu Apr 2 17:04:24 2020 OpenSSL: error:0908F066:PEM
routines:get_header_and_data:bad end line
Thu Apr 2 17:04:24 2020 CRL: cannot read CRL from file combined_crl.pem
Thu Apr 2 17:04:24 2020 CRL: loaded 1 CRLs from file combined_crl.pem
Thu Apr 2 17:04:24 2020 Failed to extract curve from certificate (UNDEF),
using secp384r1 instead
The first error comes god knows where from, and the second error is
actually related to CRL (I corrupted the END X509 CRL line on purpose).
It's hard to figure out what the errors printed by crypto_msg are
actually related to.
Compare with how it was in current edition of the patch:
Thu Apr 2 17:07:13 2020 Diffie-Hellman initialized with 2048 bit key
Thu Apr 2 17:07:13 2020 OpenSSL: error:0909006C:PEM routines:get_name:no
start line
Thu Apr 2 17:07:13 2020 CRL: cannot read CRL from file combined_crl.pem:
error:0908F066:PEM routines:get_header_and_data:bad end line
Thu Apr 2 17:07:13 2020 CRL: loaded 1 CRLs from file combined_crl.pem
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
