Skip to content

Commit

Permalink
Fix incomplete API key migration
Browse files Browse the repository at this point in the history 
Fixes #4652

Signed-off-by: nscuro <nscuro@protonmail.com>
  • Loading branch information
@nscuro
nscuro committed Feb 22, 2025
1 parent 297f3b2 commit 6a4b953
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 93 deletions.
3 changes: 3 additions & 0 deletions dev/docker-compose.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ name: "dependency-track"
services:
apiserver:
image: dependencytrack/apiserver:snapshot
environment:
# Speed up password hashing for faster initial login (default is 14 rounds).
ALPINE_BCRYPT_ROUNDS: "4"
ports:
- "127.0.0.1:8080:8080"
volumes:
Expand Down
49 changes: 26 additions & 23 deletions src/main/java/org/dependencytrack/resources/v1/TeamResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import alpine.model.ApiKey;
import alpine.model.Team;
import alpine.model.UserPrincipal;
import alpine.security.ApiKeyDecoder;
import alpine.security.InvalidApiKeyFormatException;
import alpine.server.auth.PermissionRequired;
import alpine.server.resources.AlpineResource;
import io.swagger.v3.oas.annotations.Operation;
Expand All @@ -36,7 +38,6 @@
import io.swagger.v3.oas.annotations.security.SecurityRequirement;
import io.swagger.v3.oas.annotations.security.SecurityRequirements;
import io.swagger.v3.oas.annotations.tags.Tag;

import org.dependencytrack.auth.Permissions;
import org.dependencytrack.model.validation.ValidUuid;
import org.dependencytrack.persistence.QueryManager;
Expand All @@ -59,8 +60,6 @@
import java.util.ArrayList;
import java.util.List;

import static org.datanucleus.PropertyNames.PROPERTY_RETAIN_VALUES;

/**
* JAX-RS resources for processing teams.
*
Expand Down Expand Up @@ -308,12 +307,14 @@ public Response regenerateApiKey(
@Parameter(description = "The public ID for the API key or for Legacy the complete Key to regenerate", required = true)
@PathParam("publicIdOrKey") String publicIdOrKey) {
try (QueryManager qm = new QueryManager()) {
boolean isLegacy = publicIdOrKey.length() == ApiKey.LEGACY_FULL_KEY_LENGTH;
ApiKey apiKey;
if (publicIdOrKey.length() == ApiKey.FULL_KEY_LENGTH || isLegacy) {
apiKey = qm.getApiKeyByPublicId(ApiKey.getPublicId(publicIdOrKey, isLegacy));
} else {
apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
ApiKey apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
if (apiKey == null) {
try {
final ApiKey deocdedApiKey = ApiKeyDecoder.decode(publicIdOrKey);
apiKey = qm.getApiKeyByPublicId(deocdedApiKey.getPublicId());
} catch (InvalidApiKeyFormatException e) {
LOGGER.debug("Failed to decode value as API key", e);
}
}
if (apiKey != null) {
apiKey = qm.regenerateApiKey(apiKey);
Expand Down Expand Up @@ -349,15 +350,15 @@ public Response updateApiKeyComment(
@PathParam("publicIdOrKey") final String publicIdOrKey,
final String comment) {
try (final var qm = new QueryManager()) {
qm.getPersistenceManager().setProperty(PROPERTY_RETAIN_VALUES, "true");

return qm.callInTransaction(() -> {
boolean isLegacy = publicIdOrKey.length() == ApiKey.LEGACY_FULL_KEY_LENGTH;
ApiKey apiKey;
if (publicIdOrKey.length() == ApiKey.FULL_KEY_LENGTH || isLegacy) {
apiKey = qm.getApiKeyByPublicId(ApiKey.getPublicId(publicIdOrKey, isLegacy));
} else {
apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
ApiKey apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
if (apiKey == null) {
try {
final ApiKey deocdedApiKey = ApiKeyDecoder.decode(publicIdOrKey);
apiKey = qm.getApiKeyByPublicId(deocdedApiKey.getPublicId());
} catch (InvalidApiKeyFormatException e) {
LOGGER.debug("Failed to decode value as API key", e);
}
}
if (apiKey == null) {
return Response
Expand Down Expand Up @@ -388,12 +389,14 @@ public Response deleteApiKey(
@Parameter(description = "The public ID for the API key or for Legacy the full Key to delete", required = true)
@PathParam("publicIdOrKey") String publicIdOrKey) {
try (QueryManager qm = new QueryManager()) {
boolean isLegacy = publicIdOrKey.length() == ApiKey.LEGACY_FULL_KEY_LENGTH;
ApiKey apiKey;
if (publicIdOrKey.length() == ApiKey.FULL_KEY_LENGTH || isLegacy) {
apiKey = qm.getApiKeyByPublicId(ApiKey.getPublicId(publicIdOrKey, isLegacy));
} else {
apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
ApiKey apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
if (apiKey == null) {
try {
final ApiKey deocdedApiKey = ApiKeyDecoder.decode(publicIdOrKey);
apiKey = qm.getApiKeyByPublicId(deocdedApiKey.getPublicId());
} catch (InvalidApiKeyFormatException e) {
LOGGER.debug("Failed to decode value as API key", e);
}
}
if (apiKey != null) {
qm.delete(apiKey);
Expand Down
136 changes: 91 additions & 45 deletions src/main/java/org/dependencytrack/upgrade/v4130/v4130Updater.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,18 @@
*/
package org.dependencytrack.upgrade.v4130;

