Hi,
On 07-07-2020 18:20, Antonio Quartulli wrote:
> On 07/07/2020 14:16, Arne Schwabe wrote:
>> This order the states from unauthenticated to authenticated and also
>> changes the comparison for KS_AUTH_FALSE from != to >
>>
>> Also remove a now obsolete comment and two obsolete ifdefs. While
>> keeping the ifdef in ssl_verify would save a few bytes of code,
>> this is too minor to justify keeping the ifdef
>>
>> Signed-off-by: Arne Schwabe <a...@rfc2549.org>
>> ---
>> src/openvpn/ssl.c | 6 +++---
>> src/openvpn/ssl_common.h | 7 ++-----
>> src/openvpn/ssl_verify.c | 15 ++++-----------
>> 3 files changed, 9 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
>> index 71565dd3..c73b51c3 100644
>> --- a/src/openvpn/ssl.c
>> +++ b/src/openvpn/ssl.c
>> @@ -2465,7 +2465,7 @@ key_method_2_write(struct buffer *buf, struct
>> tls_session *session)
>> */
>> if (session->opt->server && !(session->opt->mode == MODE_SERVER &&
>> ks->key_id <= 0))
>> {
>> - if (ks->authenticated != KS_AUTH_FALSE)
>> + if (ks->authenticated > KS_AUTH_FALSE)
>> {
>> if (!tls_session_generate_data_channel_keys(session))
>> {
>> @@ -2646,7 +2646,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi
>> *multi, struct tls_sessio
>> secure_memzero(up, sizeof(*up));
>>
>> /* Perform final authentication checks */
>> - if (ks->authenticated != KS_AUTH_FALSE)
>> + if (ks->authenticated > KS_AUTH_FALSE)
>> {
>> verify_final_auth_checks(multi, session);
>> }
>> @@ -2671,7 +2671,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi
>> *multi, struct tls_sessio
>> * Call OPENVPN_PLUGIN_TLS_FINAL plugin if defined, for final
>> * veto opportunity over authentication decision.
>> */
>> - if ((ks->authenticated != KS_AUTH_FALSE)
>> + if ((ks->authenticated > KS_AUTH_FALSE)
>> && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
>> {
>> key_state_export_keying_material(&ks->ks_ssl, session);
>> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
>> index fdf589b5..7d841ffb 100644
>> --- a/src/openvpn/ssl_common.h
>> +++ b/src/openvpn/ssl_common.h
>> @@ -129,8 +129,8 @@ struct key_source2 {
>>
>> enum ks_auth_state {
>> KS_AUTH_FALSE,
>
> I suggest to add comments here describing how states work and how new
> states should be added.
>
> I.e. something like: "any state before or equal to KS_AUTH_FALSE is
> considered unauthorized". Not sure the terminology is right, but
> something like this would help introducing new states, like Steffan
> reported before.
+1
Don't care much about the terminology. As long as the intention from the
quote from Antonio is immediately obvious from the code.
> Maybe he has additional comments too.
Nope, otherwise I think this looks good.
-Steffan
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
