From 58cd89cc80616e95650e455019ec8c1c20483a29 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sat, 12 Aug 2017 20:31:57 +0200 Subject: [PATCH] upd: move revocation info to own table This commit moves the information about certificate revocation into an own table. All locations accessing this information have been adapted. Old Behavior: Table "certs" has a column "revoked" that is "NULL" for all non-revoked certificates and contains the revocation time (from cassiopeia) for all revoked certificates. on revoke: gigi creates a "job" indicating the row in "certs" to revoke on revoke execution: cassiopeia marks the "job" done and inserts a revocation date in "certs" Possible States: 1. revoked is NULL, no revocation job exists 2. revoked is NULL, revocation job exists. (2b. this transition is not a transaction in cassiopeia so an in-between state might be observed) 3. revoked is not NULL, revocation job is marked as done New Behavior: Table "certs" doesn't have a "revoked" column instead there is a new table. on revoke: gigi inserts the "revocation"-row without a revocation date filled in, but with the reason and other information about the revocation. Additionally a "job" is created to trigger cassiopeia. on revoke execution: cassiopeia marks the job as done and updates the revocation date in the revocation column. Possible States: 1. no revocation entry and not revocation job exist. (1b. again strictly speaking not a transaction here) 2. revocation entry without date and revocation job exist (2b. also still no transaction) 3. date in revocation entry is filled and revocation job is marked done Change-Id: Ie2a51a16eed420b284f9fd5660e057da1069b740 --- src/club/wpia/gigi/api/RevokeCertificate.java | 5 ++-- .../gigi/database/DatabaseConnection.java | 2 +- .../wpia/gigi/database/tableStructure.sql | 15 +++++++++--- .../wpia/gigi/database/upgrade/from_28.sql | 12 ++++++++++ src/club/wpia/gigi/dbObjects/Certificate.java | 23 +++++++++++++++---- .../wpia/gigi/dbObjects/CertificateOwner.java | 4 ++-- src/club/wpia/gigi/dbObjects/Job.java | 10 ++++++-- .../wpia/gigi/dbObjects/Organisation.java | 3 ++- .../wpia/gigi/dbObjects/SupportedUser.java | 5 ++-- .../certs/CertificateModificationForm.java | 3 ++- .../account/certs/RevokeSingleCertForm.java | 3 ++- tests/club/wpia/gigi/TestCertificate.java | 5 ++-- .../club/wpia/gigi/util/SimpleSigner.java | 12 +++++----- 13 files changed, 75 insertions(+), 27 deletions(-) create mode 100644 src/club/wpia/gigi/database/upgrade/from_28.sql diff --git a/src/club/wpia/gigi/api/RevokeCertificate.java b/src/club/wpia/gigi/api/RevokeCertificate.java index 1b21e240..5259fb02 100644 --- a/src/club/wpia/gigi/api/RevokeCertificate.java +++ b/src/club/wpia/gigi/api/RevokeCertificate.java @@ -6,9 +6,10 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import club.wpia.gigi.dbObjects.Certificate; +import club.wpia.gigi.dbObjects.Certificate.CertificateStatus; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.dbObjects.Job; import club.wpia.gigi.dbObjects.User; -import club.wpia.gigi.dbObjects.Certificate.CertificateStatus; public class RevokeCertificate extends APIPoint { @@ -39,7 +40,7 @@ public class RevokeCertificate extends APIPoint { return; } - Job job = c.revoke(); + Job job = c.revoke(RevocationType.USER); job.waitFor(60000); if (c.getStatus() != CertificateStatus.REVOKED) { resp.sendError(510, "Error, issuing timed out"); diff --git a/src/club/wpia/gigi/database/DatabaseConnection.java b/src/club/wpia/gigi/database/DatabaseConnection.java index 819dcf93..b7b9c14f 100644 --- a/src/club/wpia/gigi/database/DatabaseConnection.java +++ b/src/club/wpia/gigi/database/DatabaseConnection.java @@ -122,7 +122,7 @@ public class DatabaseConnection { } - public static final int CURRENT_SCHEMA_VERSION = 28; + public static final int CURRENT_SCHEMA_VERSION = 29; public static final int CONNECTION_TIMEOUT = 24 * 60 * 60; diff --git a/src/club/wpia/gigi/database/tableStructure.sql b/src/club/wpia/gigi/database/tableStructure.sql index 4b5194f7..0662bf18 100644 --- a/src/club/wpia/gigi/database/tableStructure.sql +++ b/src/club/wpia/gigi/database/tableStructure.sql @@ -159,7 +159,6 @@ CREATE TABLE "certs" ( "crt_name" varchar(255) NOT NULL DEFAULT '', "created" timestamp NULL DEFAULT NULL, "modified" timestamp NULL DEFAULT NULL, - "revoked" timestamp NULL DEFAULT NULL, "expire" timestamp NULL DEFAULT NULL, "renewed" boolean NOT NULL DEFAULT 'false', "pkhash" char(40) DEFAULT NULL, @@ -168,13 +167,23 @@ CREATE TABLE "certs" ( PRIMARY KEY ("id") ); CREATE INDEX ON "certs" ("pkhash"); -CREATE INDEX ON "certs" ("revoked"); CREATE INDEX ON "certs" ("created"); CREATE INDEX ON "certs" ("memid"); CREATE INDEX ON "certs" ("serial"); CREATE INDEX ON "certs" ("expire"); CREATE INDEX ON "certs" ("crt_name"); +DROP TABLE IF EXISTS "certsRevoked"; +DROP TYPE IF EXISTS "revocationType"; +CREATE TYPE "revocationType" AS ENUM('user', 'support', 'ping_timeout'); +CREATE TABLE "certsRevoked" ( + "id" int NOT NULL, + -- the time when the certificate was revoked by cassiopeia (and that is stored in the CRL) + -- NULL indicated the revocation is pending + "revoked" timestamp NULL, + "type" "revocationType" NOT NULL, + PRIMARY KEY ("id") +); DROP TABLE IF EXISTS "certAvas"; @@ -375,7 +384,7 @@ CREATE TABLE "schemeVersion" ( "version" smallint NOT NULL, PRIMARY KEY ("version") ); -INSERT INTO "schemeVersion" (version) VALUES(28); +INSERT INTO "schemeVersion" (version) VALUES(29); DROP TABLE IF EXISTS `passwordResetTickets`; CREATE TABLE `passwordResetTickets` ( diff --git a/src/club/wpia/gigi/database/upgrade/from_28.sql b/src/club/wpia/gigi/database/upgrade/from_28.sql new file mode 100644 index 00000000..190a7c93 --- /dev/null +++ b/src/club/wpia/gigi/database/upgrade/from_28.sql @@ -0,0 +1,12 @@ +CREATE TYPE "revocationType" AS ENUM('user', 'support', 'ping_timeout'); +CREATE TABLE "certsRevoked" ( + "id" int NOT NULL, + "revoked" timestamp NULL, + "type" "revocationType" NOT NULL, + PRIMARY KEY ("id") +); + +INSERT INTO "certsRevoked" (SELECT "id", "revoked", 'user' AS "type" FROM "certs" WHERE "revoked" IS NOT NULL); +INSERT INTO "certsRevoked" (SELECT "targetId" AS "id", NULL AS "revoked", 'user' AS "type" FROM "jobs" WHERE "task" = 'revoke' AND "state" != 'done'); + +ALTER TABLE "certs" DROP COLUMN "revoked"; diff --git a/src/club/wpia/gigi/dbObjects/Certificate.java b/src/club/wpia/gigi/dbObjects/Certificate.java index ffc51547..c3ff5ab1 100644 --- a/src/club/wpia/gigi/dbObjects/Certificate.java +++ b/src/club/wpia/gigi/dbObjects/Certificate.java @@ -27,6 +27,21 @@ import club.wpia.gigi.util.KeyStorage; public class Certificate implements IdCachable { + public enum RevocationType implements DBEnum { + USER("user"), SUPPORT("support"), PING_TIMEOUT("ping_timeout"); + + private final String dbName; + + private RevocationType(String dbName) { + this.dbName = dbName; + } + + @Override + public String getDBName() { + return dbName; + } + } + public enum SANType implements DBEnum { EMAIL("email"), DNS("DNS"); @@ -282,7 +297,7 @@ public class Certificate implements IdCachable { } public synchronized CertificateStatus getStatus() { - try (GigiPreparedStatement searcher = new GigiPreparedStatement("SELECT crt_name, created, revoked, serial, caid FROM certs WHERE id=?")) { + try (GigiPreparedStatement searcher = new GigiPreparedStatement("SELECT crt_name, created, `revoked`, serial, caid FROM certs LEFT JOIN `certsRevoked` ON `certs`.`id` = `certsRevoked`.`id` WHERE `certs`.id=?")) { searcher.setInt(1, id); GigiResultSet rs = searcher.executeQuery(); if ( !rs.next()) { @@ -325,11 +340,11 @@ public class Certificate implements IdCachable { } - public Job revoke() { + public Job revoke(RevocationType type) { if (getStatus() != CertificateStatus.ISSUED) { throw new IllegalStateException(); } - return Job.revoke(this); + return Job.revoke(this, type); } @@ -467,7 +482,7 @@ public class Certificate implements IdCachable { public java.util.Date getRevocationDate() { if (getStatus() == CertificateStatus.REVOKED) { - try (GigiPreparedStatement prep = new GigiPreparedStatement("SELECT revoked FROM certs WHERE id=?")) { + try (GigiPreparedStatement prep = new GigiPreparedStatement("SELECT revoked FROM `certsRevoked` WHERE id=?")) { prep.setInt(1, getId()); GigiResultSet res = prep.executeQuery(); if (res.next()) { diff --git a/src/club/wpia/gigi/dbObjects/CertificateOwner.java b/src/club/wpia/gigi/dbObjects/CertificateOwner.java index 1c7cdb4a..798f6e40 100644 --- a/src/club/wpia/gigi/dbObjects/CertificateOwner.java +++ b/src/club/wpia/gigi/dbObjects/CertificateOwner.java @@ -83,7 +83,7 @@ public abstract class CertificateOwner implements IdCachable, Serializable { } public Certificate[] getCertificates(boolean includeRevoked) { - try (GigiPreparedStatement ps = new GigiPreparedStatement(includeRevoked ? "SELECT id FROM certs WHERE memid=? ORDER BY id DESC" : "SELECT id FROM certs WHERE memid=? AND revoked IS NULL ORDER BY id DESC")) { + try (GigiPreparedStatement ps = new GigiPreparedStatement(includeRevoked ? "SELECT id FROM certs WHERE memid=? ORDER BY id DESC" : "SELECT id FROM certs WHERE memid=? AND NOT EXISTS (SELECT 1 FROM `certsRevoked` WHERE `certsRevoked`.`id` = `certs`.`id` AND `certsRevoked`.`revoked` IS NOT NULL) ORDER BY id DESC")) { ps.setInt(1, getId()); GigiResultSet rs = ps.executeQuery(); @@ -132,7 +132,7 @@ public abstract class CertificateOwner implements IdCachable, Serializable { } public static CertificateOwner getByEnabledSerial(String serial) { - try (GigiPreparedStatement prep = new GigiPreparedStatement("SELECT `memid` FROM `certs` INNER JOIN `logincerts` ON `logincerts`.`id`=`certs`.`id` WHERE serial=? AND `revoked` is NULL")) { + try (GigiPreparedStatement prep = new GigiPreparedStatement("SELECT `memid` FROM `certs` INNER JOIN `logincerts` ON `logincerts`.`id`=`certs`.`id` WHERE serial=? AND NOT EXISTS (SELECT 1 FROM `certsRevoked` WHERE `certsRevoked`.`id` = `certs`.`id` AND `certsRevoked`.`revoked` IS NOT NULL)")) { prep.setString(1, serial); GigiResultSet res = prep.executeQuery(); if (res.next()) { diff --git a/src/club/wpia/gigi/dbObjects/Job.java b/src/club/wpia/gigi/dbObjects/Job.java index ea8fc97d..8941e38a 100644 --- a/src/club/wpia/gigi/dbObjects/Job.java +++ b/src/club/wpia/gigi/dbObjects/Job.java @@ -6,6 +6,7 @@ import club.wpia.gigi.GigiApiException; import club.wpia.gigi.database.DBEnum; import club.wpia.gigi.database.GigiPreparedStatement; import club.wpia.gigi.database.GigiResultSet; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.output.CertificateValiditySelector; public class Job implements IdCachable { @@ -31,7 +32,7 @@ public class Job implements IdCachable { } } - public synchronized static Job sign(Certificate targetId, Date start, String period) throws GigiApiException { + protected synchronized static Job sign(Certificate targetId, Date start, String period) throws GigiApiException { CertificateValiditySelector.checkValidityLength(period); try (GigiPreparedStatement ps = new GigiPreparedStatement("INSERT INTO `jobs` SET targetId=?, task=?::`jobType`, executeFrom=?, executeTo=?")) { ps.setInt(1, targetId.getId()); @@ -43,7 +44,12 @@ public class Job implements IdCachable { } } - public synchronized static Job revoke(Certificate targetId) { + protected synchronized static Job revoke(Certificate targetId, RevocationType type) { + try (GigiPreparedStatement ps = new GigiPreparedStatement("INSERT INTO `certsRevoked` SET id=?, type=?::`revocationType`")) { + ps.setInt(1, targetId.getId()); + ps.setEnum(2, type); + ps.execute(); + } try (GigiPreparedStatement ps = new GigiPreparedStatement("INSERT INTO `jobs` SET targetId=?, task=?::`jobType`")) { ps.setInt(1, targetId.getId()); diff --git a/src/club/wpia/gigi/dbObjects/Organisation.java b/src/club/wpia/gigi/dbObjects/Organisation.java index 70299293..4ee25d0e 100644 --- a/src/club/wpia/gigi/dbObjects/Organisation.java +++ b/src/club/wpia/gigi/dbObjects/Organisation.java @@ -10,6 +10,7 @@ import club.wpia.gigi.GigiApiException; import club.wpia.gigi.database.GigiPreparedStatement; import club.wpia.gigi.database.GigiResultSet; import club.wpia.gigi.dbObjects.Certificate.CertificateStatus; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.dbObjects.Country.CountryCodeType; import club.wpia.gigi.dbObjects.wrappers.DataContainer; @@ -219,7 +220,7 @@ public class Organisation extends CertificateOwner { } for (Certificate cert : getCertificates(false)) { if (cert.getStatus() == CertificateStatus.ISSUED) { - cert.revoke(); + cert.revoke(RevocationType.USER); } } try (GigiPreparedStatement ps = new GigiPreparedStatement("UPDATE `organisations` SET `name`=?, `country`=?, `province`=?, `city`=? WHERE `id`=?")) { diff --git a/src/club/wpia/gigi/dbObjects/SupportedUser.java b/src/club/wpia/gigi/dbObjects/SupportedUser.java index c9364f4e..5d023e9b 100644 --- a/src/club/wpia/gigi/dbObjects/SupportedUser.java +++ b/src/club/wpia/gigi/dbObjects/SupportedUser.java @@ -9,6 +9,7 @@ import javax.servlet.http.HttpServletRequest; import club.wpia.gigi.GigiApiException; import club.wpia.gigi.database.GigiPreparedStatement; import club.wpia.gigi.dbObjects.Certificate.CertificateStatus; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.localisation.Language; import club.wpia.gigi.output.template.MailTemplate; import club.wpia.gigi.output.template.Outputable; @@ -54,7 +55,7 @@ public class SupportedUser { // TODO Check for open jobs! for (int i = 0; i < certs.length; i++) { if (certs[i].getStatus() == CertificateStatus.ISSUED) { - certs[i].revoke(); + certs[i].revoke(RevocationType.SUPPORT); } } // send notification to support @@ -69,7 +70,7 @@ public class SupportedUser { // TODO Check for open jobs! if (cert.getStatus() == CertificateStatus.ISSUED) { writeSELog("SE Revoke certificate"); - cert.revoke().waitFor(60000); + cert.revoke(RevocationType.SUPPORT).waitFor(60000); // send notification to support String subject = "Revoke certificate"; Outputable message = SprintfCommand.createSimple("Certificate with serial number {0} for {1} <{2}> has been revoked.", cert.getSerial(), target.getPreferredName().toString(), target.getEmail()); diff --git a/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java b/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java index 567f4cb3..6d6cc8b5 100644 --- a/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java +++ b/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java @@ -8,6 +8,7 @@ import javax.servlet.http.HttpServletRequest; import club.wpia.gigi.GigiApiException; import club.wpia.gigi.dbObjects.Certificate; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.dbObjects.CertificateOwner; import club.wpia.gigi.dbObjects.Job; import club.wpia.gigi.localisation.Language; @@ -48,7 +49,7 @@ public class CertificateModificationForm extends Form { if (c == null || c.getOwner() != target) { continue; } - revokes.add(c.revoke()); + revokes.add(c.revoke(RevocationType.SUPPORT)); } long start = System.currentTimeMillis(); for (Job job : revokes) { diff --git a/src/club/wpia/gigi/pages/account/certs/RevokeSingleCertForm.java b/src/club/wpia/gigi/pages/account/certs/RevokeSingleCertForm.java index 034d9577..30e404c0 100644 --- a/src/club/wpia/gigi/pages/account/certs/RevokeSingleCertForm.java +++ b/src/club/wpia/gigi/pages/account/certs/RevokeSingleCertForm.java @@ -7,6 +7,7 @@ import javax.servlet.http.HttpServletRequest; import club.wpia.gigi.GigiApiException; import club.wpia.gigi.dbObjects.Certificate; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.dbObjects.SupportedUser; import club.wpia.gigi.localisation.Language; import club.wpia.gigi.output.template.Form; @@ -31,7 +32,7 @@ public class RevokeSingleCertForm extends Form { if (target != null) { target.revokeCertificate(c); } else { - c.revoke().waitFor(60000); + c.revoke(RevocationType.USER).waitFor(60000); } return new RedirectResult(req.getPathInfo()); } diff --git a/tests/club/wpia/gigi/TestCertificate.java b/tests/club/wpia/gigi/TestCertificate.java index 53b6509d..058c029e 100644 --- a/tests/club/wpia/gigi/TestCertificate.java +++ b/tests/club/wpia/gigi/TestCertificate.java @@ -17,6 +17,7 @@ import org.junit.Test; import club.wpia.gigi.dbObjects.Certificate; import club.wpia.gigi.dbObjects.Certificate.CSRType; import club.wpia.gigi.dbObjects.Certificate.CertificateStatus; +import club.wpia.gigi.dbObjects.Certificate.RevocationType; import club.wpia.gigi.dbObjects.Certificate.SANType; import club.wpia.gigi.dbObjects.Certificate.SubjectAlternateName; import club.wpia.gigi.dbObjects.Digest; @@ -120,7 +121,7 @@ public class TestCertificate extends ManagedTest { assertNotNull(login(pk, cert)); assertEquals(1, countRegex(IOUtils.readURL(get(cookie, Certificates.PATH)), "(?:REVOKED|ISSUED)")); assertEquals(1, countRegex(IOUtils.readURL(get(cookie, Certificates.PATH + "?withRevoked")), "(?:REVOKED|ISSUED)")); - await(c.revoke()); + await(c.revoke(RevocationType.USER)); testFails(CertificateStatus.REVOKED, c); assertNull(login(pk, cert)); @@ -133,7 +134,7 @@ public class TestCertificate extends ManagedTest { assertEquals(status, c.getStatus()); if (status != CertificateStatus.ISSUED) { try { - c.revoke(); + c.revoke(RevocationType.USER); fail(status + " is in invalid state"); } catch (IllegalStateException ise) { diff --git a/util-testing/club/wpia/gigi/util/SimpleSigner.java b/util-testing/club/wpia/gigi/util/SimpleSigner.java index 070194e4..495e720c 100644 --- a/util-testing/club/wpia/gigi/util/SimpleSigner.java +++ b/util-testing/club/wpia/gigi/util/SimpleSigner.java @@ -138,7 +138,7 @@ public class SimpleSigner { warnMail = new GigiPreparedStatement("UPDATE jobs SET warning=warning+1, state=CASE WHEN warning<3 THEN 'open'::`jobState` ELSE 'error'::`jobState` END WHERE id=?"); revoke = new GigiPreparedStatement("SELECT certs.id, certs.csr_name,jobs.id FROM jobs INNER JOIN certs ON jobs.`targetId`=certs.id" + " WHERE jobs.state='open' AND task='revoke'"); - revokeCompleted = new GigiPreparedStatement("UPDATE certs SET revoked=NOW() WHERE id=?"); + revokeCompleted = new GigiPreparedStatement("UPDATE `certsRevoked` SET revoked=NOW() WHERE id=?"); finishJob = new GigiPreparedStatement("UPDATE jobs SET state='done' WHERE id=?"); @@ -199,9 +199,9 @@ public class SimpleSigner { worked = true; System.out.println("Revoke faked: " + id); revokeCompleted.setInt(1, id); - revokeCompleted.execute(); + revokeCompleted.executeUpdate(); finishJob.setInt(1, rs.getInt(3)); - finishJob.execute(); + finishJob.executeUpdate(); } if (worked) { gencrl(); @@ -371,10 +371,10 @@ public class SimpleSigner { updateMail.setInt(3, caRs.getInt("id")); updateMail.setTimestamp(4, new Timestamp(toDate.getTime())); updateMail.setInt(5, id); - updateMail.execute(); + updateMail.executeUpdate(); finishJob.setInt(1, rs.getInt("jobid")); - finishJob.execute(); + finishJob.executeUpdate(); System.out.println("signed: " + id); continue; } @@ -388,7 +388,7 @@ public class SimpleSigner { } System.out.println("Error with: " + id); warnMail.setInt(1, rs.getInt("jobid")); - warnMail.execute(); + warnMail.executeUpdate(); } rs.close(); -- 2.39.5