authRules = getEffectiveRules(apiMethod);
logger.info("Authenticated: "+AuthRule.AUTHENTICATED +"User: "+TeamUtil.getCurrentUser());
if (authRules.contains(AuthRule.AUTHENTICATED) && TeamUtil.getCurrentUser() == null) {
diff --git a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/AuthRule.java b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/AuthRule.java
index dc2f4d2a175..84fa281d81d 100644
--- a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/AuthRule.java
+++ b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/AuthRule.java
@@ -26,7 +26,13 @@ public enum AuthRule {
/** if amp offline user-agent is present in headers check for AMP_OFFLINE. If not, check for other actions */
AMP_OFFLINE_OPTIONAL,
/** Current rule: If activity was created in private ws, it can only be access from there **/
- PUBLIC_VIEW_ACTIVITY;
+ PUBLIC_VIEW_ACTIVITY,
+ /**
+ * Explicitly marks a method as intentionally open to anonymous callers.
+ * Must be used deliberately: a method with no authTypes at all is treated as
+ * {@link #AUTHENTICATED} by default (see {@link org.digijava.kernel.ampapi.endpoints.security.ActionAuthorizer}).
+ */
+ PUBLIC;
@Override
public String toString() {
diff --git a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/SecurityService.java b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/SecurityService.java
index 73762c6b17c..fbd3d6fcf3c 100644
--- a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/SecurityService.java
+++ b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/SecurityService.java
@@ -14,6 +14,8 @@
import org.digijava.kernel.ampapi.endpoints.security.dto.*;
import org.digijava.kernel.request.SiteDomain;
import org.digijava.kernel.request.TLSUtils;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
+import org.digijava.kernel.security.auth.DigiUserDetailsService;
import org.digijava.kernel.services.AmpVersionInfo;
import org.digijava.kernel.services.AmpVersionService;
import org.digijava.kernel.translator.TranslatorWorker;
@@ -34,12 +36,12 @@
import org.digijava.module.gateperm.util.PermissionUtil;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.web.authentication.WebAuthenticationDetails;
+import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
-import java.nio.charset.StandardCharsets;
-import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
@@ -205,12 +207,8 @@ public UserSessionInformation authenticate(AuthenticationRequest authRequest) {
ApiErrorResponseService.reportError(BAD_REQUEST, SecurityErrors.INVALID_USER_PASSWORD);
}
- User user = UserUtils.getUserByEmailAddress(username);
- String storedPassword = (user != null && user.getPassword() != null) ? user.getPassword() : "";
- boolean passwordMatches = MessageDigest.isEqual(
- storedPassword.getBytes(StandardCharsets.UTF_8),
- password.getBytes(StandardCharsets.UTF_8));
- if (user == null || !passwordMatches) {
+ User user = verifyCredentials(username, password);
+ if (user == null) {
ApiErrorResponseService.reportForbiddenAccess(SecurityErrors.INVALID_USER_PASSWORD);
}
@@ -232,6 +230,26 @@ public UserSessionInformation authenticate(AuthenticationRequest authRequest) {
return SecurityService.getInstance().createUserSessionInformation(isAdmin, user, ampTeamName, true);
}
+ /**
+ * Verifies the given credentials, opportunistically upgrading a legacy (non-bcrypt) stored
+ * password on success, so both the REST login and the login widget share one verification path.
+ *
+ * @return the matching user, or null if the credentials are invalid
+ */
+ public User verifyCredentials(String username, String password) {
+ User user = UserUtils.getUserByEmailAddress(username);
+ String storedPassword = (user != null) ? user.getPassword() : null;
+ AmpPasswordEncoder passwordEncoder = new AmpPasswordEncoder();
+ if (storedPassword == null || !passwordEncoder.matches(password, storedPassword)) {
+ return null;
+ }
+ if (!passwordEncoder.isHashed(storedPassword)) {
+ // opportunistically upgrade legacy plaintext password to a bcrypt hash on successful login
+ user.setPassword(passwordEncoder.encode(password));
+ }
+ return user;
+ }
+
public void invalidateExistingSession() {
HttpSession session = TLSUtils.getRequest().getSession(false);
if (session != null) {
@@ -247,14 +265,27 @@ private AmpTeamMember getAmpTeamMember(String username, Long workspaceId) {
return teamMember;
}
- private void storeInSession(String username, AmpTeamMember teamMember, User user) {
+ /**
+ * Establishes the AMP session (legacy attributes + Spring Security context) for a user that has
+ * already passed {@link #verifyCredentials(String, String)} and {@link ApiAuthentication#login}.
+ * Used by both the REST login and the login widget (see AmpPostLoginAction), so both share the
+ * same authenticated state, including on pages/apps (e.g. the activity form) gated by Spring's
+ * own ROLE_AUTHENTICATED access control rather than only the legacy session attributes.
+ */
+ public void storeInSession(String username, AmpTeamMember teamMember, User user) {
// Do not pass credentials to the token — the submitted hash must not be
// stored in the Spring Security context or serialised into the session.
+ UserDetails userDetails = SpringUtil.getBean(DigiUserDetailsService.class).loadUserByUsername(username);
final UsernamePasswordAuthenticationToken authToken =
- new UsernamePasswordAuthenticationToken(username, null);
+ new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities());
authToken.setDetails(new WebAuthenticationDetails(TLSUtils.getRequest()));
SecurityContextHolder.getContext().setAuthentication(authToken);
final HttpSession session = TLSUtils.getRequest().getSession();
+ // /rest/** is create-session="stateless" so SecurityContextPersistenceFilter never persists this;
+ // save it explicitly under the same key so non-REST pages (Struts .do actions, the Wicket activity
+ // form) restore it via the normal HttpSessionSecurityContextRepository on their own request chain.
+ session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY,
+ SecurityContextHolder.getContext());
PermissionUtil.putInScope(session, GatePermConst.ScopeKeys.CURRENT_MEMBER, teamMember);
if (teamMember != null) {
session.setAttribute(Constants.CURRENT_MEMBER, teamMember.toTeamMember());
diff --git a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/services/UserManagerService.java b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/services/UserManagerService.java
index f36cc0f31db..57cec192bee 100644
--- a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/services/UserManagerService.java
+++ b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/security/services/UserManagerService.java
@@ -32,6 +32,7 @@
import org.digijava.kernel.request.SiteDomain;
import org.digijava.kernel.request.TLSUtils;
import org.digijava.kernel.security.PasswordPolicyValidator;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.services.AmpVersionInfo;
import org.digijava.kernel.services.AmpVersionService;
import org.digijava.kernel.translator.TranslatorWorker;
@@ -114,8 +115,10 @@ public LoggedUserInformation createUser(CreateUserRequest createUser) {
user.setFirstNames(firstName);
user.setLastName(lastName);
user.setEmail(email);
- user.setPassword(password);
- user.setSalt(password);
+ // AMP-SEC-017/054: never store the plaintext password
+ String hashedPassword = new AmpPasswordEncoder().encode(ShaCrypt.crypt(password.trim()).trim());
+ user.setPassword(hashedPassword);
+ user.setSalt(hashedPassword);
user.setNotificationEmailEnabled(notificationEmailEnabled);
if(notificationEmailEnabled){
user.setNotificationEmail(notificationEmail);
diff --git a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/util/ApiMethod.java b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/util/ApiMethod.java
index 386b2258c95..5c9f7e03549 100644
--- a/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/util/ApiMethod.java
+++ b/amp/src/main/java/org/digijava/kernel/ampapi/endpoints/util/ApiMethod.java
@@ -64,7 +64,11 @@
String visibilityCheck() default "";
/**
- * Authorization rules that must be applied to this method. Default is no authorization to be done.
+ * Authorization rules that must be applied to this method.
+ *
+ * Fail-closed default: when left empty, the method still requires an authenticated session
+ * (equivalent to {@link AuthRule#AUTHENTICATED}). Use {@link AuthRule#PUBLIC} explicitly to allow
+ * anonymous access.
*/
AuthRule[] authTypes() default {};
}
diff --git a/amp/src/main/java/org/digijava/kernel/ampapi/swagger/SwaggerAuthorization.java b/amp/src/main/java/org/digijava/kernel/ampapi/swagger/SwaggerAuthorization.java
index 4e41bcd5a3b..a2a26e4c732 100644
--- a/amp/src/main/java/org/digijava/kernel/ampapi/swagger/SwaggerAuthorization.java
+++ b/amp/src/main/java/org/digijava/kernel/ampapi/swagger/SwaggerAuthorization.java
@@ -20,7 +20,8 @@ public class SwaggerAuthorization extends AbstractSwaggerExtension {
private static final Set IGNORE_RULES = new TreeSet<>(Arrays.asList(
AuthRule.AMP_OFFLINE,
AuthRule.AMP_OFFLINE_OPTIONAL,
- AuthRule.PUBLIC_VIEW_ACTIVITY));
+ AuthRule.PUBLIC_VIEW_ACTIVITY,
+ AuthRule.PUBLIC));
@Override
public void decorateOperation(Operation operation, Method method, Iterator chain) {
diff --git a/amp/src/main/java/org/digijava/kernel/security/auth/AmpPasswordEncoder.java b/amp/src/main/java/org/digijava/kernel/security/auth/AmpPasswordEncoder.java
new file mode 100644
index 00000000000..f14e0d5078f
--- /dev/null
+++ b/amp/src/main/java/org/digijava/kernel/security/auth/AmpPasswordEncoder.java
@@ -0,0 +1,46 @@
+package org.digijava.kernel.security.auth;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.util.regex.Pattern;
+
+import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
+import org.springframework.security.crypto.password.PasswordEncoder;
+
+/**
+ * Hashes new/changed passwords with bcrypt (replaces {@code NoOpPasswordEncoder}),
+ * while still being able to verify accounts whose password was stored in plaintext before this
+ * encoder was introduced, so existing users are not locked out.
+ */
+public class AmpPasswordEncoder implements PasswordEncoder {
+
+ private static final Pattern BCRYPT_PATTERN = Pattern.compile("^\\$2[aby]\\$\\d{2}\\$.{53}$");
+
+ private final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+
+ @Override
+ public String encode(CharSequence rawPassword) {
+ return encoder.encode(rawPassword);
+ }
+
+ @Override
+ public boolean matches(CharSequence rawPassword, String encodedPassword) {
+ if (rawPassword == null || encodedPassword == null) {
+ return false;
+ }
+ if (isHashed(encodedPassword)) {
+ return encoder.matches(rawPassword, encodedPassword);
+ }
+ // legacy account: password column still holds the plaintext value, compare directly
+ return MessageDigest.isEqual(
+ encodedPassword.getBytes(StandardCharsets.UTF_8),
+ rawPassword.toString().getBytes(StandardCharsets.UTF_8));
+ }
+
+ /**
+ * @return true if the stored value is already a bcrypt hash produced by this encoder
+ */
+ public boolean isHashed(String storedPassword) {
+ return storedPassword != null && BCRYPT_PATTERN.matcher(storedPassword).matches();
+ }
+}
diff --git a/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss1Impl.java b/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss1Impl.java
index 46deabee28c..ec68930dc28 100644
--- a/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss1Impl.java
+++ b/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss1Impl.java
@@ -27,6 +27,7 @@
import org.digijava.kernel.syndication.digester.Rss;
import org.digijava.kernel.syndication.digester.RssChannel;
import org.digijava.kernel.syndication.digester.RssItem;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.xml.sax.SAXException;
import java.io.IOException;
@@ -66,6 +67,8 @@ public Rss parseXml(InputStream stream) throws SAXException, IOException{
digester.clear();
digester.setValidating(false);
digester.setUseContextClassLoader(true);
+ // AMP-SEC-025/061: remote RSS feeds are untrusted input, block XXE
+ XmlSecurityUtils.secureDigester(digester);
digester.addObjectCreate("rdf:RDF", Rss.class);
digester.addObjectCreate("rdf:RDF/channel", RssChannel.class);
diff --git a/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss2Impl.java b/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss2Impl.java
index 5175250f56c..efb253c31e8 100644
--- a/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss2Impl.java
+++ b/amp/src/main/java/org/digijava/kernel/syndication/aggregator/Rss2Impl.java
@@ -27,6 +27,7 @@
import org.digijava.kernel.syndication.digester.Rss;
import org.digijava.kernel.syndication.digester.RssChannel;
import org.digijava.kernel.syndication.digester.RssItem;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.xml.sax.SAXException;
import java.io.IOException;
@@ -98,6 +99,8 @@ public Digester createDigester() {
digester.clear();
digester.setValidating(false);
digester.setUseContextClassLoader(true);
+ // AMP-SEC-025/061: remote RSS feeds are untrusted input, block XXE
+ XmlSecurityUtils.secureDigester(digester);
digester.addObjectCreate("rss", Rss.class);
digester.addObjectCreate("rss/channel", RssChannel.class);
diff --git a/amp/src/main/java/org/digijava/kernel/text/parser/LocaleParser.java b/amp/src/main/java/org/digijava/kernel/text/parser/LocaleParser.java
index 65397320e7b..d77a5a976ed 100644
--- a/amp/src/main/java/org/digijava/kernel/text/parser/LocaleParser.java
+++ b/amp/src/main/java/org/digijava/kernel/text/parser/LocaleParser.java
@@ -24,6 +24,7 @@
import org.apache.commons.digester.Digester;
import org.apache.log4j.Logger;
+import org.digijava.kernel.util.XmlSecurityUtils;
import java.io.InputStream;
import java.lang.ref.SoftReference;
@@ -63,6 +64,8 @@ private static Digester createDigester() {
digester.setValidating(true);
// Workaround Tomcat's issue with ClassLoader
digester.setUseContextClassLoader(true);
+ // AMP-SEC-025/061: block XXE; DOCTYPE stays allowed since validation needs the registered local DTD
+ XmlSecurityUtils.secureDigester(digester, true);
// Configure digester
digester.addObjectCreate("locale", LocaleData.class);
diff --git a/amp/src/main/java/org/digijava/kernel/util/DigesterFactory.java b/amp/src/main/java/org/digijava/kernel/util/DigesterFactory.java
index f6f523795dc..bcf606e2b26 100644
--- a/amp/src/main/java/org/digijava/kernel/util/DigesterFactory.java
+++ b/amp/src/main/java/org/digijava/kernel/util/DigesterFactory.java
@@ -127,6 +127,9 @@ public static Digester newDigester(boolean xmlValidation,
if (rule != null) {
digester.addRuleSet(rule);
}
+ // AMP-SEC-025/061: block XXE, but this factory is only used for trusted, locally-deployed
+ // config files (digi.xml relies on a DOCTYPE-declared general entity to include digi-common.xml)
+ XmlSecurityUtils.secureDigester(digester, true);
return (digester);
}
diff --git a/amp/src/main/java/org/digijava/kernel/util/DigiSchemaPopulate.java b/amp/src/main/java/org/digijava/kernel/util/DigiSchemaPopulate.java
index 0033e359ee7..ddcc4c3bee0 100644
--- a/amp/src/main/java/org/digijava/kernel/util/DigiSchemaPopulate.java
+++ b/amp/src/main/java/org/digijava/kernel/util/DigiSchemaPopulate.java
@@ -294,7 +294,8 @@ static void createGlobalAdmin() throws Exception {
user.setFirstNames("System");
user.setLastName("System");
user.setEmail("system@digijava.org");
- user.setPassword("changeme");
+ // AMP-SEC-017/054: never store the plaintext password
+ user.setPassword(new org.digijava.kernel.security.auth.AmpPasswordEncoder().encode(ShaCrypt.crypt("changeme").trim()));
user.setRegisterLanguage(english);
user.setBanned(false);
user.setOrganizationTypeOther(" ");
diff --git a/amp/src/main/java/org/digijava/kernel/util/UserUtils.java b/amp/src/main/java/org/digijava/kernel/util/UserUtils.java
index 088b6c0581a..5d09d48a010 100644
--- a/amp/src/main/java/org/digijava/kernel/util/UserUtils.java
+++ b/amp/src/main/java/org/digijava/kernel/util/UserUtils.java
@@ -33,6 +33,7 @@
import org.digijava.kernel.request.SiteDomain;
import org.digijava.kernel.security.DgSecurityManager;
import org.digijava.kernel.security.ResourcePermission;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.security.principal.GroupPrincipal;
import org.digijava.kernel.security.principal.UserPrincipal;
import org.digijava.kernel.user.Group;
@@ -512,7 +513,9 @@ public static User getUserByEmailAddress(String email) {
* @param password String new password
*/
public static void setPassword(User user, String password) {
- user.setPassword(ShaCrypt.crypt(password.trim()).trim());
+ // bcrypt-wrap the SHA1(password) value, matching the domain the login widget submits
+ // (see AmpPasswordEncoder / SecurityService.verifyCredentials)
+ user.setPassword(new AmpPasswordEncoder().encode(ShaCrypt.crypt(password.trim()).trim()));
user.setSalt(new Long(password.trim().hashCode()).toString());
user.setPasswordChangedAt(new Date());
}
diff --git a/amp/src/main/java/org/digijava/kernel/util/XmlSecurityUtils.java b/amp/src/main/java/org/digijava/kernel/util/XmlSecurityUtils.java
new file mode 100644
index 00000000000..eaaaf9e1a1d
--- /dev/null
+++ b/amp/src/main/java/org/digijava/kernel/util/XmlSecurityUtils.java
@@ -0,0 +1,100 @@
+package org.digijava.kernel.util;
+
+import org.apache.commons.digester.Digester;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+import javax.xml.bind.Unmarshaller;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.Source;
+import javax.xml.transform.sax.SAXSource;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.Reader;
+
+/**
+ * Helpers to lock down JAXB and Commons Digester XML parsing against XML External Entity (XXE) attacks
+ * (AMP-SEC-025/026/061/062): disallow DOCTYPE declarations and external entity/DTD resolution.
+ */
+public final class XmlSecurityUtils {
+
+ private static final String FEATURE_DISALLOW_DOCTYPE = "http://apache.org/xml/features/disallow-doctype-decl";
+ private static final String FEATURE_EXTERNAL_GENERAL_ENTITIES = "http://xml.org/sax/features/external-general-entities";
+ private static final String FEATURE_EXTERNAL_PARAMETER_ENTITIES = "http://xml.org/sax/features/external-parameter-entities";
+ private static final String FEATURE_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
+
+ private XmlSecurityUtils() {
+ }
+
+ /** A {@link SAXParserFactory} hardened against XXE, with DOCTYPE declarations disallowed entirely. */
+ public static SAXParserFactory secureSaxParserFactory() {
+ SAXParserFactory factory = SAXParserFactory.newInstance();
+ try {
+ factory.setFeature(FEATURE_DISALLOW_DOCTYPE, true);
+ factory.setFeature(FEATURE_EXTERNAL_GENERAL_ENTITIES, false);
+ factory.setFeature(FEATURE_EXTERNAL_PARAMETER_ENTITIES, false);
+ factory.setXIncludeAware(false);
+ } catch (ParserConfigurationException | SAXException e) {
+ throw new IllegalStateException("Unable to configure a secure SAXParserFactory", e);
+ }
+ return factory;
+ }
+
+ private static XMLReader secureXmlReader() {
+ try {
+ return secureSaxParserFactory().newSAXParser().getXMLReader();
+ } catch (ParserConfigurationException | SAXException e) {
+ throw new IllegalStateException("Unable to create a secure XMLReader", e);
+ }
+ }
+
+ /** Wraps a stream so it can be safely passed to {@link Unmarshaller#unmarshal(Source)}. */
+ public static Source secureSource(InputStream inputStream) {
+ return new SAXSource(secureXmlReader(), new InputSource(inputStream));
+ }
+
+ /** Wraps a reader so it can be safely passed to {@link Unmarshaller#unmarshal(Source)}. */
+ public static Source secureSource(Reader reader) {
+ return new SAXSource(secureXmlReader(), new InputSource(reader));
+ }
+
+ /** Wraps a file so it can be safely passed to {@link Unmarshaller#unmarshal(Source)}. */
+ public static Source secureSource(File file) throws FileNotFoundException {
+ InputSource inputSource = new InputSource(new FileInputStream(file));
+ inputSource.setSystemId(file.toURI().toString());
+ return new SAXSource(secureXmlReader(), inputSource);
+ }
+
+ /** Wraps an existing {@link InputSource} so it can be safely passed to {@link Unmarshaller#unmarshal(Source)}. */
+ public static Source secureSource(InputSource inputSource) {
+ return new SAXSource(secureXmlReader(), inputSource);
+ }
+
+ /**
+ * Hardens a Commons Digester instance against XXE. DOCTYPE declarations (and the general
+ * entities they may declare, e.g. this codebase's own digi.xml file-inclusion trick) are
+ * disallowed entirely unless {@code allowDoctypeDecl} is true, which is only needed for
+ * digesters that parse trusted, locally-controlled config files relying on DOCTYPE features.
+ */
+ public static void secureDigester(Digester digester, boolean allowDoctypeDecl) {
+ try {
+ if (!allowDoctypeDecl) {
+ digester.setFeature(FEATURE_DISALLOW_DOCTYPE, true);
+ digester.setFeature(FEATURE_EXTERNAL_GENERAL_ENTITIES, false);
+ }
+ digester.setFeature(FEATURE_EXTERNAL_PARAMETER_ENTITIES, false);
+ digester.setFeature(FEATURE_LOAD_EXTERNAL_DTD, false);
+ } catch (ParserConfigurationException | SAXException e) {
+ throw new IllegalStateException("Unable to configure a secure Digester", e);
+ }
+ }
+
+ /** Hardens a Commons Digester instance against XXE, disallowing DOCTYPE declarations entirely. */
+ public static void secureDigester(Digester digester) {
+ secureDigester(digester, false);
+ }
+}
diff --git a/amp/src/main/java/org/digijava/kernel/viewmanager/RepositoryParser.java b/amp/src/main/java/org/digijava/kernel/viewmanager/RepositoryParser.java
index 5cd19e79fab..de9020e5e8e 100644
--- a/amp/src/main/java/org/digijava/kernel/viewmanager/RepositoryParser.java
+++ b/amp/src/main/java/org/digijava/kernel/viewmanager/RepositoryParser.java
@@ -25,6 +25,7 @@
import org.apache.commons.digester.Digester;
import org.apache.log4j.Logger;
import org.digijava.kernel.siteconfig.*;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.xml.sax.SAXException;
import java.io.File;
@@ -46,6 +47,8 @@ private static Digester createDigester() {
digester.setValidating(false);
// Workaround Tomcat's issue with ClassLoader
digester.setUseContextClassLoader(true);
+ // AMP-SEC-025/061: block XXE
+ XmlSecurityUtils.secureDigester(digester);
// Configure digester
digester.addObjectCreate("config", RepositoryLayout.class);
diff --git a/amp/src/main/java/org/digijava/kernel/viewmanager/SiteConfigParser.java b/amp/src/main/java/org/digijava/kernel/viewmanager/SiteConfigParser.java
index bae53554a88..591f01f9324 100644
--- a/amp/src/main/java/org/digijava/kernel/viewmanager/SiteConfigParser.java
+++ b/amp/src/main/java/org/digijava/kernel/viewmanager/SiteConfigParser.java
@@ -25,6 +25,7 @@
import org.apache.commons.digester.Digester;
import org.apache.log4j.Logger;
import org.digijava.kernel.siteconfig.*;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.xml.sax.SAXException;
import java.io.File;
@@ -46,6 +47,9 @@ private static Digester createDigester() {
digester.setValidating(false);
// Workaround Tomcat's issue with ClassLoader
digester.setUseContextClassLoader(true);
+ // AMP-SEC-025/061: block XXE, but allow DOCTYPE - ampTemplate/site-config.xml declares
+ // internal-only general entities (&Version; etc.) via a DOCTYPE internal subset
+ XmlSecurityUtils.secureDigester(digester, true);
// Configure digester
digester.addObjectCreate("site-config", SiteConfig.class);
diff --git a/amp/src/main/java/org/digijava/module/aim/action/RegisterUser.java b/amp/src/main/java/org/digijava/module/aim/action/RegisterUser.java
index a5d56cbc456..f519a210445 100644
--- a/amp/src/main/java/org/digijava/module/aim/action/RegisterUser.java
+++ b/amp/src/main/java/org/digijava/module/aim/action/RegisterUser.java
@@ -14,6 +14,7 @@
import org.digijava.kernel.request.Site;
import org.digijava.kernel.request.SiteDomain;
import org.digijava.kernel.security.PasswordPolicyValidator;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.user.Group;
import org.digijava.kernel.user.User;
import org.digijava.kernel.util.DgUtil;
@@ -55,9 +56,10 @@ public ActionForward execute(ActionMapping mapping, ActionForm form,
request.setAttribute(PasswordPolicyValidator.SHOW_PASSWORD_POLICY_RULES, true);
return (mapping.getInputForward());
}
- // set password
- user.setPassword(userRegisterForm.getPassword().trim());
- user.setSalt(userRegisterForm.getPassword().trim());
+ // set password (AMP-SEC-017/054: never store the plaintext password)
+ String hashedPassword = new AmpPasswordEncoder().encode(ShaCrypt.crypt(userRegisterForm.getPassword().trim()).trim());
+ user.setPassword(hashedPassword);
+ user.setSalt(hashedPassword);
// set Website
user.setUrl(userRegisterForm.getWebSite());
diff --git a/amp/src/main/java/org/digijava/module/aim/action/TranslatorManager.java b/amp/src/main/java/org/digijava/module/aim/action/TranslatorManager.java
index 5e822b83687..67a86c1b20e 100644
--- a/amp/src/main/java/org/digijava/module/aim/action/TranslatorManager.java
+++ b/amp/src/main/java/org/digijava/module/aim/action/TranslatorManager.java
@@ -12,6 +12,7 @@
import org.digijava.kernel.translator.CachedTranslatorWorker;
import org.digijava.kernel.translator.TranslatorWorker;
import org.digijava.kernel.util.RequestUtils;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.digijava.module.aim.form.TranslatorManagerForm;
import org.digijava.module.aim.helper.TrnHashMap;
import org.digijava.module.translation.entity.MessageGroup;
@@ -67,7 +68,7 @@ public ActionForward execute(ActionMapping mapping, ActionForm form,HttpServletR
List languagesImport = new ArrayList();
try {
- trns_in = (Translations) m.unmarshal(inputStream);
+ trns_in = (Translations) m.unmarshal(XmlSecurityUtils.secureSource(inputStream));
if (trns_in.getTrn() != null) {
Iterator it = trns_in.getTrn().iterator();
while (it.hasNext()) {
@@ -121,7 +122,7 @@ public ActionForward execute(ActionMapping mapping, ActionForm form,HttpServletR
trnHashMaps.add(tHashMap);
}
try {
- trns_in = (Translations) m.unmarshal(inputStream);
+ trns_in = (Translations) m.unmarshal(XmlSecurityUtils.secureSource(inputStream));
if (trns_in.getTrn() != null) {
logger.info("Processing "+trns_in.getTrn().size()+" translation groups (trn tags)...");
// Iterator it = trns_in.getTrn().iterator();
diff --git a/amp/src/main/java/org/digijava/module/aim/action/VisibilityManager.java b/amp/src/main/java/org/digijava/module/aim/action/VisibilityManager.java
index 3ca82e69c5c..b545da84d75 100644
--- a/amp/src/main/java/org/digijava/module/aim/action/VisibilityManager.java
+++ b/amp/src/main/java/org/digijava/module/aim/action/VisibilityManager.java
@@ -18,6 +18,7 @@
import org.digijava.module.aim.helper.VisibilityManagerExportHelper;
import org.digijava.module.aim.util.DbUtil;
import org.digijava.module.aim.util.FeaturesUtil;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.hibernate.HibernateException;
import org.hibernate.Session;
@@ -95,7 +96,7 @@ public ActionForward modeImportTreeVisibility(ActionMapping mapping,ActionForm f
JAXBContext jc = JAXBContext.newInstance("org.dgfoundation.amp.visibility.feed.fm.schema");
Unmarshaller um = jc.createUnmarshaller();
try {
- VisibilityTemplates vtemplate = (VisibilityTemplates) um.unmarshal(vForm.getUploadFile().getInputStream());
+ VisibilityTemplates vtemplate = (VisibilityTemplates) um.unmarshal(XmlSecurityUtils.secureSource(vForm.getUploadFile().getInputStream()));
VisibilityManagerExportHelper vhelper = new VisibilityManagerExportHelper();
vhelper.importXmlVisbilityTemplate(vtemplate);
} catch (JAXBException je) {
diff --git a/amp/src/main/java/org/digijava/module/aim/auth/AmpPostLoginAction.java b/amp/src/main/java/org/digijava/module/aim/auth/AmpPostLoginAction.java
index 9ea5aa6b27b..5412126ad95 100644
--- a/amp/src/main/java/org/digijava/module/aim/auth/AmpPostLoginAction.java
+++ b/amp/src/main/java/org/digijava/module/aim/auth/AmpPostLoginAction.java
@@ -10,12 +10,10 @@
import org.digijava.kernel.ampapi.endpoints.errors.ApiErrorMessage;
import org.digijava.kernel.ampapi.endpoints.security.ApiAuthentication;
import org.digijava.kernel.ampapi.endpoints.security.SecurityErrors;
-import org.digijava.kernel.exception.DgException;
+import org.digijava.kernel.ampapi.endpoints.security.SecurityService;
import org.digijava.kernel.user.User;
-import org.digijava.kernel.util.UserUtils;
-import org.springframework.security.core.Authentication;
-import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.core.userdetails.UserDetails;
+import org.digijava.module.aim.helper.Constants;
+import org.digijava.module.aim.util.AuditLoggerUtil;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -38,19 +36,26 @@ public ActionForward execute(ActionMapping mapping, ActionForm form,
String id = request.getParameter("j_autoWorkspaceId");
request.getSession().setAttribute("j_autoWorkspaceId", id);
-
- Authentication authResult = SecurityContextHolder.getContext().getAuthentication();
- User currentUser = null;
- try {
- currentUser = getUser(authResult);
- } catch(DgException ex) {
- throw new RuntimeException(ex);
+
+ String username = request.getParameter("j_username");
+ String password = request.getParameter("j_password");
+
+ SecurityService securityService = SecurityService.getInstance();
+ User currentUser = securityService.verifyCredentials(username, password);
+ if (currentUser == null) {
+ out.println(getJsonResponse(toLoginWidgetErrorCode(SecurityErrors.INVALID_USER_PASSWORD)));
+ return null;
}
- ApiErrorMessage res = ApiAuthentication.login(currentUser, request);
- if(res != null) {
+ ApiErrorMessage res = ApiAuthentication.performSecurityChecks(currentUser, request);
+ if (res != null) {
out.println(getJsonResponse(toLoginWidgetErrorCode(res)));
} else {
+ securityService.invalidateExistingSession();
+ securityService.storeInSession(username, null, currentUser);
+ // re-apply: invalidateExistingSession() discarded the session it was written to above
+ request.getSession().setAttribute("j_autoWorkspaceId", id);
+ AuditLoggerUtil.logUserLogin(request, currentUser, Constants.LOGIN_ACTION);
out.println(getJsonResponse("noError", null));
}
@@ -99,32 +104,4 @@ private String getJsonResponse(String originalMessage, String newMessage) {
json+="}";
return json;
}
-
- protected User getUser(Authentication currentAuth) throws DgException {
- if(currentAuth == null) {
- return null;
- }
-
- if(currentAuth.getPrincipal() == null) {
- return null;
- }
-
- User user;
- Object principal = currentAuth.getPrincipal();
- if(principal instanceof Long) {
- Long userId = (Long) principal;
- user = UserUtils.getUser(userId);
- } else {
- String userName;
- if(principal instanceof UserDetails) {
- UserDetails userDetails = (UserDetails) principal;
- userName = userDetails.getUsername();
- } else {
- userName = principal.toString();
- }
- user = UserUtils.getUserByEmailAddress(userName);
- }
-
- return user;
- }
}
diff --git a/amp/src/main/java/org/digijava/module/aim/services/publicview/conf/ConfigurationUtil.java b/amp/src/main/java/org/digijava/module/aim/services/publicview/conf/ConfigurationUtil.java
index 46c73ac25ff..8822d633e9a 100644
--- a/amp/src/main/java/org/digijava/module/aim/services/publicview/conf/ConfigurationUtil.java
+++ b/amp/src/main/java/org/digijava/module/aim/services/publicview/conf/ConfigurationUtil.java
@@ -1,6 +1,7 @@
package org.digijava.module.aim.services.publicview.conf;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.xml.sax.InputSource;
import javax.servlet.ServletContext;
@@ -28,7 +29,7 @@ public static Configuration getConfiguration (ServletContext ctx) throws JAXBExc
public static Configuration initConfig (InputSource inputSource) throws JAXBException, FileNotFoundException {
JAXBContext jc = JAXBContext.newInstance(Configuration.class);
Unmarshaller um = jc.createUnmarshaller();
- Configuration retVal = (Configuration)um.unmarshal(inputSource);
+ Configuration retVal = (Configuration)um.unmarshal(XmlSecurityUtils.secureSource(inputSource));
return retVal;
}
diff --git a/amp/src/main/java/org/digijava/module/aim/startup/AmpBackgroundActivitiesUtil.java b/amp/src/main/java/org/digijava/module/aim/startup/AmpBackgroundActivitiesUtil.java
index 480f9669065..3a3e71eaf5a 100644
--- a/amp/src/main/java/org/digijava/module/aim/startup/AmpBackgroundActivitiesUtil.java
+++ b/amp/src/main/java/org/digijava/module/aim/startup/AmpBackgroundActivitiesUtil.java
@@ -7,6 +7,7 @@
import org.digijava.kernel.request.Site;
import org.digijava.kernel.user.User;
import org.digijava.kernel.util.DgUtil;
+import org.digijava.kernel.util.ShaCrypt;
import org.digijava.kernel.util.SiteUtils;
import org.digijava.kernel.util.UserUtils;
import org.digijava.module.aim.dbentity.*;
@@ -93,9 +94,10 @@ protected static void createAmpValidatorUser(String userEmail, String firstNames
// set client IP address
user.setModifyingIP("0.0.0.0");
- // set password
- user.setPassword(AMP_USER_PASSWORD);
- user.setSalt(AMP_USER_PASSWORD);
+ // set password (AMP-SEC-017/054: never store the plaintext password)
+ String hashedPassword = new org.digijava.kernel.security.auth.AmpPasswordEncoder().encode(ShaCrypt.crypt(AMP_USER_PASSWORD).trim());
+ user.setPassword(hashedPassword);
+ user.setSalt(hashedPassword);
// set Website
user.setUrl("/");
diff --git a/amp/src/main/java/org/digijava/module/autopatcher/core/PatcherUtil.java b/amp/src/main/java/org/digijava/module/autopatcher/core/PatcherUtil.java
index 04a7e0d1386..fb576f4cde3 100644
--- a/amp/src/main/java/org/digijava/module/autopatcher/core/PatcherUtil.java
+++ b/amp/src/main/java/org/digijava/module/autopatcher/core/PatcherUtil.java
@@ -3,6 +3,7 @@
import org.apache.log4j.Logger;
import org.digijava.module.autopatcher.exceptions.InvalidPatchRepositoryException;
import org.digijava.module.autopatcher.schema.Patch;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.hibernate.HibernateException;
import org.hibernate.Session;
import org.hibernate.query.Query;
@@ -84,13 +85,13 @@ public static Collection getAllPatchesFiles(String abstractPatchesLocation
}
public static Patch getUnmarshalledPatch(File patchFile)
- throws JAXBException {
+ throws JAXBException, IOException {
JAXBContext jc = JAXBContext
.newInstance("org.digijava.module.autopatcher.schema");
Unmarshaller m = jc.createUnmarshaller();
m.setValidating(true);
- Patch p = (Patch) m.unmarshal(patchFile);
+ Patch p = (Patch) m.unmarshal(XmlSecurityUtils.secureSource(patchFile));
return p;
}
diff --git a/amp/src/main/java/org/digijava/module/contentrepository/action/DownloadFile.java b/amp/src/main/java/org/digijava/module/contentrepository/action/DownloadFile.java
index 227bcb7a4ee..87f482b5eb9 100644
--- a/amp/src/main/java/org/digijava/module/contentrepository/action/DownloadFile.java
+++ b/amp/src/main/java/org/digijava/module/contentrepository/action/DownloadFile.java
@@ -8,6 +8,7 @@
import org.digijava.kernel.ampapi.endpoints.resource.ResourceErrors;
import org.digijava.kernel.util.ResponseUtil;
import org.digijava.module.aim.helper.Constants;
+import org.digijava.module.aim.util.TeamUtil;
import org.digijava.module.contentrepository.helper.CrConstants;
import org.digijava.module.contentrepository.helper.DocumentData;
import org.digijava.module.contentrepository.helper.NodeWrapper;
@@ -30,6 +31,12 @@ public ActionForward execute(ActionMapping mapping, ActionForm form,
javax.servlet.http.HttpServletResponse response)
throws java.lang.Exception {
+ // moduleConfig/contentrepository/module-spring.xml), so it must enforce its own session check here.
+ if (TeamUtil.getCurrentUser() == null) {
+ response.sendError(javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED);
+ return null;
+ }
+
String nodeUUID = request.getParameter("uuid");
if (nodeUUID != null) {
diff --git a/amp/src/main/java/org/digijava/module/gateperm/action/ExchangePermission.java b/amp/src/main/java/org/digijava/module/gateperm/action/ExchangePermission.java
index 437d4141170..c05a6fe7da6 100644
--- a/amp/src/main/java/org/digijava/module/gateperm/action/ExchangePermission.java
+++ b/amp/src/main/java/org/digijava/module/gateperm/action/ExchangePermission.java
@@ -11,6 +11,7 @@
import org.dgfoundation.amp.utils.MultiAction;
import org.digijava.kernel.exception.DgException;
import org.digijava.kernel.persistence.PersistenceManager;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.digijava.module.aim.util.Identifiable;
import org.digijava.module.gateperm.core.*;
import org.digijava.module.gateperm.feed.schema.*;
@@ -93,7 +94,7 @@ private ActionForward modeImportPerform(ActionMapping mapping,
JAXBContext jc = JAXBContext.newInstance("org.digijava.module.gateperm.feed.schema");
Unmarshaller m = jc.createUnmarshaller();
- Permissions xmlPermissions = (Permissions) m.unmarshal(inputStream);
+ Permissions xmlPermissions = (Permissions) m.unmarshal(XmlSecurityUtils.secureSource(inputStream));
List gatePerm = xmlPermissions.getGatePerm();
Iterator i=gatePerm.iterator();
Session session=PersistenceManager.getRequestDBSession();
diff --git a/amp/src/main/java/org/digijava/module/help/action/HelpActions.java b/amp/src/main/java/org/digijava/module/help/action/HelpActions.java
index eb05bd6d68f..0955927391f 100644
--- a/amp/src/main/java/org/digijava/module/help/action/HelpActions.java
+++ b/amp/src/main/java/org/digijava/module/help/action/HelpActions.java
@@ -38,6 +38,7 @@
import javax.xml.bind.JAXBContext;
import javax.xml.bind.Marshaller;
import javax.xml.bind.Unmarshaller;
+import org.digijava.kernel.util.XmlSecurityUtils;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.*;
@@ -952,7 +953,7 @@ public ActionForward importing(ActionMapping mapping,ActionForm form, HttpServle
if(xmlContent == null) return mapping.findForward("admin");
JAXBContext jc = JAXBContext.newInstance("org.digijava.module.help.jaxbi");
Unmarshaller m = jc.createUnmarshaller();
- help_in = (AmpHelpRoot) m.unmarshal(new ByteArrayInputStream(xmlContent));
+ help_in = (AmpHelpRoot) m.unmarshal(XmlSecurityUtils.secureSource(new ByteArrayInputStream(xmlContent)));
//remove all existing help topics
List firstLevelTopics=HelpUtil.getFirstLevelTopics(site);
diff --git a/amp/src/main/java/org/digijava/module/message/action/ExportAndImportTemplates.java b/amp/src/main/java/org/digijava/module/message/action/ExportAndImportTemplates.java
index f72a086ed80..4632c0a6bc3 100644
--- a/amp/src/main/java/org/digijava/module/message/action/ExportAndImportTemplates.java
+++ b/amp/src/main/java/org/digijava/module/message/action/ExportAndImportTemplates.java
@@ -16,6 +16,7 @@
import javax.servlet.http.HttpServletResponse;
import javax.xml.bind.JAXBContext;
import javax.xml.bind.Unmarshaller;
+import org.digijava.kernel.util.XmlSecurityUtils;
import java.io.*;
import java.util.List;
@@ -79,7 +80,7 @@ public ActionForward importTemplates (ActionMapping mapping,ActionForm form, Htt
Unmarshaller m = jc.createUnmarshaller();
Messaging item;
try {
- item = (Messaging) m.unmarshal(inputStream);
+ item = (Messaging) m.unmarshal(XmlSecurityUtils.secureSource(inputStream));
TemplatesList tempList=item.getTemplatesList();
if(tempList!=null){
List templates=tempList.getTemplate();
diff --git a/amp/src/main/java/org/digijava/module/translation/action/ImportExportTranslations.java b/amp/src/main/java/org/digijava/module/translation/action/ImportExportTranslations.java
index a535130d77c..936e2674441 100644
--- a/amp/src/main/java/org/digijava/module/translation/action/ImportExportTranslations.java
+++ b/amp/src/main/java/org/digijava/module/translation/action/ImportExportTranslations.java
@@ -26,6 +26,7 @@
import org.digijava.module.translation.jaxb.Translations;
import org.digijava.module.translation.lucene.TrnLuceneModule;
import org.digijava.module.translation.util.ImportExportUtil;
+import org.digijava.kernel.util.XmlSecurityUtils;
import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
@@ -314,7 +315,7 @@ private boolean doImport(HttpServletRequest request, ImportExportForm ioForm, Ht
request.getSession().setAttribute(SESSION_FILE, uploadedFile);
try {
Unmarshaller unmarshaller = ImportExportUtil.getUnmarshaler();
- Translations root = (Translations) unmarshaller.unmarshal(inputStream);
+ Translations root = (Translations) unmarshaller.unmarshal(XmlSecurityUtils.secureSource(inputStream));
request.getSession().setAttribute(SESSION_ROOT, root);
Set languagesInFile = ImportExportUtil.extractUsedLangages(root);
ioForm.setImportedLanguages(new ArrayList(languagesInFile));
diff --git a/amp/src/main/java/org/digijava/module/translation/util/ImportExportUtil.java b/amp/src/main/java/org/digijava/module/translation/util/ImportExportUtil.java
index 25369865073..25a0c55e09d 100644
--- a/amp/src/main/java/org/digijava/module/translation/util/ImportExportUtil.java
+++ b/amp/src/main/java/org/digijava/module/translation/util/ImportExportUtil.java
@@ -28,6 +28,7 @@
import org.digijava.module.translation.importexport.TranslationSearcher;
import org.digijava.module.translation.jaxb.Language;
import org.digijava.module.translation.jaxb.Translations;
+import org.digijava.kernel.util.XmlSecurityUtils;
import org.digijava.module.translation.jaxb.Trn;
import org.digijava.module.translation.util.importexport.ImportResult;
import org.digijava.module.translation.util.importexport.ImportRowConsumerCallable;
@@ -322,9 +323,11 @@ private static Translations getRootNode(File file){
Translations root = null;
try {
Unmarshaller unmarshaller = getUnmarshaler();
- root = (Translations) unmarshaller.unmarshal(file);
+ root = (Translations) unmarshaller.unmarshal(XmlSecurityUtils.secureSource(file));
} catch (JAXBException e) {
logger.error(e.getMessage(), e);
+ } catch (java.io.FileNotFoundException e) {
+ logger.error(e.getMessage(), e);
}
return root;
}
diff --git a/amp/src/main/java/org/digijava/module/um/action/RegisterUser.java b/amp/src/main/java/org/digijava/module/um/action/RegisterUser.java
index a0238dea28a..ddc90ff4a3d 100644
--- a/amp/src/main/java/org/digijava/module/um/action/RegisterUser.java
+++ b/amp/src/main/java/org/digijava/module/um/action/RegisterUser.java
@@ -17,11 +17,13 @@
import org.digijava.kernel.request.Site;
import org.digijava.kernel.request.SiteDomain;
import org.digijava.kernel.security.PasswordPolicyValidator;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.translator.TranslatorWorker;
import org.digijava.kernel.user.Group;
import org.digijava.kernel.user.User;
import org.digijava.kernel.util.DgUtil;
import org.digijava.kernel.util.RequestUtils;
+import org.digijava.kernel.util.ShaCrypt;
import org.digijava.module.aim.dbentity.*;
import org.digijava.module.aim.helper.GlobalSettingsConstants;
import org.digijava.module.aim.util.FeaturesUtil;
@@ -71,9 +73,10 @@ public ActionForward execute(ActionMapping mapping, ActionForm form,
request.setAttribute(PasswordPolicyValidator.SHOW_PASSWORD_POLICY_RULES, true);
return (mapping.getInputForward());
}
- // set password
- user.setPassword(userRegisterForm.getPassword().trim());
- user.setSalt(userRegisterForm.getPassword().trim());
+ // set password (AMP-SEC-017/054: never store the plaintext password)
+ String hashedPassword = new AmpPasswordEncoder().encode(ShaCrypt.crypt(userRegisterForm.getPassword().trim()).trim());
+ user.setPassword(hashedPassword);
+ user.setSalt(hashedPassword);
// set Website
user.setUrl(userRegisterForm.getWebSite());
diff --git a/amp/src/main/java/org/digijava/module/um/action/UserRegister.java b/amp/src/main/java/org/digijava/module/um/action/UserRegister.java
index 28c28a04ca7..26b487b5f04 100644
--- a/amp/src/main/java/org/digijava/module/um/action/UserRegister.java
+++ b/amp/src/main/java/org/digijava/module/um/action/UserRegister.java
@@ -34,10 +34,12 @@
import org.digijava.kernel.entity.UserLangPreferences;
import org.digijava.kernel.entity.UserPreferences;
import org.digijava.kernel.request.SiteDomain;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.user.User;
import org.digijava.kernel.util.DgUtil;
import org.digijava.kernel.util.I18NHelper;
import org.digijava.kernel.util.RequestUtils;
+import org.digijava.kernel.util.ShaCrypt;
import org.digijava.module.um.form.UserRegisterForm;
import org.digijava.module.um.util.DbUtil;
@@ -82,9 +84,10 @@ public ActionForward execute(ActionMapping mapping,
// set client IP address
user.setModifyingIP(RequestUtils.getRemoteAddress(request));
- // set password
- user.setPassword(userRegisterForm.getPassword().trim());
- user.setSalt(userRegisterForm.getPassword().trim());
+ // set password (AMP-SEC-017/054: never store the plaintext password)
+ String hashedPassword = new AmpPasswordEncoder().encode(ShaCrypt.crypt(userRegisterForm.getPassword().trim()).trim());
+ user.setPassword(hashedPassword);
+ user.setSalt(hashedPassword);
// set Website
user.setUrl(userRegisterForm.getWebSite());
diff --git a/amp/src/main/java/org/digijava/module/um/action/UserRegisterBlank.java b/amp/src/main/java/org/digijava/module/um/action/UserRegisterBlank.java
index e538105030e..23b247bebbe 100644
--- a/amp/src/main/java/org/digijava/module/um/action/UserRegisterBlank.java
+++ b/amp/src/main/java/org/digijava/module/um/action/UserRegisterBlank.java
@@ -34,9 +34,11 @@
import org.digijava.kernel.mail.DgEmailManager;
import org.digijava.kernel.request.SiteDomain;
import org.digijava.kernel.security.HttpLoginManager;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.user.User;
import org.digijava.kernel.util.DgUtil;
import org.digijava.kernel.util.RequestUtils;
+import org.digijava.kernel.util.ShaCrypt;
import org.digijava.kernel.util.SiteUtils;
import org.digijava.module.um.form.UserRegisterForm;
import org.digijava.module.um.util.DbUtil;
@@ -77,9 +79,10 @@ public ActionForward execute(ActionMapping mapping,
// set client IP address
user.setModifyingIP(RequestUtils.getRemoteAddress(request));
- // set password
- user.setPassword(userRegisterForm.getPassword().trim());
- user.setSalt(userRegisterForm.getPassword().trim());
+ // set password (AMP-SEC-017/054: never store the plaintext password)
+ String hashedPassword = new AmpPasswordEncoder().encode(ShaCrypt.crypt(userRegisterForm.getPassword().trim()).trim());
+ user.setPassword(hashedPassword);
+ user.setSalt(hashedPassword);
// set Website
user.setUrl(userRegisterForm.getWebSite());
diff --git a/amp/src/main/java/org/digijava/module/um/util/DbUtil.java b/amp/src/main/java/org/digijava/module/um/util/DbUtil.java
index c0b355e72b6..628edd8c0c3 100644
--- a/amp/src/main/java/org/digijava/module/um/util/DbUtil.java
+++ b/amp/src/main/java/org/digijava/module/um/util/DbUtil.java
@@ -24,6 +24,7 @@
import org.digijava.kernel.exception.DgException;
import org.digijava.kernel.persistence.PersistenceManager;
import org.digijava.kernel.request.Site;
+import org.digijava.kernel.security.auth.AmpPasswordEncoder;
import org.digijava.kernel.user.Group;
import org.digijava.kernel.user.User;
import org.digijava.kernel.util.*;
@@ -102,6 +103,17 @@ public static boolean isCorrectPassword(String user, String pass) throws
//////////////////////
while(iter.hasNext()) {
User iterUser = (User) iter.next();
+ AmpPasswordEncoder passwordEncoder = new AmpPasswordEncoder();
+
+ if (passwordEncoder.isHashed(iterUser.getPassword())) {
+ // password was already upgraded (e.g. on a successful login); the login widget
+ // hashes with SHA1 before this encoder bcrypt-wraps it, so mirror that here
+ String sha1OfPass = ShaCrypt.crypt(pass.trim()).trim();
+ if (passwordEncoder.matches(sha1OfPass, iterUser.getPassword())) {
+ iscorrect = true;
+ }
+ continue;
+ }
for(int i = 0; i < 3; i++) {
@@ -198,7 +210,7 @@ public static boolean ResetPassword(String email, String code, String newPasswor
return false;
}
- iterUser.setPassword(ShaCrypt.crypt(newPassword.trim()).trim());
+ iterUser.setPassword(hashNewPassword(newPassword));
iterUser.setSalt(new Long(newPassword.trim().hashCode()).toString());
session.update(iterUser);
session.delete(resetPassword);
@@ -223,6 +235,14 @@ public static boolean ResetPassword(String email, String code, String newPasswor
public static void updatePassword(String user, String newPassword) throws UMException{
updatePassword(user, null, newPassword);
}
+
+ /**
+ * Bcrypt-wraps the SHA1(password) value, matching the domain the login widget submits
+ * (see AmpPasswordEncoder / SecurityService.verifyCredentials).
+ */
+ private static String hashNewPassword(String newPassword) {
+ return new AmpPasswordEncoder().encode(ShaCrypt.crypt(newPassword.trim()).trim());
+ }
/**
* Update password in database see table
*
@@ -238,7 +258,7 @@ public static void updatePassword(String user, String oldPassword,
session = PersistenceManager.getSession();
User userToUpdate = UserUtils.getUserByEmailAddress(user);
- userToUpdate.setPassword(ShaCrypt.crypt(newPassword.trim()).trim());
+ userToUpdate.setPassword(hashNewPassword(newPassword));
userToUpdate.setSalt(new Long(newPassword.trim().hashCode()).toString());
userToUpdate.updateLastModified();
session.saveOrUpdate(userToUpdate);
@@ -411,12 +431,8 @@ public static void registerUser(User user) throws UMException {
session = PersistenceManager.getSession();
//beginTransaction();
- // set encrypted password
- user.setPassword(ShaCrypt.crypt(user.getPassword().trim()).trim());
-
- // set hashed password
- user.setSalt(new Long(user.getPassword().trim().hashCode()).
- toString());
+ // AMP-SEC-017/054: caller already stores the password hashed via AmpPasswordEncoder;
+ // do not re-hash it here with the legacy (unsalted SHA1) ShaCrypt scheme
// update user
session.save(user);
diff --git a/amp/src/main/java/org/digijava/module/xmlpatcher/util/XmlPatcherUtil.java b/amp/src/main/java/org/digijava/module/xmlpatcher/util/XmlPatcherUtil.java
index 80f250641a5..887d6f4d3dd 100644
--- a/amp/src/main/java/org/digijava/module/xmlpatcher/util/XmlPatcherUtil.java
+++ b/amp/src/main/java/org/digijava/module/xmlpatcher/util/XmlPatcherUtil.java
@@ -212,10 +212,22 @@ public static Set getAllDiscoveredPatchNames() throws DgException,
return ret;
}
- static javax.xml.transform.TransformerFactory transFact = javax.xml.transform.TransformerFactory.newInstance( );
+ static javax.xml.transform.TransformerFactory transFact = createSecureTransformerFactory();
static javax.xml.transform.Transformer cached_transformer;
static String lastPathTransformerPath = null;
+ // AMP-SEC-026/062: block XXE via XSLT (external DTD/stylesheet access)
+ private static javax.xml.transform.TransformerFactory createSecureTransformerFactory() {
+ javax.xml.transform.TransformerFactory factory = javax.xml.transform.TransformerFactory.newInstance();
+ try {
+ factory.setAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD, "");
+ factory.setAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+ } catch (IllegalArgumentException e) {
+ logger.warn("TransformerFactory implementation does not support restricting external access", e);
+ }
+ return factory;
+ }
+
static Unmarshaller cached_unmarshaller;
static String lastUnmarshallerPath = null;
@@ -244,6 +256,13 @@ static Unmarshaller getUnmarshaller(String schemaURI) throws JAXBException, SAXE
// initialize JAXB 2.0 validation
SchemaFactory sf = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema");
+ // AMP-SEC-026/062: block XXE via the XSD (external DTD/schema access)
+ try {
+ sf.setProperty(javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD, "");
+ sf.setProperty(javax.xml.XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+ } catch (SAXException e) {
+ logger.warn("SchemaFactory implementation does not support restricting external access", e);
+ }
Schema schema = sf.newSchema(new File(schemaURI));
cached_unmarshaller.setSchema(schema);
cached_unmarshaller.setEventHandler(new DefaultValidationEventHandler());
diff --git a/amp/src/main/webapp/WEB-INF/applicationContext.xml b/amp/src/main/webapp/WEB-INF/applicationContext.xml
index 7af9c51952d..fd445c968bc 100644
--- a/amp/src/main/webapp/WEB-INF/applicationContext.xml
+++ b/amp/src/main/webapp/WEB-INF/applicationContext.xml
@@ -109,7 +109,14 @@
-
+
+
+
+
+
+
@@ -224,6 +231,14 @@
+
+
+
+
+
+
+
@@ -275,8 +290,9 @@
-
+
+