diff options
8 files changed, 148 insertions, 22 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java new file mode 100644 index 00000000000..5f430f70584 --- /dev/null +++ b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java @@ -0,0 +1,18 @@ +package com.yahoo.config.application; + +import org.w3c.dom.Document; +import org.w3c.dom.NodeList; + +import javax.xml.transform.TransformerException; +import java.io.IOException; + +public class ValidationProcessor implements PreProcessor { + + @Override + public Document process(Document input) throws IOException, TransformerException { + NodeList includeitems = input.getElementsByTagNameNS("http://www.w3.org/2001/XInclude", "*"); + if (includeitems.getLength() > 0) + throw new UnsupportedOperationException("XInclude not supported, use preprocess:include instead"); + return input; + } +}
\ No newline at end of file diff --git a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java index 640cc7bcfa7..ba68894c9f9 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java @@ -5,6 +5,7 @@ import com.yahoo.config.application.FileSystemWrapper.FileWrapper; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; +import com.yahoo.text.XML; import org.w3c.dom.Document; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -57,7 +58,7 @@ public class XmlPreProcessor { } public Document run() throws ParserConfigurationException, IOException, SAXException, TransformerException { - DocumentBuilder docBuilder = Xml.getPreprocessDocumentBuilder(); + DocumentBuilder docBuilder = XML.getDocumentBuilder(); Document document = docBuilder.parse(new InputSource(xmlInput)); return execute(document); } @@ -74,6 +75,7 @@ public class XmlPreProcessor { chain.add(new IncludeProcessor(applicationDir)); chain.add(new OverrideProcessor(instance, environment, region)); chain.add(new PropertiesProcessor()); + chain.add(new ValidationProcessor()); // must be last in chain return chain; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java index 1642a8cb3cc..ea6a081bf6a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java @@ -9,6 +9,7 @@ import com.yahoo.config.application.api.ComponentInfo; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.path.Path; +import com.yahoo.text.XML; import com.yahoo.vespa.model.VespaModel; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -16,8 +17,6 @@ import org.w3c.dom.NodeList; import org.xml.sax.InputSource; import org.xml.sax.SAXException; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; @@ -96,20 +95,19 @@ public abstract class AbstractBundleValidator extends Validator { } private static final Pattern POM_FILE_LOCATION = Pattern.compile("META-INF/maven/.+?/.+?/pom.xml"); - private Optional<Document> getPomXmlContent(DeployLogger deployLogger, JarFile jar) { + public Optional<Document> getPomXmlContent(DeployLogger deployLogger, JarFile jar) { return jar.stream() .filter(f -> POM_FILE_LOCATION.matcher(f.getName()).matches()) .findFirst() .map(f -> { try { String text = new String(jar.getInputStream(f).readAllBytes()); - return DocumentBuilderFactory.newDefaultInstance().newDocumentBuilder() + return XML.getDocumentBuilder(false) .parse(new InputSource(new StringReader(text))); - } catch (ParserConfigurationException e) { - throw new RuntimeException(e); } catch (SAXException e) { - deployLogger.log(Level.INFO, String.format("Unable to parse pom.xml from %s", filename(jar))); - return null; + String message = String.format("Unable to parse pom.xml from %s", filename(jar)); + deployLogger.log(Level.SEVERE, message); + throw new RuntimeException(message, e); } catch (IOException e) { deployLogger.log(Level.INFO, String.format("Unable to read '%s' from '%s'", f.getName(), jar.getName())); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index 1e073ac3458..63ba8197960 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -6,6 +6,9 @@ import com.yahoo.component.Version; import com.yahoo.component.Vtag; import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.config.FileReference; +import com.yahoo.config.application.ValidationProcessor; +import com.yahoo.config.application.XmlPreProcessor; +import com.yahoo.config.application.api.ApplicationMetaData; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.FileRegistry; @@ -26,6 +29,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.net.HostName; import com.yahoo.path.Path; +import com.yahoo.text.XML; import com.yahoo.vespa.config.server.ConfigServerSpec; import com.yahoo.vespa.config.server.TimeoutBudget; import com.yahoo.vespa.config.server.application.ApplicationSet; @@ -47,9 +51,14 @@ import com.yahoo.vespa.config.server.tenant.EndpointCertificateRetriever; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.model.application.validation.BundleValidator; +import org.xml.sax.SAXException; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.TransformerException; import java.io.File; import java.io.IOException; +import java.nio.file.Files; import java.security.cert.X509Certificate; import java.time.Instant; import java.util.Collection; @@ -58,9 +67,11 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; +import java.util.jar.JarFile; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import java.util.zip.ZipException; import static com.yahoo.vespa.config.server.ConfigServerSpec.fromConfig; @@ -238,6 +249,7 @@ public class SessionPreparer { void preprocess() { try { + validateXmlFeatures(applicationPackage, logger); this.preprocessedApplicationPackage = applicationPackage.preprocess(zone, logger); } catch (IOException | RuntimeException e) { throw new IllegalArgumentException("Error preprocessing application package for " + applicationId + @@ -246,6 +258,74 @@ public class SessionPreparer { checkTimeout("preprocess"); } + /** + * Warn on use of deprecated XML features + */ + private void validateXmlFeatures(ApplicationPackage applicationPackage, DeployLogger logger) { + // TODO: Validate no use of XInclude, datatype definitions or external entities + // in any xml file we parse, such as services.xml, deployment.xml, hosts.xml, + // validation-overrides.xml and any pom.xml files in OSGi bundles + // services.xml and hosts.xml will need to be preprocessed by our own processors first + + File applicationPackageDir = applicationPackage.getFileReference(Path.fromString(".")); + File servicesXml = applicationPackage.getFileReference(Path.fromString("services.xml")); + File hostsXml = applicationPackage.getFileReference(Path.fromString("hosts.xml")); + + // Validate after doing our own preprocessing on these two files + if(servicesXml.exists()) { + vespaPreprocess(applicationPackageDir.getAbsoluteFile(), servicesXml, applicationPackage.getMetaData()); + } + if(hostsXml.exists()) { + vespaPreprocess(applicationPackageDir.getAbsoluteFile(), hostsXml, applicationPackage.getMetaData()); + } + + if (zone.system().isPublic()) { + // Validate all other XML files + try (var paths = Files.find(applicationPackageDir.getAbsoluteFile().toPath(), Integer.MAX_VALUE, + (path, attr) -> attr.isRegularFile() && path.getFileName().toString().matches(".*\\.[Xx][Mm][Ll]"))) { + paths.filter(p -> !(p.equals(servicesXml.getAbsoluteFile().toPath()) || p.equals(hostsXml.getAbsoluteFile().toPath()))) + .forEach(xmlPath -> { + try { + new ValidationProcessor().process(XML.getDocument(xmlPath.toFile())); + } catch (IOException | TransformerException e) { + throw new RuntimeException(e); + } + }); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + // Validate pom.xml files in OSGi bundles + try (var paths = Files.find(applicationPackageDir.getAbsoluteFile().toPath(), Integer.MAX_VALUE, + (path, attr) -> attr.isRegularFile() && path.getFileName().toString().matches(".*\\.[Jj][Aa][Rr]"))) { + paths.forEach(jarPath -> { + try { + new BundleValidator().getPomXmlContent(logger, new JarFile(jarPath.toFile())); + } catch (ZipException e) { + // ignore for tests + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + void vespaPreprocess(File appDir, File inputXml, ApplicationMetaData metaData) { + try { + new XmlPreProcessor(appDir, + inputXml, + metaData.getApplicationId().instance(), + zone.environment(), + zone.region()) + .run(); + } catch (ParserConfigurationException | IOException | SAXException | TransformerException e) { + throw new RuntimeException(e); + } + } + AllocatedHosts buildModels(Instant now) { var allocatedHosts = new AllocatedHostsFromAllModels(); this.modelResultList = preparedModelsBuilder.buildModels(applicationId, dockerImageRepository, vespaVersion, diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java index 7f16505c500..d7ef20c31c8 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java @@ -150,7 +150,7 @@ public class DefaultZmsClient extends ClientBase implements ZmsClient { @Override public void addRoleMember(AthenzRole role, AthenzIdentity member, Optional<String> reason) { URI uri = zmsUrl.resolve(String.format("domain/%s/role/%s/member/%s", role.domain().getName(), role.roleName(), member.getFullName())); - MembershipEntity membership = new MembershipEntity.RoleMembershipEntity(member.getFullName(), true, role.roleName(), null); + MembershipEntity membership = new MembershipEntity.RoleMembershipEntity(member.getFullName(), true, role.roleName(), null, true); RequestBuilder requestBuilder = RequestBuilder.put(uri) @@ -176,7 +176,7 @@ public class DefaultZmsClient extends ClientBase implements ZmsClient { .build(); return execute(request, response -> { MembershipEntity membership = readEntity(response, MembershipEntity.GroupMembershipEntity.class); - return membership.isMember; + return membership.isMember && membership.approved; }); } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/bindings/MembershipEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/bindings/MembershipEntity.java index dcffe006112..ef97fb02bfa 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/bindings/MembershipEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/bindings/MembershipEntity.java @@ -17,14 +17,17 @@ public class MembershipEntity { public final String memberName; public final boolean isMember; public final String expiration; + public final boolean approved; @JsonCreator public MembershipEntity(@JsonProperty("memberName") String memberName, @JsonProperty("isMember") boolean isMember, - @JsonProperty("expiration") String expiration) { + @JsonProperty("expiration") String expiration, + @JsonProperty("approved") boolean approved) { this.memberName = memberName; this.isMember = isMember; this.expiration = expiration; + this.approved = approved; } @JsonGetter("memberName") @@ -49,8 +52,9 @@ public class MembershipEntity { public RoleMembershipEntity(@JsonProperty("memberName") String memberName, @JsonProperty("isMember") boolean isMember, @JsonProperty("roleName") String roleName, - @JsonProperty("expiration") String expiration) { - super(memberName, isMember, expiration); + @JsonProperty("expiration") String expiration, + @JsonProperty("approved") boolean approved) { + super(memberName, isMember, expiration, approved); this.roleName = roleName; } @@ -62,16 +66,13 @@ public class MembershipEntity { } public static class RoleMembershipDecisionEntity extends RoleMembershipEntity { - public final boolean approved; - @JsonCreator public RoleMembershipDecisionEntity(@JsonProperty("memberName") String memberName, @JsonProperty("isMember") boolean isMember, @JsonProperty("roleName") String roleName, @JsonProperty("expiration") String expiration, @JsonProperty("approved") boolean approved) { - super(memberName, isMember, roleName, expiration); - this.approved = approved; + super(memberName, isMember, roleName, expiration, approved); } } @@ -83,8 +84,9 @@ public class MembershipEntity { public GroupMembershipEntity(@JsonProperty("memberName") String memberName, @JsonProperty("isMember") boolean isMember, @JsonProperty("groupName") String groupName, - @JsonProperty("expiration") String expiration) { - super(memberName, isMember, expiration); + @JsonProperty("expiration") String expiration, + @JsonProperty("approved") boolean approved) { + super(memberName, isMember, expiration, approved); this.groupName = groupName; } diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 97eaa1f76f6..fbf1203acdf 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -3358,6 +3358,8 @@ "public static org.w3c.dom.Document getDocument(java.lang.String)", "public static javax.xml.parsers.DocumentBuilder getDocumentBuilder()", "public static javax.xml.parsers.DocumentBuilder getDocumentBuilder(java.lang.String, java.lang.ClassLoader)", + "public static javax.xml.parsers.DocumentBuilder getDocumentBuilder(boolean)", + "public static javax.xml.parsers.DocumentBuilder getDocumentBuilder(java.lang.String, java.lang.ClassLoader, boolean)", "public static java.util.List getChildren(org.w3c.dom.Element)", "public static java.util.List getChildren(org.w3c.dom.Element, java.lang.String)", "public static java.lang.String getValue(org.w3c.dom.Element)", diff --git a/vespajlib/src/main/java/com/yahoo/text/XML.java b/vespajlib/src/main/java/com/yahoo/text/XML.java index 6cc042123dc..6aa42773ac0 100644 --- a/vespajlib/src/main/java/com/yahoo/text/XML.java +++ b/vespajlib/src/main/java/com/yahoo/text/XML.java @@ -452,7 +452,7 @@ public class XML { * @throws RuntimeException if we fail to create one */ public static DocumentBuilder getDocumentBuilder() { - return getDocumentBuilder("com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl", null); + return getDocumentBuilder("com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl", null, true); } /** @@ -464,9 +464,33 @@ public class XML { * @return a DocumentBuilder */ public static DocumentBuilder getDocumentBuilder(String implementation, ClassLoader classLoader) { + return getDocumentBuilder(true); + } + + /** + * Creates a new XML DocumentBuilder + * + * @return a DocumentBuilder + * @throws RuntimeException if we fail to create one + * @param namespaceAware Whether the parser should be aware of xml namespaces + */ + public static DocumentBuilder getDocumentBuilder(boolean namespaceAware) { + return getDocumentBuilder("com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl", null, namespaceAware); + } + + /** + * Creates a new XML DocumentBuilder + * + * @param implementation which jaxp implementation should be used + * @param classLoader which class loader should be used when getting a new DocumentBuilder + * @param namespaceAware Whether the parser should be aware of xml namespaces + * @throws RuntimeException if we fail to create one + * @return a DocumentBuilder + */ + public static DocumentBuilder getDocumentBuilder(String implementation, ClassLoader classLoader, boolean namespaceAware) { try { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(implementation, classLoader); - factory.setNamespaceAware(true); + factory.setNamespaceAware(namespaceAware); // Disable include directives. If enabled this allows inclusion of any resource, such as file:/// and // http:///, and these are read even if the document eventually fails to parse factory.setXIncludeAware(false); |