import java.security.MessageDigest;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.HexFormat;

import alpine.common.logging.Logger;
import alpine.model.ApiKey;
import alpine.persistence.AlpineQueryManager;
import alpine.security.ApiKeyDecoder;
import alpine.server.upgrade.AbstractUpgradeItem;
import alpine.server.util.DbUtil;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.Statement;

public class v4130Updater extends AbstractUpgradeItem {

private static final Logger LOGGER = Logger.getLogger(v4130Updater.class);
Expand All @@ -46,48 +45,95 @@ public void executeUpgrade(final AlpineQueryManager qm, final Connection connect
}

private void migrateToHashedApiKey(final Connection connection) throws Exception {
LOGGER.info("Store API keys in hashed format!");

try (final PreparedStatement ps = connection.prepareStatement("""
UPDATE "APIKEY"
SET "APIKEY" = ?, "PUBLIC_ID" = ?, "IS_LEGACY" = ?
WHERE "ID" = ?
""")) {

if (DbUtil.isMysql() || DbUtil.isMssql()) {
ps.setInt(3, 1);
} else {
ps.setBoolean(3, true);
}
LOGGER.info("Performing API key migration");

final boolean shouldReEnableAutoCommit = connection.getAutoCommit();
connection.setAutoCommit(false);
boolean committed = false;

try (final Statement statement = connection.createStatement()) {
statement.execute("""
SELECT "ID", "APIKEY"
FROM "APIKEY"
try (final PreparedStatement selectLegacyKeysStatement = connection.prepareStatement("""
SELECT "ID"
, "APIKEY"
FROM "APIKEY"
WHERE "PUBLIC_ID" IS NULL
""");
try (final ResultSet rs = statement.getResultSet()) {
String clearKey;
int id;
String hashedKey;
String publicId;
while (rs.next()) {
clearKey = rs.getString("apikey");
if (clearKey.length() != ApiKey.LEGACY_FULL_KEY_LENGTH) {
continue;
}
final MessageDigest digest = MessageDigest.getInstance("SHA3-256");
id = rs.getInt("id");
hashedKey = HexFormat.of().formatHex(digest.digest(ApiKey.getOnlyKeyAsBytes(clearKey, true)));
publicId = ApiKey.getPublicId(clearKey, true);

ps.setString(1, hashedKey);
ps.setString(2, publicId);
ps.setInt(4, id);

ps.executeUpdate();
}
final PreparedStatement updateLegacyKeysStatement = connection.prepareStatement("""
UPDATE "APIKEY"
SET "SECRET_HASH" = ?
, "PUBLIC_ID" = ?
, "IS_LEGACY" = ?
, "APIKEY" = ?
WHERE "ID" = ?
""");
// Legacy keys that were migrated by a previous SNAPSHOT version of v4.13.0, and have not been regenerated yet,
// *or* non-legacy keys that were created by a previous SNAPSHOT version of v4.13.0.
//
// For these keys, the APIKEY column already contains the hashed secret value.
final PreparedStatement updateMigratedKeysStatement = connection.prepareStatement("""
UPDATE "APIKEY"
SET "SECRET_HASH" = "APIKEY"
, "APIKEY" = CONCAT('migrated-', CAST("ID" AS VARCHAR))
WHERE (NOT "IS_LEGACY" AND "APIKEY" NOT LIKE 'migrated-%')
OR ("IS_LEGACY" AND "PUBLIC_ID" IS NOT NULL AND "SECRET_HASH" IS NULL)
""")) {
final ResultSet rs = selectLegacyKeysStatement.executeQuery();
while (rs.next()) {
final long apiKeyId = rs.getLong("ID");
final ApiKey decodedApiKey = ApiKeyDecoder.decode(rs.getString("APIKEY"));

// Perform some sanity checks and fail the migration if anything looks odd.
// Best to fail the migration entirely than to mess up any API keys.
if (!decodedApiKey.isLegacy()) {
throw new IllegalStateException("""
Unable to migrate API key with ID %d: Failed to recognize \
it as legacy format.""".formatted(apiKeyId));
}
if (decodedApiKey.getSecretHash() == null) {
throw new IllegalStateException("""
Unable to migrate API key with ID %d: No secret hash generated \
during conversion.""".formatted(apiKeyId));
}
if (decodedApiKey.getPublicId() == null) {
throw new IllegalStateException("""
Unable to migrate API key with ID %d: No public ID determined \
during conversion.""".formatted(apiKeyId));
}

updateLegacyKeysStatement.setString(1, decodedApiKey.getSecretHash());
updateLegacyKeysStatement.setString(2, decodedApiKey.getPublicId());
if (DbUtil.isMysql() || DbUtil.isMssql()) {
updateLegacyKeysStatement.setInt(3, 1);
} else {
updateLegacyKeysStatement.setBoolean(3, true);
}
updateLegacyKeysStatement.setString(4, "migrated-" + apiKeyId);
updateLegacyKeysStatement.setLong(5, apiKeyId);
updateLegacyKeysStatement.executeUpdate();

LOGGER.info("Migrated legacy API key with ID " + apiKeyId);
}

final int updatedMigratedKeys = updateMigratedKeysStatement.executeUpdate();
if (updatedMigratedKeys > 0) {
LOGGER.info("Updated %d previously migrated keys".formatted(updatedMigratedKeys));
}

connection.commit();
} finally {
if (!committed) {
connection.rollback();
}

if (shouldReEnableAutoCommit) {
connection.setAutoCommit(true);
}
}

LOGGER.info("API key migration completed; Dropping \"APIKEY\" column from \"APIKEY\" table");
try (final Statement statement = connection.createStatement()) {
statement.execute("""
ALTER TABLE "APIKEY" DROP COLUMN "APIKEY"
""");
}
}
}
2 changes: 1 addition & 1 deletion src/test/java/org/dependencytrack/ResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void before() throws Exception {
// Add a test user and team with API key. Optional if this is used, but its available to all tests.
this.qm = new QueryManager();
team = qm.createTeam("Test Users");
this.apiKey = qm.createApiKey(team).getClearTextKey();
this.apiKey = qm.createApiKey(team).getKey();
}

@After
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/dependencytrack/resources/v1/FindingResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ public void getAllFindingsWithAclEnabled() {
qm.persist(aclToggle);
}
Response response = jersey.target(V1_FINDING).request()
.header(X_API_KEY, apiKey.getClearTextKey())
.header(X_API_KEY, apiKey.getKey())
.get(Response.class);
Assert.assertEquals(200, response.getStatus(), 0);
Assert.assertEquals(String.valueOf(3), response.getHeaderString(TOTAL_COUNT_HEADER));
Expand Down Expand Up @@ -581,7 +581,7 @@ public void getAllFindingsGroupedByVulnerabilityWithAclEnabled() {
qm.persist(aclToggle);
}
Response response = jersey.target(V1_FINDING + "/grouped").request()
.header(X_API_KEY, apiKey.getClearTextKey())
.header(X_API_KEY, apiKey.getKey())
.get(Response.class);
Assert.assertEquals(200, response.getStatus(), 0);
Assert.assertEquals(String.valueOf(3), response.getHeaderString(TOTAL_COUNT_HEADER));
Expand Down
Loading

0 comments on commit 6a4b953

Please sign in to comment.