Fix for configurable max devices

Put a time limit on device provisioning codes while we're at it

// FREEBIE
This commit is contained in:
Moxie Marlinspike 2017-03-05 12:47:18 -08:00
parent dabd294eaf
commit 3b9a76c1f2
10 changed files with 241 additions and 48 deletions

View File

@ -0,0 +1,39 @@
package org.whispersystems.textsecuregcm.auth;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.security.MessageDigest;
import java.util.concurrent.TimeUnit;
public class StoredVerificationCode {
@JsonProperty
private final String code;
@JsonProperty
private final long timestamp;
public StoredVerificationCode(String code, long timestamp) {
this.code = code;
this.timestamp = timestamp;
}
public String getCode() {
return code;
}
public long getTimestamp() {
return timestamp;
}
public boolean isValid(String theirCodeString) {
if (timestamp + TimeUnit.MINUTES.toMillis(30) < System.currentTimeMillis()) {
return false;
}
byte[] ourCode = code.getBytes();
byte[] theirCode = theirCodeString.getBytes();
return MessageDigest.isEqual(ourCode, theirCode);
}
}

View File

@ -29,6 +29,7 @@ import org.whispersystems.textsecuregcm.auth.AuthorizationHeader;
import org.whispersystems.textsecuregcm.auth.AuthorizationToken;
import org.whispersystems.textsecuregcm.auth.AuthorizationTokenGenerator;
import org.whispersystems.textsecuregcm.auth.InvalidAuthorizationHeaderException;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.auth.TurnToken;
import org.whispersystems.textsecuregcm.auth.TurnTokenGenerator;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
@ -137,8 +138,11 @@ public class AccountController {
throw new WebApplicationException(Response.status(422).build());
}
VerificationCode verificationCode = generateVerificationCode(number);
pendingAccounts.store(number, verificationCode.getVerificationCode());
VerificationCode verificationCode = generateVerificationCode(number);
StoredVerificationCode storedVerificationCode = new StoredVerificationCode(verificationCode.getVerificationCode(),
System.currentTimeMillis());
pendingAccounts.store(number, storedVerificationCode);
if (testDevices.containsKey(number)) {
// noop
@ -168,11 +172,9 @@ public class AccountController {
rateLimiters.getVerifyLimiter().validate(number);
Optional<String> storedVerificationCode = pendingAccounts.getCodeForNumber(number);
Optional<StoredVerificationCode> storedVerificationCode = pendingAccounts.getCodeForNumber(number);
if (!storedVerificationCode.isPresent() ||
!verificationCode.equals(storedVerificationCode.get()))
{
if (!storedVerificationCode.isPresent() || !storedVerificationCode.get().isValid(verificationCode)) {
throw new WebApplicationException(Response.status(403).build());
}

View File

@ -24,6 +24,7 @@ import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials;
import org.whispersystems.textsecuregcm.auth.AuthorizationHeader;
import org.whispersystems.textsecuregcm.auth.InvalidAuthorizationHeaderException;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
import org.whispersystems.textsecuregcm.entities.DeviceInfo;
import org.whispersystems.textsecuregcm.entities.DeviceInfoList;
@ -49,7 +50,6 @@ import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.LinkedList;
@ -134,8 +134,11 @@ public class DeviceController {
throw new WebApplicationException(Response.Status.UNAUTHORIZED);
}
VerificationCode verificationCode = generateVerificationCode();
pendingDevices.store(account.getNumber(), verificationCode.getVerificationCode());
VerificationCode verificationCode = generateVerificationCode();
StoredVerificationCode storedVerificationCode = new StoredVerificationCode(verificationCode.getVerificationCode(),
System.currentTimeMillis());
pendingDevices.store(account.getNumber(), storedVerificationCode);
return verificationCode;
}
@ -157,11 +160,9 @@ public class DeviceController {
rateLimiters.getVerifyDeviceLimiter().validate(number);
Optional<String> storedVerificationCode = pendingDevices.getCodeForNumber(number);
Optional<StoredVerificationCode> storedVerificationCode = pendingDevices.getCodeForNumber(number);
if (!storedVerificationCode.isPresent() ||
!MessageDigest.isEqual(verificationCode.getBytes(), storedVerificationCode.get().getBytes()))
{
if (!storedVerificationCode.isPresent() || !storedVerificationCode.get().isValid(verificationCode)) {
throw new WebApplicationException(Response.status(403).build());
}
@ -171,7 +172,13 @@ public class DeviceController {
throw new WebApplicationException(Response.status(403).build());
}
if (account.get().getActiveDeviceCount() >= MAX_DEVICES) {
int maxDeviceLimit = MAX_DEVICES;
if (maxDeviceConfiguration.containsKey(account.get().getNumber())) {
maxDeviceLimit = maxDeviceConfiguration.get(account.get().getNumber());
}
if (account.get().getActiveDeviceCount() >= maxDeviceLimit) {
throw new DeviceLimitExceededException(account.get().getDevices().size(), MAX_DEVICES);
}

View File

@ -16,22 +16,40 @@
*/
package org.whispersystems.textsecuregcm.storage;
import org.skife.jdbi.v2.StatementContext;
import org.skife.jdbi.v2.sqlobject.Bind;
import org.skife.jdbi.v2.sqlobject.SqlQuery;
import org.skife.jdbi.v2.sqlobject.SqlUpdate;
import org.skife.jdbi.v2.sqlobject.customizers.Mapper;
import org.skife.jdbi.v2.tweak.ResultSetMapper;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import java.sql.ResultSet;
import java.sql.SQLException;
public interface PendingAccounts {
@SqlUpdate("WITH upsert AS (UPDATE pending_accounts SET verification_code = :verification_code WHERE number = :number RETURNING *) " +
"INSERT INTO pending_accounts (number, verification_code) SELECT :number, :verification_code WHERE NOT EXISTS (SELECT * FROM upsert)")
void insert(@Bind("number") String number, @Bind("verification_code") String verificationCode);
@SqlUpdate("WITH upsert AS (UPDATE pending_accounts SET verification_code = :verification_code, timestamp = :timestamp WHERE number = :number RETURNING *) " +
"INSERT INTO pending_accounts (number, verification_code, timestamp) SELECT :number, :verification_code, :timestamp WHERE NOT EXISTS (SELECT * FROM upsert)")
void insert(@Bind("number") String number, @Bind("verification_code") String verificationCode, @Bind("timestamp") long timestamp);
@SqlQuery("SELECT verification_code FROM pending_accounts WHERE number = :number")
String getCodeForNumber(@Bind("number") String number);
@Mapper(StoredVerificationCodeMapper.class)
@SqlQuery("SELECT verification_code, timestamp FROM pending_accounts WHERE number = :number")
StoredVerificationCode getCodeForNumber(@Bind("number") String number);
@SqlUpdate("DELETE FROM pending_accounts WHERE number = :number")
void remove(@Bind("number") String number);
@SqlUpdate("VACUUM pending_accounts")
public void vacuum();
public static class StoredVerificationCodeMapper implements ResultSetMapper<StoredVerificationCode> {
@Override
public StoredVerificationCode map(int i, ResultSet resultSet, StatementContext statementContext)
throws SQLException
{
return new StoredVerificationCode(resultSet.getString("verification_code"),
resultSet.getLong("timestamp"));
}
}
}

View File

@ -16,27 +16,40 @@
*/
package org.whispersystems.textsecuregcm.storage;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.util.SystemMapper;
import java.io.IOException;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.JedisPool;
public class PendingAccountsManager {
private static final String CACHE_PREFIX = "pending_account::";
private final Logger logger = LoggerFactory.getLogger(PendingAccountsManager.class);
private static final String CACHE_PREFIX = "pending_account2::";
private final PendingAccounts pendingAccounts;
private final JedisPool cacheClient;
private final ObjectMapper mapper;
public PendingAccountsManager(PendingAccounts pendingAccounts, JedisPool cacheClient)
{
this.pendingAccounts = pendingAccounts;
this.cacheClient = cacheClient;
this.mapper = SystemMapper.getMapper();
}
public void store(String number, String code) {
public void store(String number, StoredVerificationCode code) {
memcacheSet(number, code);
pendingAccounts.insert(number, code);
pendingAccounts.insert(number, code.getCode(), code.getTimestamp());
}
public void remove(String number) {
@ -44,8 +57,8 @@ public class PendingAccountsManager {
pendingAccounts.remove(number);
}
public Optional<String> getCodeForNumber(String number) {
Optional<String> code = memcacheGet(number);
public Optional<StoredVerificationCode> getCodeForNumber(String number) {
Optional<StoredVerificationCode> code = memcacheGet(number);
if (!code.isPresent()) {
code = Optional.fromNullable(pendingAccounts.getCodeForNumber(number));
@ -58,15 +71,23 @@ public class PendingAccountsManager {
return code;
}
private void memcacheSet(String number, String code) {
private void memcacheSet(String number, StoredVerificationCode code) {
try (Jedis jedis = cacheClient.getResource()) {
jedis.set(CACHE_PREFIX + number, code);
jedis.set(CACHE_PREFIX + number, mapper.writeValueAsString(code));
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(e);
}
}
private Optional<String> memcacheGet(String number) {
private Optional<StoredVerificationCode> memcacheGet(String number) {
try (Jedis jedis = cacheClient.getResource()) {
return Optional.fromNullable(jedis.get(CACHE_PREFIX + number));
String json = jedis.get(CACHE_PREFIX + number);
if (json == null) return Optional.absent();
else return Optional.of(mapper.readValue(json, StoredVerificationCode.class));
} catch (IOException e) {
logger.warn("PendingAccountsManager", "Error deserializing value...");
return Optional.absent();
}
}

View File

@ -16,19 +16,38 @@
*/
package org.whispersystems.textsecuregcm.storage;
import org.skife.jdbi.v2.StatementContext;
import org.skife.jdbi.v2.sqlobject.Bind;
import org.skife.jdbi.v2.sqlobject.SqlQuery;
import org.skife.jdbi.v2.sqlobject.SqlUpdate;
import org.skife.jdbi.v2.sqlobject.customizers.Mapper;
import org.skife.jdbi.v2.tweak.ResultSetMapper;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import java.sql.ResultSet;
import java.sql.SQLException;
public interface PendingDevices {
@SqlUpdate("WITH upsert AS (UPDATE pending_devices SET verification_code = :verification_code WHERE number = :number RETURNING *) " +
"INSERT INTO pending_devices (number, verification_code) SELECT :number, :verification_code WHERE NOT EXISTS (SELECT * FROM upsert)")
void insert(@Bind("number") String number, @Bind("verification_code") String verificationCode);
@SqlUpdate("WITH upsert AS (UPDATE pending_devices SET verification_code = :verification_code, timestamp = :timestamp WHERE number = :number RETURNING *) " +
"INSERT INTO pending_devices (number, verification_code, timestamp) SELECT :number, :verification_code, :timestamp WHERE NOT EXISTS (SELECT * FROM upsert)")
void insert(@Bind("number") String number, @Bind("verification_code") String verificationCode, @Bind("timestamp") long timestamp);
@SqlQuery("SELECT verification_code FROM pending_devices WHERE number = :number")
String getCodeForNumber(@Bind("number") String number);
@Mapper(StoredVerificationCodeMapper.class)
@SqlQuery("SELECT verification_code, timestamp FROM pending_devices WHERE number = :number")
StoredVerificationCode getCodeForNumber(@Bind("number") String number);
@SqlUpdate("DELETE FROM pending_devices WHERE number = :number")
void remove(@Bind("number") String number);
public static class StoredVerificationCodeMapper implements ResultSetMapper<StoredVerificationCode> {
@Override
public StoredVerificationCode map(int i, ResultSet resultSet, StatementContext statementContext)
throws SQLException
{
return new StoredVerificationCode(resultSet.getString("verification_code"),
resultSet.getLong("timestamp"));
}
}
}

View File

@ -16,28 +16,40 @@
*/
package org.whispersystems.textsecuregcm.storage;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.util.SystemMapper;
import java.io.IOException;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.JedisPool;
public class PendingDevicesManager {
private static final String CACHE_PREFIX = "pending_devices::";
private final Logger logger = LoggerFactory.getLogger(PendingDevicesManager.class);
private static final String CACHE_PREFIX = "pending_devices2::";
private final PendingDevices pendingDevices;
private final JedisPool cacheClient;
private final ObjectMapper mapper;
public PendingDevicesManager(PendingDevices pendingDevices,
JedisPool cacheClient)
{
this.pendingDevices = pendingDevices;
this.cacheClient = cacheClient;
this.mapper = SystemMapper.getMapper();
}
public void store(String number, String code) {
public void store(String number, StoredVerificationCode code) {
memcacheSet(number, code);
pendingDevices.insert(number, code);
pendingDevices.insert(number, code.getCode(), code.getTimestamp());
}
public void remove(String number) {
@ -45,8 +57,8 @@ public class PendingDevicesManager {
pendingDevices.remove(number);
}
public Optional<String> getCodeForNumber(String number) {
Optional<String> code = memcacheGet(number);
public Optional<StoredVerificationCode> getCodeForNumber(String number) {
Optional<StoredVerificationCode> code = memcacheGet(number);
if (!code.isPresent()) {
code = Optional.fromNullable(pendingDevices.getCodeForNumber(number));
@ -59,15 +71,23 @@ public class PendingDevicesManager {
return code;
}
private void memcacheSet(String number, String code) {
private void memcacheSet(String number, StoredVerificationCode code) {
try (Jedis jedis = cacheClient.getResource()) {
jedis.set(CACHE_PREFIX + number, code);
jedis.set(CACHE_PREFIX + number, mapper.writeValueAsString(code));
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(e);
}
}
private Optional<String> memcacheGet(String number) {
private Optional<StoredVerificationCode> memcacheGet(String number) {
try (Jedis jedis = cacheClient.getResource()) {
return Optional.fromNullable(jedis.get(CACHE_PREFIX + number));
String json = jedis.get(CACHE_PREFIX + number);
if (json == null) return Optional.absent();
else return Optional.of(mapper.readValue(json, StoredVerificationCode.class));
} catch (IOException e) {
logger.warn("Could not parse pending device stored verification json");
return Optional.absent();
}
}

View File

@ -4,7 +4,8 @@
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-2.0.xsd">
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-2.0.xsd"
logicalFilePath="migrations.xml">
<changeSet id="1" author="moxie">
<createTable tableName="accounts">
@ -171,4 +172,18 @@
<changeSet id="4" author="moxie">
<dropColumn tableName="keys" columnName="identity_key"/>
</changeSet>
<changeSet id="5" author="moxie">
<addColumn tableName="pending_accounts">
<column name="timestamp" type="bigint" defaultValueComputed="extract(epoch from now()) * 1000">
<constraints nullable="false"/>
</column>
</addColumn>
<addColumn tableName="pending_devices">
<column name="timestamp" type="bigint" defaultValueComputed="extract(epoch from now()) * 1000">
<constraints nullable="false"/>
</column>
</addColumn>
</changeSet>
</databaseChangeLog>

View File

@ -8,6 +8,7 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.whispersystems.dropwizard.simpleauth.AuthValueFactoryProvider;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.auth.TurnTokenGenerator;
import org.whispersystems.textsecuregcm.controllers.AccountController;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
@ -26,6 +27,7 @@ import javax.ws.rs.client.Entity;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.util.HashMap;
import java.util.concurrent.TimeUnit;
import io.dropwizard.testing.junit.ResourceTestRule;
import static org.assertj.core.api.Assertions.assertThat;
@ -34,7 +36,8 @@ import static org.mockito.Mockito.*;
public class AccountControllerTest {
private static final String SENDER = "+14152222222";
private static final String SENDER = "+14152222222";
private static final String SENDER_OLD = "+14151111111";
private PendingAccountsManager pendingAccountsManager = mock(PendingAccountsManager.class);
private AccountsManager accountsManager = mock(AccountsManager.class );
@ -72,7 +75,8 @@ public class AccountControllerTest {
when(timeProvider.getCurrentTimeMillis()).thenReturn(System.currentTimeMillis());
when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(Optional.of("1234"));
when(pendingAccountsManager.getCodeForNumber(SENDER)).thenReturn(Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis())));
when(pendingAccountsManager.getCodeForNumber(SENDER_OLD)).thenReturn(Optional.of(new StoredVerificationCode("1234", System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(31))));
}
@Test
@ -117,6 +121,21 @@ public class AccountControllerTest {
verify(accountsManager, times(1)).create(isA(Account.class));
}
@Test
public void testVerifyCodeOld() throws Exception {
Response response =
resources.getJerseyTest()
.target(String.format("/v1/accounts/code/%s", "1234"))
.request()
.header("Authorization", AuthHelper.getAuthHeader(SENDER_OLD, "bar"))
.put(Entity.entity(new AccountAttributes("keykeykeykey", false, 2222),
MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(403);
verifyNoMoreInteractions(accountsManager);
}
@Test
public void testVerifyBadCode() throws Exception {
Response response =

View File

@ -22,6 +22,7 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.whispersystems.dropwizard.simpleauth.AuthValueFactoryProvider;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.controllers.DeviceController;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
import org.whispersystems.textsecuregcm.entities.DeviceResponse;
@ -39,9 +40,9 @@ import javax.ws.rs.Path;
import javax.ws.rs.client.Entity;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import io.dropwizard.jersey.validation.ConstraintViolationExceptionMapper;
import io.dropwizard.testing.junit.ResourceTestRule;
@ -105,8 +106,8 @@ public class DeviceControllerTest {
when(account.getNextDeviceId()).thenReturn(42L);
// when(maxedAccount.getActiveDeviceCount()).thenReturn(6);
when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of("5678901"));
when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of("1112223"));
when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(new StoredVerificationCode("5678901", System.currentTimeMillis())));
when(pendingDevicesManager.getCodeForNumber(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(new StoredVerificationCode("1112223", System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(31))));
when(accountsManager.get(AuthHelper.VALID_NUMBER)).thenReturn(Optional.of(account));
when(accountsManager.get(AuthHelper.VALID_NUMBER_TWO)).thenReturn(Optional.of(maxedAccount));
}
@ -134,6 +135,38 @@ public class DeviceControllerTest {
verify(pendingDevicesManager).remove(AuthHelper.VALID_NUMBER);
}
@Test
public void invalidDeviceRegisterTest() throws Exception {
VerificationCode deviceCode = resources.getJerseyTest()
.target("/v1/devices/provisioning/code")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD))
.get(VerificationCode.class);
assertThat(deviceCode).isEqualTo(new VerificationCode(5678901));
Response response = resources.getJerseyTest()
.target("/v1/devices/5678902")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, "password1"))
.put(Entity.entity(new AccountAttributes("keykeykeykey", false, 1234),
MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(403);
}
@Test
public void oldDeviceRegisterTest() throws Exception {
Response response = resources.getJerseyTest()
.target("/v1/devices/1112223")
.request()
.header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER_TWO, AuthHelper.VALID_PASSWORD_TWO))
.put(Entity.entity(new AccountAttributes("keykeykeykey", false, 1234),
MediaType.APPLICATION_JSON_TYPE));
assertThat(response.getStatus()).isEqualTo(403);
}
@Test
public void maxDevicesTest() throws Exception {
Response response = resources.getJerseyTest()