From 6dde34e6434dd384ae298cdd4b18fc2036494ae6 Mon Sep 17 00:00:00 2001 From: Albert Louis Rossi Date: Thu, 29 Jun 2023 12:33:19 -0500 Subject: [PATCH] dcache-qos(verifier,engine): fix incompatibility issues with Subject and attributes Motivation: See RT 10469 QoS change is not working. This brought to my attention two incompatibilities which require a bit more defensive code. 1. https://github.com/dCache/dcache-security-fixes/tree/fix/master/qos-propagate-subject-to-adjuster introduced a subject field/column in the `qos-operation` table to enforce certain kinds of restrictions. However, it neglected to consider the case where a pre-existing table would be modified by Liquibase and the update would fill that field with `null` values, causing a deserialization error when refetching the metadata. 2. The `ALRPStorageUnitQoSProvider` does not know how to handle undefined AL or RP values (it expects the file attributes always to be defined). Modification: 1. Just return a null for the Subject and let the operation fail if it is a QoSModify. The operation should be removed from the database, and a subsequent retry should work. 2. Inject the default values into the class and use those when the FileAttributes do not contain them. Result: Two small incompatibilities fixed. Target: master Request: 9.1 Request: 9.0 Request: 8.2 Patch: https://rb.dcache.org/r/13997/ Bug: #10469 (RT) Requires-notes: yes Requires-book: no Acked-by: Marina --- .../engine/provider/ALRPStorageUnitQoSProvider.java | 4 ++-- .../verifier/data/db/JdbcVerifyOperationDao.java | 6 ++++++ .../java/org/dcache/qos/util/QoSPermissionUtils.java | 12 ++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/provider/ALRPStorageUnitQoSProvider.java b/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/provider/ALRPStorageUnitQoSProvider.java index d3c91d20c47..2831e6a355e 100644 --- a/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/provider/ALRPStorageUnitQoSProvider.java +++ b/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/provider/ALRPStorageUnitQoSProvider.java @@ -159,8 +159,8 @@ public FileQoSRequirements fetchRequirements(FileQoSUpdate update) throws QoSExc } FileAttributes attributes = descriptor.getAttributes(); - AccessLatency accessLatency = attributes.getAccessLatency(); - RetentionPolicy retentionPolicy = attributes.getRetentionPolicy(); + AccessLatency accessLatency = attributes.getAccessLatencyIfPresent().orElse(null); + RetentionPolicy retentionPolicy = attributes.getRetentionPolicyIfPresent().orElse(null); String unitKey = attributes.getStorageClass() + "@" + attributes.getHsm(); StorageUnit storageUnit = poolSelectionUnit().getStorageUnit(unitKey); diff --git a/modules/dcache-qos/src/main/java/org/dcache/qos/services/verifier/data/db/JdbcVerifyOperationDao.java b/modules/dcache-qos/src/main/java/org/dcache/qos/services/verifier/data/db/JdbcVerifyOperationDao.java index c39754d3f36..91c0a152bbb 100644 --- a/modules/dcache-qos/src/main/java/org/dcache/qos/services/verifier/data/db/JdbcVerifyOperationDao.java +++ b/modules/dcache-qos/src/main/java/org/dcache/qos/services/verifier/data/db/JdbcVerifyOperationDao.java @@ -118,6 +118,9 @@ private static VerifyOperation toOperation(ResultSet rs, int row) throws SQLExce } private static String serialize(Subject subject) throws QoSException { + if (subject == null) { + return null; + } ByteArrayOutputStream baos = new ByteArrayOutputStream(); try (ObjectOutputStream ostream = new ObjectOutputStream(baos)) { ostream.writeObject(subject); @@ -128,6 +131,9 @@ private static String serialize(Subject subject) throws QoSException { } private static Object deserialize(String base64) throws SQLException { + if (base64 == null) { + return null; + } byte[] array = Base64.getDecoder().decode(base64); ByteArrayInputStream bais = new ByteArrayInputStream(array); try (ObjectInputStream istream = new ObjectInputStream(bais)) { diff --git a/modules/dcache-qos/src/main/java/org/dcache/qos/util/QoSPermissionUtils.java b/modules/dcache-qos/src/main/java/org/dcache/qos/util/QoSPermissionUtils.java index a941a9e8071..daa4f19c1ad 100644 --- a/modules/dcache-qos/src/main/java/org/dcache/qos/util/QoSPermissionUtils.java +++ b/modules/dcache-qos/src/main/java/org/dcache/qos/util/QoSPermissionUtils.java @@ -74,6 +74,18 @@ public class QoSPermissionUtils { * @param attributes */ public static boolean canModifyQos(Subject subject, FileAttributes attributes) { + if (subject == null) { + /* + * This is a workaround for legacy database entries before + * https://github.com/dCache/dcache-security-fixes/tree/fix/master/qos-propagate-subject-to-adjuster + * was introduced. An incompatibility was overlooked whereby + * the database could contain entries when updated by liquibase, + * thus making the new subject field null. + * + * In this case we just return false. + */ + return false; + } return Subjects.isRoot(subject) || Subjects.getUid(subject) == attributes.getOwner(); }