| Message ID | 20221215190143.2107896-2-arne@rfc2549.org |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Improvement/fixes based on Trail of Bits audit | expand |
Hi, On Thu, Dec 15, 2022 at 08:01:36PM +0100, Arne Schwabe wrote: > This changes the password check on the management interface to be constant > time. Normally the management port should not be exposed in a way that allows > an attacker to even interact with it but making the check constant time as > an additional layer of security is always good. NAK on this, the min_int() logic is wrong. My fault, sorry. With this, it will only compare "up the the number of bytes that the attacker has entered", and if he happens to catch the first character of the password, he's in... Escape character is '^]'. ENTER PASSWORD:f SUCCESS: password is correct ... the password here is actually "foobar"... Doing a "+1" on the compare length will include the 0-byte (which is different here, "o" vs. "0"), and that should cover all cases. gert
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 9349b62ad..d952618e7 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -198,7 +198,11 @@ man_check_password(struct management *man, const char *line) { if (man_password_needed(man)) { - if (streq(line, man->settings.up.password)) + /* This comparison is not fixed time but since strlen(time) is based on + * the attacker choice, it should not give any indication of the real + * password length */ + if (memcmp_constant_time(line, man->settings.up.password, + min_uint(strlen(line), sizeof(man->settings.up.password))) == 0) { man->connection.password_verified = true; msg(M_CLIENT, "SUCCESS: password is correct");
This changes the password check on the management interface to be constant time. Normally the management port should not be exposed in a way that allows an attacker to even interact with it but making the check constant time as an additional layer of security is always good. Reported-by: Connor Edwards <cedw@pm.me> Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/manage.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)