Mir

Code review comment for lp:~mir-team/mir/attestable-timestamps-server

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

> Is libmircookie really something that will need to be released in sync with
> every Mir release? Or should it be a separate project?

I dont have a clear answer for this one. I just imagine since the project is pretty small it makes it easier to keep it in lp:mir atm? Will have to poke raof about that one.
>
> ~~~~
>
> +std::vector<uint8_t> get_random_data(unsigned size);
>
> Could be:
>
> CookieFactory::CookieFactory(unsigned size);
>

I had this at first, but switched up because one of the primary use-cases for CookieFactory is when you've shared the secret with another process, which you can't do if you've called CookieFactory(unsigned size)

> ~~~~
>
> +void mir::Server::set_cookie_secret(std::vector<uint8_t> const& secret)
> +{
> + verify_setting_allowed(self->server_config);
> + self->cookie_factory = std::make_shared<CookieFactory>(secret);
> +}
>
> It is a little surprising that this sets the cookie factory, not the secret.

I think this function should just be renamed to initialize_cookie_factory(..)

« Back to merge proposal