Hi
On 13-07-2020 11:46, Arne Schwabe wrote:
> @@ -1100,7 +1100,7 @@ process_incoming_link_part1(struct context *c, struct
> link_socket_info *lsi, boo
> floated, &ad_start))
> {
> /* Restore pre-NCP frame parameters */
> - if (is_hard_reset(opcode, c->options.key_method))
> + if (is_hard_reset(opcode, KEY_METHOD_2))
> {
Can't we just remove the key_method parameter from is_hard_reset()?
> @@ -3817,10 +3803,9 @@ options_string(const struct options *o,
> * tls-auth/tls-crypt does not match. Removing tls-auth here
> would
> * break stuff, so leaving that in place. */
>
> - if (o->key_method > 1)
> - {
> - buf_printf(&out, ",key-method %d", o->key_method);
> - }
> +
> +
> + buf_printf(&out, ",key-method %d", 2);
This could do with one less newline.
> @@ -2404,7 +2348,7 @@ key_method_2_write(struct buffer *buf, struct
> tls_session *session)
> }
>
> /* write key_method + flags */
> - if (!buf_write_u8(buf, (session->opt->key_method & KEY_METHOD_MASK)))
> + if (!buf_write_u8(buf, (KEY_METHOD_2 & KEY_METHOD_MASK)))
> {
> goto error;
> }
The masking looks a bit silly now. Maybe replace with a static_assert()
if you want to be sure that no "wrong" bits are set?
> static bool
> key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct
> tls_session *session)
> {
> struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */
>
> - int key_method_flags;
> bool username_status, password_status;
>
> struct gc_arena gc = gc_new();
> @@ -2582,8 +2464,6 @@ key_method_2_read(struct buffer *buf, struct tls_multi
> *multi, struct tls_sessio
> /* allocate temporary objects */
> ALLOC_ARRAY_CLEAR_GC(options, char, TLS_OPTIONS_LEN, &gc);
>
> - ASSERT(session->opt->key_method == 2);
> -
> /* discard leading uint32 */
> if (!buf_advance(buf, 4))
> {
> @@ -2593,7 +2473,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi
> *multi, struct tls_sessio
> }
>
> /* get key method */
> - key_method_flags = buf_read_u8(buf);
> + int key_method_flags = buf_read_u8(buf);
This variable declaration is now *after* a "goto error" jump. Currently
not harmful, because the variable isn't used after the error label, but
static analyzers might complain about "jump past initialization".
Otherwise this looks good based on stare-at-code. Didn't have time to
test yet.
-Steffan
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
