[Pki-devel] [PATCH] 0057..0061 LDAPProfileSubsystem concurrency fixes
Fraser Tweedale
ftweedal at redhat.com
Tue Dec 1 04:56:51 UTC 2015
The attached patches resolve concurrency issues in the
LDAPProfileSubsystem (which conducts its LDAP persisent search in
background thread).
Ticket: https://fedorahosted.org/pki/ticket/1700
Thanks,
Fraser
-------------- next part --------------
From 9b3408e446811df06ee83fc4c8078a21bc8a1063 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Fri, 27 Nov 2015 14:31:18 +1100
Subject: [PATCH 57/61] Extract LDAPControl search function to LDAPUtil
---
.../netscape/cmscore/profile/LDAPProfileSubsystem.java | 15 ++++-----------
base/util/src/com/netscape/cmsutil/ldap/LDAPUtil.java | 18 ++++++++++++++++++
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
index cc2e43dfa2e9b1a4eb3bdb53eeb3ace6cfd1d6ac..7be70dff1315c150422be303d7be50fb2fb245f0 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
@@ -25,7 +25,6 @@ import java.util.LinkedHashMap;
import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPConnection;
-import netscape.ldap.LDAPControl;
import netscape.ldap.LDAPDN;
import netscape.ldap.LDAPEntry;
import netscape.ldap.LDAPException;
@@ -47,6 +46,7 @@ import com.netscape.certsrv.profile.IProfileSubsystem;
import com.netscape.certsrv.registry.IPluginInfo;
import com.netscape.certsrv.registry.IPluginRegistry;
import com.netscape.cmscore.base.LDAPConfigStore;
+import com.netscape.cmsutil.ldap.LDAPUtil;
public class LDAPProfileSubsystem
extends AbstractProfileSubsystem
@@ -290,16 +290,9 @@ public class LDAPProfileSubsystem
null, false, cons);
while (!stopped && results.hasMoreElements()) {
LDAPEntry entry = results.next();
- LDAPEntryChangeControl changeControl = null;
- LDAPControl[] changeControls = results.getResponseControls();
- if (changeControls != null) {
- for (LDAPControl control : changeControls) {
- if (control instanceof LDAPEntryChangeControl) {
- changeControl = (LDAPEntryChangeControl) control;
- break;
- }
- }
- }
+ LDAPEntryChangeControl changeControl = (LDAPEntryChangeControl)
+ LDAPUtil.getControl(
+ LDAPEntryChangeControl.class, results.getResponseControls());
CMS.debug("Profile change monitor: Processed change controls.");
if (changeControl != null) {
int changeType = changeControl.getChangeType();
diff --git a/base/util/src/com/netscape/cmsutil/ldap/LDAPUtil.java b/base/util/src/com/netscape/cmsutil/ldap/LDAPUtil.java
index b02ffee787121b252e9f97e3c0358a200c8940ac..f8162235d191763eb17d798096804b7ba55a9840 100644
--- a/base/util/src/com/netscape/cmsutil/ldap/LDAPUtil.java
+++ b/base/util/src/com/netscape/cmsutil/ldap/LDAPUtil.java
@@ -18,11 +18,13 @@
package com.netscape.cmsutil.ldap;
import java.io.IOException;
+import java.lang.Class;
import java.util.ArrayList;
import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPAttributeSet;
import netscape.ldap.LDAPConnection;
+import netscape.ldap.LDAPControl;
import netscape.ldap.LDAPEntry;
import netscape.ldap.LDAPException;
import netscape.ldap.LDAPModification;
@@ -145,4 +147,20 @@ public class LDAPUtil {
}
}
}
+
+ /**
+ * Get the control of the specified class from the array of controls.
+ *
+ * @return the LDAPControl, or null if not found
+ */
+ public static LDAPControl getControl(
+ Class<? extends LDAPControl> cls, LDAPControl[] controls) {
+ if (controls != null) {
+ for (LDAPControl control : controls) {
+ if (cls.isInstance(control))
+ return control;
+ }
+ }
+ return null;
+ }
}
--
2.4.3
-------------- next part --------------
From 9b3c2abb8f7c1089b4653dd2fee8d0c9f557dde6 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Mon, 30 Nov 2015 14:01:38 +1100
Subject: [PATCH 58/61] Add LDAPPostReadControl class
The LDAPPostReadControl can be used to read an entry after perfoming
an add, modify or modrdn, giving atomic access to operational
attributes.
Part of: https://fedorahosted.org/pki/ticket/1700
---
.../netscape/cmsutil/ldap/LDAPPostReadControl.java | 109 +++++++++++++++++++++
1 file changed, 109 insertions(+)
create mode 100644 base/util/src/com/netscape/cmsutil/ldap/LDAPPostReadControl.java
diff --git a/base/util/src/com/netscape/cmsutil/ldap/LDAPPostReadControl.java b/base/util/src/com/netscape/cmsutil/ldap/LDAPPostReadControl.java
new file mode 100644
index 0000000000000000000000000000000000000000..501c476ef87eeac6bb5551b246466a5fe62f764e
--- /dev/null
+++ b/base/util/src/com/netscape/cmsutil/ldap/LDAPPostReadControl.java
@@ -0,0 +1,109 @@
+// --- BEGIN COPYRIGHT BLOCK ---
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; version 2 of the License.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+//
+// (C) 2015 Red Hat, Inc.
+// All rights reserved.
+// --- END COPYRIGHT BLOCK ---
+package com.netscape.cmsutil.ldap;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+
+import netscape.ldap.LDAPAttribute;
+import netscape.ldap.LDAPAttributeSet;
+import netscape.ldap.LDAPControl;
+import netscape.ldap.LDAPEntry;
+import netscape.ldap.LDAPException;
+import netscape.ldap.ber.stream.BERElement;
+import netscape.ldap.ber.stream.BEROctetString;
+import netscape.ldap.ber.stream.BERSequence;
+import netscape.ldap.ber.stream.BERTag;
+import netscape.ldap.client.JDAPBERTagDecoder;
+
+public class LDAPPostReadControl extends LDAPControl {
+
+ private static final long serialVersionUID = -3988578305868188089L;
+
+ public final static String OID_POSTREAD = "1.3.6.1.1.13.2";
+
+ private LDAPEntry entry = null;
+
+ static {
+ try {
+ register(OID_POSTREAD, LDAPPostReadControl.class);
+ } catch (Throwable e) {
+ }
+ }
+
+ /**
+ * Response control constructor.
+ *
+ * This is called automatically by response processing code,
+ * should not need to be called by user.
+ */
+ public LDAPPostReadControl(String oid, boolean critical, byte[] value)
+ throws LDAPException, IOException {
+ super(OID_POSTREAD, critical, value);
+ if (!oid.equals(OID_POSTREAD)) {
+ throw new LDAPException(
+ "oid must be LDAPPostReadControl.OID_POSTREAD",
+ LDAPException.PARAM_ERROR);
+ }
+
+ ByteArrayInputStream in = new ByteArrayInputStream(value);
+ int[] numRead = new int[1];
+ BERTag tag = (BERTag)
+ BERElement.getElement(new JDAPBERTagDecoder(), in, numRead);
+ BERSequence seq = (BERSequence)tag.getValue();
+
+ BEROctetString name = (BEROctetString)seq.elementAt(0);
+ byte buf[] = name.getValue();
+ String dn = null;
+ if (buf != null) {
+ try {
+ dn = new String(buf, "UTF8");
+ } catch(Throwable e) {
+ }
+ }
+
+ BERSequence attrs = (BERSequence)seq.elementAt(1);
+ LDAPAttributeSet attrSet = new LDAPAttributeSet();
+ for (int i = 0; i < attrs.size(); i++) {
+ attrSet.add(new LDAPAttribute(attrs.elementAt(i)));
+ }
+
+ entry = new LDAPEntry(dn, attrSet);
+ }
+
+ /**
+ * Request control constructor.
+ */
+ public LDAPPostReadControl(boolean critical, String[] attrs) {
+ super(OID_POSTREAD, critical, null);
+ BERSequence ber_attrs = new BERSequence();
+ for (int i = 0; i < attrs.length; i++) {
+ ber_attrs.addElement(new BEROctetString(attrs[i]));
+ }
+ m_value = flattenBER(ber_attrs);
+ }
+
+ /**
+ * Get the entry from the control.
+ *
+ * Returns null if constructed as a request control.
+ */
+ public LDAPEntry getEntry() {
+ return entry;
+ }
+};
--
2.4.3
-------------- next part --------------
From e585953a1c8dc32b74ba1a1293286e2d555114eb Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Mon, 30 Nov 2015 14:04:08 +1100
Subject: [PATCH 59/61] Avoid profile race conditions by tracking entryUSN
Avoid race conditions in the LDAPProfileSubsystem by tracking the
most recently known entryUSN of profiles' LDAP entries.
As part of this change, add the commitProfile method to the
IProfileSubsystem interface, remove commit behaviour from the
enableProfile and disableProfile methods and update ProfileService
and ProfileApproveServlet to commit the profile (using the
commitProfile method) where needed.
Part of: https://fedorahosted.org/pki/ticket/1700
---
.../dogtagpki/server/ca/rest/ProfileService.java | 12 +++--
.../certsrv/profile/IProfileSubsystem.java | 5 ++
.../cms/servlet/profile/ProfileApproveServlet.java | 3 ++
.../com/netscape/cmscore/base/LDAPConfigStore.java | 57 ++++++++++++++++------
.../cmscore/profile/AbstractProfileSubsystem.java | 15 ++++--
.../cmscore/profile/LDAPProfileSubsystem.java | 56 ++++++++++++++++++---
6 files changed, 118 insertions(+), 30 deletions(-)
diff --git a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
index 488dd5ab960fd45fa3dad0dd83398b4317f2cb4f..807c3f98bd4df57d33b8e427b5d3f59b522c2c0b 100644
--- a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
+++ b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
@@ -419,6 +419,7 @@ public class ProfileService extends PKIService implements ProfileResource {
}
try {
ps.enableProfile(profileId, principal.getName());
+ ps.commitProfile(profileId);
auditProfileChangeState(profileId, "approve", ILogger.SUCCESS);
} catch (EProfileException e) {
CMS.debug("modifyProfileState: error enabling profile. " + e);
@@ -436,6 +437,7 @@ public class ProfileService extends PKIService implements ProfileResource {
if (ps.checkOwner()) {
if (ps.getProfileEnableBy(profileId).equals(userid)) {
ps.disableProfile(profileId);
+ ps.commitProfile(profileId);
auditProfileChangeState(profileId, "disapprove", ILogger.SUCCESS);
} else {
auditProfileChangeState(profileId, "disapprove", ILogger.FAILURE);
@@ -444,6 +446,7 @@ public class ProfileService extends PKIService implements ProfileResource {
}
} else {
ps.disableProfile(profileId);
+ ps.commitProfile(profileId);
auditProfileChangeState(profileId, "disapprove", ILogger.SUCCESS);
}
} catch (EProfileException e) {
@@ -493,7 +496,7 @@ public class ProfileService extends PKIService implements ProfileResource {
profile.setName(getLocale(headers), data.getName());
profile.setDescription(getLocale(headers), data.getDescription());
profile.setVisible(data.isVisible());
- profile.getConfigStore().commit(false);
+ ps.commitProfile(profileId);
if (profile instanceof IProfileEx) {
// populates profile specific plugins such as
@@ -606,7 +609,8 @@ public class ProfileService extends PKIService implements ProfileResource {
// no error thrown, proceed with profile creation
profile = ps.createProfile(profileId, classId, className);
profile.getConfigStore().load(new ByteArrayInputStream(data));
- ps.disableProfile(profileId); // also commits
+ ps.disableProfile(profileId);
+ ps.commitProfile(profileId);
auditProfileChange(
ScopeDef.SC_PROFILE_RULES,
@@ -740,7 +744,7 @@ public class ProfileService extends PKIService implements ProfileResource {
// no error thrown, so commit updated profile config
profile.getConfigStore().load(new ByteArrayInputStream(data));
ps.disableProfile(profileId);
- profile.getConfigStore().commit(false);
+ ps.commitProfile(profileId);
return createOKResponse(data);
} catch (EBaseException | IOException e) {
@@ -817,7 +821,7 @@ public class ProfileService extends PKIService implements ProfileResource {
populateProfileInputs(data, profile);
populateProfileOutputs(data, profile);
populateProfilePolicies(data, profile);
- profile.getConfigStore().commit(false);
+ ps.commitProfile(profileId);
} catch (EBaseException e) {
CMS.debug("changeProfileData: Error changing profile inputs/outputs/policies: " + e);
e.printStackTrace();
diff --git a/base/common/src/com/netscape/certsrv/profile/IProfileSubsystem.java b/base/common/src/com/netscape/certsrv/profile/IProfileSubsystem.java
index b7071fe7526132d7f9ff1945819f0d1f67c18719..b7b06b92b92bf74f9d7ce9fd504a9e99fdd4839c 100644
--- a/base/common/src/com/netscape/certsrv/profile/IProfileSubsystem.java
+++ b/base/common/src/com/netscape/certsrv/profile/IProfileSubsystem.java
@@ -94,6 +94,11 @@ public interface IProfileSubsystem extends ISubsystem {
throws EProfileException;
/**
+ * Commit a profile's underlying config store.
+ */
+ public void commitProfile(String id) throws EProfileException;
+
+ /**
* Retrieves the id of the implementation of the given profile.
*
* @param id profile id
diff --git a/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileApproveServlet.java b/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileApproveServlet.java
index 7ae623f32eeae02290432e7e218ade93ce038213..89ba1bd8c785a745f0dd64a1e4be97626fd0e251 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileApproveServlet.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileApproveServlet.java
@@ -267,6 +267,7 @@ public class ProfileApproveServlet extends ProfileServlet {
if (ps.checkOwner()) {
if (ps.getProfileEnableBy(profileId).equals(userid)) {
ps.disableProfile(profileId);
+ ps.commitProfile(profileId);
} else {
// only enableBy can disable profile
args.set(ARG_ERROR_CODE, "1");
@@ -288,9 +289,11 @@ public class ProfileApproveServlet extends ProfileServlet {
}
} else {
ps.disableProfile(profileId);
+ ps.commitProfile(profileId);
}
} else {
ps.enableProfile(profileId, userid);
+ ps.commitProfile(profileId);
}
// store a message in the signed audit log file
diff --git a/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java b/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java
index b7b4ca46ee3e4cebfd169338dede3c9f64a45410..0b4ff707dddee6bd40447b85219f5a84edfe76db 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java
@@ -26,13 +26,17 @@ import java.util.Map;
import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPAttributeSet;
import netscape.ldap.LDAPConnection;
+import netscape.ldap.LDAPConstraints;
+import netscape.ldap.LDAPControl;
import netscape.ldap.LDAPEntry;
import netscape.ldap.LDAPException;
import netscape.ldap.LDAPModification;
-import com.netscape.certsrv.base.EBaseException;
import com.netscape.certsrv.base.IConfigStore;
+import com.netscape.certsrv.ldap.ELdapException;
import com.netscape.certsrv.ldap.ILdapConnFactory;
+import com.netscape.cmsutil.ldap.LDAPPostReadControl;
+import com.netscape.cmsutil.ldap.LDAPUtil;
/**
* LDAPConfigStore:
@@ -65,8 +69,6 @@ public class LDAPConfigStore extends PropConfigStore implements IConfigStore {
* @param attr Name of attribute containing config store
* @param createAttrs Set of initial attributes if creating the entry. Should
* contain cn, objectclass and possibly other attributes.
- *
- * @exception EBaseException failed to create file configuration
*/
public LDAPConfigStore(
ILdapConnFactory dbFactory,
@@ -102,7 +104,17 @@ public class LDAPConfigStore extends PropConfigStore implements IConfigStore {
*
* @param createBackup Ignored.
*/
- public void commit(boolean createBackup) throws EBaseException {
+ public void commit(boolean createBackup) throws ELdapException {
+ String[] attrs = {};
+ commitReturn(createBackup, attrs);
+ }
+
+ /**
+ * This version of commit also returns the post-read entry that
+ * the change resulted in.
+ */
+ public LDAPEntry commitReturn(boolean createBackup, String[] attrs)
+ throws ELdapException {
ByteArrayOutputStream data = new ByteArrayOutputStream();
save(data, null);
@@ -110,26 +122,37 @@ public class LDAPConfigStore extends PropConfigStore implements IConfigStore {
LDAPConnection conn = dbFactory.getConn();
+ LDAPConstraints cons = new LDAPConstraints();
+ cons.setServerControls(new LDAPPostReadControl(true, attrs));
+
+ LDAPControl[] responseControls;
+
// first attempt to modify; if modification fails (due
// to no such object), try and add the entry instead.
try {
try {
- commitModify(conn, configAttr);
+ commitModify(conn, configAttr, cons);
} catch (LDAPException e) {
if (e.getLDAPResultCode() == LDAPException.NO_SUCH_OBJECT) {
- commitAdd(conn, configAttr);
+ commitAdd(conn, configAttr, cons);
} else {
throw e;
}
}
+ responseControls = conn.getResponseControls();
} catch (LDAPException e) {
- throw new EBaseException(
+ throw new ELdapException(
"Error writing LDAPConfigStore '"
+ dn + "': " + e.toString()
);
} finally {
dbFactory.returnConn(conn);
}
+
+ LDAPPostReadControl control = (LDAPPostReadControl)
+ LDAPUtil.getControl(LDAPPostReadControl.class, responseControls);
+
+ return control.getEntry();
}
/**
@@ -139,12 +162,14 @@ public class LDAPConfigStore extends PropConfigStore implements IConfigStore {
* @param configAttr Config store attribute.
* @return true on success, false if the entry does not exist.
*/
- private void commitModify(LDAPConnection conn, LDAPAttribute configAttr)
- throws LDAPException
- {
+ private void commitModify(
+ LDAPConnection conn,
+ LDAPAttribute configAttr,
+ LDAPConstraints cons)
+ throws LDAPException {
LDAPModification ldapMod =
new LDAPModification(LDAPModification.REPLACE, configAttr);
- conn.modify(dn, ldapMod);
+ conn.modify(dn, ldapMod, cons);
}
/**
@@ -154,12 +179,14 @@ public class LDAPConfigStore extends PropConfigStore implements IConfigStore {
* @param configAttr Config store attribute.
* @return true on success, false if the entry already exists.
*/
- private void commitAdd(LDAPConnection conn, LDAPAttribute configAttr)
- throws LDAPException
- {
+ private void commitAdd(
+ LDAPConnection conn,
+ LDAPAttribute configAttr,
+ LDAPConstraints cons)
+ throws LDAPException {
LDAPAttributeSet attrSet = new LDAPAttributeSet(createAttrs);
attrSet.add(configAttr);
LDAPEntry ldapEntry = new LDAPEntry(dn, attrSet);
- conn.add(ldapEntry);
+ conn.add(ldapEntry, cons);
}
}
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
index cf5d77f191af93b05e71f0b1cce085a7d8161874..076e9c93e461f00e7a6dcde92f9fdd06d23a0283 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
@@ -95,10 +95,6 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
profile.getConfigStore().putString(PROP_ENABLE, "true");
profile.getConfigStore().putString(PROP_ENABLE_BY, enableBy);
- try {
- profile.getConfigStore().commit(false);
- } catch (EBaseException e) {
- }
}
/**
@@ -117,9 +113,18 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
IProfile profile = mProfiles.get(id);
profile.getConfigStore().putString(PROP_ENABLE, "false");
+ }
+
+ /**
+ * Commits a profile.
+ */
+ public void commitProfile(String id)
+ throws EProfileException {
try {
- profile.getConfigStore().commit(false);
+ mProfiles.get(id).getConfigStore().commit(false);
} catch (EBaseException e) {
+ throw new EProfileException(
+ "Failed to commit config store of profile '" + id + ": " + e);
}
}
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
index 7be70dff1315c150422be303d7be50fb2fb245f0..b16a33fe25881956051e2f65a5a99b73f7a624b4 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
@@ -19,9 +19,9 @@ package com.netscape.cmscore.profile;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
-import java.util.Enumeration;
import java.util.Hashtable;
import java.util.LinkedHashMap;
+import java.util.TreeMap;
import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPConnection;
@@ -58,6 +58,10 @@ public class LDAPProfileSubsystem
private boolean stopped = false;
private Thread monitor;
+ /* Map of profileId -> entryUSN for the most recent view
+ * of the profile entry that this instance has seen */
+ private TreeMap<String,Integer> entryUSNs;
+
/**
* Initializes this subsystem with the given configuration
* store.
@@ -74,6 +78,7 @@ public class LDAPProfileSubsystem
// (re)init member collections
mProfiles = new LinkedHashMap<String, IProfile>();
mProfileClassIds = new Hashtable<String, String>();
+ entryUSNs = new TreeMap<>();
IConfigStore cs = CMS.getConfigStore();
IConfigStore dbCfg = cs.getSubStore("internaldb");
@@ -101,7 +106,7 @@ public class LDAPProfileSubsystem
/**
* Read the given LDAPEntry into the profile subsystem.
*/
- private void readProfile(LDAPEntry ldapProfile) {
+ private synchronized void readProfile(LDAPEntry ldapProfile) {
IPluginRegistry registry = (IPluginRegistry)
CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
@@ -113,12 +118,24 @@ public class LDAPProfileSubsystem
}
profileId = LDAPDN.explodeDN(dn, true)[0];
+ Integer newEntryUSN = new Integer(
+ ldapProfile.getAttribute("entryUSN").getStringValueArray()[0]);
+ CMS.debug("readProfile: new entryUSN = " + newEntryUSN);
+
+ Integer knownEntryUSN = entryUSNs.get(profileId);
+ if (knownEntryUSN != null) {
+ CMS.debug("readProfile: known entryUSN = " + knownEntryUSN);
+ if (newEntryUSN <= knownEntryUSN) {
+ CMS.debug("readProfile: data is current");
+ return;
+ }
+ }
+
String classId = (String)
ldapProfile.getAttribute("classId").getStringValues().nextElement();
- Enumeration<String> vals =
- ldapProfile.getAttribute("certProfileConfig").getStringValues();
- InputStream data = new ByteArrayInputStream(vals.nextElement().getBytes());
+ InputStream data = new ByteArrayInputStream(
+ ldapProfile.getAttribute("certProfileConfig").getByteValueArray()[0]);
IPluginInfo info = registry.getPluginInfo("profile", classId);
if (info == null) {
@@ -127,6 +144,7 @@ public class LDAPProfileSubsystem
try {
CMS.debug("Start Profile Creation - " + profileId + " " + classId + " " + info.getClassName());
createProfile(profileId, classId, info.getClassName(), data);
+ entryUSNs.put(profileId, newEntryUSN);
CMS.debug("Done Profile Creation - " + profileId);
} catch (EProfileException e) {
CMS.debug("Error creating profile '" + profileId + "'; skipping.");
@@ -210,6 +228,29 @@ public class LDAPProfileSubsystem
readProfile(entry);
}
+ @Override
+ public synchronized void commitProfile(String id) throws EProfileException {
+ LDAPConfigStore cs = (LDAPConfigStore) mProfiles.get(id).getConfigStore();
+ try {
+ String[] attrs = {"entryUSN"};
+ LDAPEntry entry = cs.commitReturn(false, attrs);
+
+ LDAPAttribute attr = null;
+ if (entry != null)
+ attr = entry.getAttribute("entryUSN");
+
+ Integer entryUSN = null;
+ if (attr != null)
+ entryUSN = new Integer(attr.getStringValueArray()[0]);
+
+ entryUSNs.put(id, entryUSN);
+ CMS.debug("commitProfile: new entryUSN = " + entryUSN);
+ } catch (ELdapException e) {
+ throw new EProfileException(
+ "Failed to commit config store of profile '" + id + ": " + e);
+ }
+ }
+
/**
* Forget a profile without deleting it from the database.
*
@@ -219,6 +260,7 @@ public class LDAPProfileSubsystem
private void forgetProfile(String id) {
mProfiles.remove(id);
mProfileClassIds.remove(id);
+ entryUSNs.remove(id);
}
private void forgetProfile(LDAPEntry entry) {
@@ -253,6 +295,7 @@ public class LDAPProfileSubsystem
private void forgetAllProfiles() {
mProfiles.clear();
mProfileClassIds.clear();
+ entryUSNs.clear();
}
/**
@@ -285,9 +328,10 @@ public class LDAPProfileSubsystem
cons.setServerControls(persistCtrl);
cons.setBatchSize(1);
cons.setServerTimeLimit(0 /* seconds */);
+ String[] attrs = {"*", "entryUSN"};
LDAPSearchResults results = conn.search(
dn, LDAPConnection.SCOPE_ONE, "(objectclass=*)",
- null, false, cons);
+ attrs, false, cons);
while (!stopped && results.hasMoreElements()) {
LDAPEntry entry = results.next();
LDAPEntryChangeControl changeControl = (LDAPEntryChangeControl)
--
2.4.3
-------------- next part --------------
From 5fe0116aaa649de28f5770fda079072b87fdf21c Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Mon, 30 Nov 2015 16:05:03 +1100
Subject: [PATCH 60/61] Handle LDAPProfileSubsystem delete-then-recreate races
Deleting and then immediately recreating a profile can result in the
new profile temporarily going missing, if the DELETE
EntryChangeControl is processed after profile readdition.
Handle this case by tracking the nsUniqueId of entries that are
deleted by an LDAPProfileSubsystem and NOT (re-)forgetting the
profile when the subsequent EntryChangeControl gets processed.
Fixes: https://fedorahosted.org/pki/ticket/1700
---
.../cmscore/profile/LDAPProfileSubsystem.java | 112 +++++++++++++++++----
1 file changed, 92 insertions(+), 20 deletions(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
index b16a33fe25881956051e2f65a5a99b73f7a624b4..f48aea3918e6fc4a08c9bc90a433b834a29f35d3 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
@@ -22,6 +22,7 @@ import java.io.InputStream;
import java.util.Hashtable;
import java.util.LinkedHashMap;
import java.util.TreeMap;
+import java.util.TreeSet;
import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPConnection;
@@ -62,6 +63,11 @@ public class LDAPProfileSubsystem
* of the profile entry that this instance has seen */
private TreeMap<String,Integer> entryUSNs;
+ private TreeMap<String,String> nsUniqueIds;
+
+ /* Set of nsUniqueIds of deleted entries */
+ private TreeSet<String> deletedNsUniqueIds;
+
/**
* Initializes this subsystem with the given configuration
* store.
@@ -79,6 +85,8 @@ public class LDAPProfileSubsystem
mProfiles = new LinkedHashMap<String, IProfile>();
mProfileClassIds = new Hashtable<String, String>();
entryUSNs = new TreeMap<>();
+ nsUniqueIds = new TreeMap<>();
+ deletedNsUniqueIds = new TreeSet<>();
IConfigStore cs = CMS.getConfigStore();
IConfigStore dbCfg = cs.getSubStore("internaldb");
@@ -110,6 +118,14 @@ public class LDAPProfileSubsystem
IPluginRegistry registry = (IPluginRegistry)
CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
+ String nsUniqueId =
+ ldapProfile.getAttribute("nsUniqueId").getStringValueArray()[0];
+ if (deletedNsUniqueIds.contains(nsUniqueId)) {
+ CMS.debug("readProfile: ignoring entry with nsUniqueId '"
+ + nsUniqueId + "' due to deletion");
+ return;
+ }
+
String profileId = null;
String dn = ldapProfile.getDN();
if (!dn.startsWith("cn=")) {
@@ -145,6 +161,7 @@ public class LDAPProfileSubsystem
CMS.debug("Start Profile Creation - " + profileId + " " + classId + " " + info.getClassName());
createProfile(profileId, classId, info.getClassName(), data);
entryUSNs.put(profileId, newEntryUSN);
+ nsUniqueIds.put(profileId, nsUniqueId);
CMS.debug("Done Profile Creation - " + profileId);
} catch (EProfileException e) {
CMS.debug("Error creating profile '" + profileId + "'; skipping.");
@@ -192,7 +209,7 @@ public class LDAPProfileSubsystem
}
}
- public void deleteProfile(String id) throws EProfileException {
+ public synchronized void deleteProfile(String id) throws EProfileException {
if (isProfileEnable(id)) {
throw new EProfileException("CMS_PROFILE_DELETE_ENABLEPROFILE");
}
@@ -215,9 +232,31 @@ public class LDAPProfileSubsystem
}
}
+ deletedNsUniqueIds.add(nsUniqueIds.get(id));
forgetProfile(id);
}
+ private synchronized void handleDELETE(LDAPEntry entry) {
+ LDAPAttribute attr = entry.getAttribute("nsUniqueId");
+ String nsUniqueId = null;
+ if (attr != null)
+ nsUniqueId = attr.getStringValueArray()[0];
+
+ if (deletedNsUniqueIds.remove(nsUniqueId)) {
+ CMS.debug("handleDELETE: delete was already effected");
+ return;
+ }
+
+ String profileId = null;
+ String dn = entry.getDN();
+ if (!dn.startsWith("cn=")) {
+ CMS.debug("handleDELETE: DN " + dn + " does not start with 'cn='");
+ return;
+ }
+ profileId = LDAPDN.explodeDN(dn, true)[0];
+ forgetProfile(profileId);
+ }
+
private synchronized void handleMODDN(DN oldDN, LDAPEntry entry) {
DN profilesDN = new DN(dn);
@@ -231,20 +270,61 @@ public class LDAPProfileSubsystem
@Override
public synchronized void commitProfile(String id) throws EProfileException {
LDAPConfigStore cs = (LDAPConfigStore) mProfiles.get(id).getConfigStore();
+
+ // first create a *new* profile object from the configStore
+ // and initialise it with the updated configStore
+ //
+ IPluginRegistry registry = (IPluginRegistry)
+ CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
+ String classId = mProfileClassIds.get(id);
+ IPluginInfo info = registry.getPluginInfo("profile", classId);
+ String className = info.getClassName();
+ IProfile newProfile = null;
try {
- String[] attrs = {"entryUSN"};
+ newProfile = (IProfile) Class.forName(className).newInstance();
+ } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
+ throw new EProfileException("Could not instantiate class '"
+ + classId + "' for profile '" + id + "': " + e);
+ }
+ newProfile.setId(id);
+ try {
+ newProfile.init(this, cs);
+ } catch (EBaseException e) {
+ throw new EProfileException(
+ "Failed to initialise profile '" + id + "': " + e);
+ }
+
+ // next replace the existing profile with the new profile;
+ // this is to avoid any intermediate state where the profile
+ // is not fully initialised with its inputs, outputs and
+ // policy objects.
+ //
+ mProfiles.put(id, newProfile);
+
+ // finally commit the configStore and track the resulting
+ // entryUSN and (in case of add) the nsUniqueId
+ //
+ try {
+ String[] attrs = {"entryUSN", "nsUniqueId"};
LDAPEntry entry = cs.commitReturn(false, attrs);
-
- LDAPAttribute attr = null;
- if (entry != null)
- attr = entry.getAttribute("entryUSN");
+ if (entry == null) {
+ // shouldn't happen, but let's be sure not to crash anyway
+ return;
+ }
Integer entryUSN = null;
+ LDAPAttribute attr = entry.getAttribute("entryUSN");
if (attr != null)
entryUSN = new Integer(attr.getStringValueArray()[0]);
-
entryUSNs.put(id, entryUSN);
CMS.debug("commitProfile: new entryUSN = " + entryUSN);
+
+ String nsUniqueId = null;
+ attr = entry.getAttribute("nsUniqueId");
+ if (attr != null)
+ nsUniqueId = attr.getStringValueArray()[0];
+ CMS.debug("commitProfile: nsUniqueId = " + nsUniqueId);
+ nsUniqueIds.put(id, nsUniqueId);
} catch (ELdapException e) {
throw new EProfileException(
"Failed to commit config store of profile '" + id + ": " + e);
@@ -261,17 +341,7 @@ public class LDAPProfileSubsystem
mProfiles.remove(id);
mProfileClassIds.remove(id);
entryUSNs.remove(id);
- }
-
- private void forgetProfile(LDAPEntry entry) {
- String profileId = null;
- String dn = entry.getDN();
- if (!dn.startsWith("cn=")) {
- CMS.debug("forgetProfile: DN " + dn + " does not start with 'cn='");
- return;
- }
- profileId = LDAPDN.explodeDN(dn, true)[0];
- forgetProfile(profileId);
+ nsUniqueIds.remove(id);
}
/**
@@ -296,6 +366,8 @@ public class LDAPProfileSubsystem
mProfiles.clear();
mProfileClassIds.clear();
entryUSNs.clear();
+ nsUniqueIds.clear();
+ deletedNsUniqueIds.clear();
}
/**
@@ -328,7 +400,7 @@ public class LDAPProfileSubsystem
cons.setServerControls(persistCtrl);
cons.setBatchSize(1);
cons.setServerTimeLimit(0 /* seconds */);
- String[] attrs = {"*", "entryUSN"};
+ String[] attrs = {"*", "entryUSN", "nsUniqueId"};
LDAPSearchResults results = conn.search(
dn, LDAPConnection.SCOPE_ONE, "(objectclass=*)",
attrs, false, cons);
@@ -347,7 +419,7 @@ public class LDAPProfileSubsystem
break;
case LDAPPersistSearchControl.DELETE:
CMS.debug("Profile change monitor: DELETE");
- forgetProfile(entry);
+ handleDELETE(entry);
break;
case LDAPPersistSearchControl.MODIFY:
CMS.debug("Profile change monitor: MODIFY");
--
2.4.3
-------------- next part --------------
From 2b790945f75257d098ca965644d222975f395d4b Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Tue, 1 Dec 2015 14:21:08 +1100
Subject: [PATCH 61/61] Ensure config store commits refresh file-based profile
data
The file-based LDAP profile subsystem does not update profiles
correctly. Ensure that each commit of the underlying config store
refreshes the profile inputs, outputs and policy objects.
Part of: https://fedorahosted.org/pki/ticket/1700
---
.../cmscore/profile/AbstractProfileSubsystem.java | 39 +++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
index 076e9c93e461f00e7a6dcde92f9fdd06d23a0283..fc26131112d73c40930e5630049e71644007d934 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
@@ -22,12 +22,15 @@ import java.util.Enumeration;
import java.util.Hashtable;
import java.util.LinkedHashMap;
+import com.netscape.certsrv.apps.CMS;
import com.netscape.certsrv.base.EBaseException;
import com.netscape.certsrv.base.IConfigStore;
import com.netscape.certsrv.base.ISubsystem;
import com.netscape.certsrv.profile.EProfileException;
import com.netscape.certsrv.profile.IProfile;
import com.netscape.certsrv.profile.IProfileSubsystem;
+import com.netscape.certsrv.registry.IPluginInfo;
+import com.netscape.certsrv.registry.IPluginRegistry;
public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
protected static final String PROP_CHECK_OWNER = "checkOwner";
@@ -120,8 +123,42 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
*/
public void commitProfile(String id)
throws EProfileException {
+ IConfigStore cs = mProfiles.get(id).getConfigStore();
+
+ // first create a *new* profile object from the configStore
+ // and initialise it with the updated configStore
+ //
+ IPluginRegistry registry = (IPluginRegistry)
+ CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
+ String classId = mProfileClassIds.get(id);
+ IPluginInfo info = registry.getPluginInfo("profile", classId);
+ String className = info.getClassName();
+ IProfile newProfile = null;
try {
- mProfiles.get(id).getConfigStore().commit(false);
+ newProfile = (IProfile) Class.forName(className).newInstance();
+ } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
+ throw new EProfileException("Could not instantiate class '"
+ + classId + "' for profile '" + id + "': " + e);
+ }
+ newProfile.setId(id);
+ try {
+ newProfile.init(this, cs);
+ } catch (EBaseException e) {
+ throw new EProfileException(
+ "Failed to initialise profile '" + id + "': " + e);
+ }
+
+ // next replace the existing profile with the new profile;
+ // this is to avoid any intermediate state where the profile
+ // is not fully initialised with its inputs, outputs and
+ // policy objects.
+ //
+ mProfiles.put(id, newProfile);
+
+ // finally commit the configStore
+ //
+ try {
+ cs.commit(false);
} catch (EBaseException e) {
throw new EProfileException(
"Failed to commit config store of profile '" + id + ": " + e);
--
2.4.3
More information about the Pki-devel
mailing list