Hi, On Fri, Oct 24, 2014 at 02:23:48PM +0300, Lev Stipakov wrote: > As discussed on IRC meeting, we replace session-id with peer-id. > > So, waiting for review and code-ACK :)
A few bits of review on the "non-critical" parts - so, most of it is style,
but nevertheless:
> /* Decrypt packet ID + payload */
> -
> if (ctx->cipher)
... please don't do whitespace changes in places where no code changes
(as it makes it harder to see where changes happened)
[..]
> + int i;
> + m->instances = malloc(sizeof(struct multi_instance*) * m->max_clients);
> + for (i = 0; i < m->max_clients; ++ i)
> + {
> + m->instances[i] = NULL;
> + }
> +
This doesn't look right :-) - what's wrong with calloc()?
> @@ -3922,9 +3923,9 @@ apply_push_options (struct options *options,
> ++line_num;
> if (parse_line (line, p, SIZE (p), file, line_num, msglevel,
> &options->gc))
> {
> - add_option (options, p, file, line_num, 0, msglevel, permission_mask,
> option_types_found, es);
> + add_option (options, p, file, line_num, 0, msglevel,
> permission_mask, option_types_found, es);
> + }
> }
> - }
> return true;
> }
Here's an escaped tab-to-space conversion or so, but "just whitespace
change" nonetheless.
[..]
> @@ -2799,7 +2808,12 @@ tls_pre_decrypt (struct tls_multi *multi,
> opt->pid_persist = NULL;
> opt->flags &= multi->opt.crypto_flags_and;
> opt->flags |= multi->opt.crypto_flags_or;
> +
> ASSERT (buf_advance (buf, 1));
> + if (op == P_DATA_V2) {
> + buf_advance (buf, 3);
> + }
> +
I think that one should be "ASSERT(buf_advance(buf, 3 ));"
I'm not sure it is possible to exploit this, but since *every* buf_advance()
is protected, this one should be as well.
> @@ -3296,6 +3310,7 @@ tls_pre_decrypt_lite (const struct tls_auth_standalone
> *tas,
> return ret;
>
> error:
> +
> tls_clear_error();
> gc_free (&gc);
> return ret;
Whitespace...
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]
fax: +49-89-35655025 [email protected]
pgpHGR8ACkvD2.pgp
Description: PGP signature
