From f214afa977f288938ed3fc0306e271176f9e3552 Mon Sep 17 00:00:00 2001
From: Luc Boudreau
Example Request:
+ * PUT pentaho/api/repo/files/:jmeter-test:test_file_1.xml/acl
+ *
PUT data:
+ *
+ * <?xml version="1.0" encoding="UTF-8" standalone="yes"?><repositoryFileAclDto><entriesInheriting>true</entriesInheriting><id>d45d4972-989e-48d5-8bd0-f7024a77f08f</id><owner>admin</owner><ownerType>0</ownerType></repositoryFileAclDto> + *+ * + * + * @param pathId Colon separated path for the repository file. + * @param acl Acl of the repository file RepositoryFileAclDto. + * + * @return A jax-rs Response object with the appropriate status code, header, and body. + * + *
Example Response:
+ *+ * This response does not contain data. + *+ */ + @PUT + @Path ( "{pathId : .+}/acl" ) + @Consumes ( { MediaType.APPLICATION_JSON } ) + @StatusCodes ( { + @ResponseCode ( code = 200, condition = "Successfully saved file." ), + @ResponseCode ( code = 403, condition = "Failed to save acls due to missing or incorrect properties." ), + @ResponseCode ( code = 400, condition = "Failed to save acls due to malformed xml." ), + @ResponseCode ( code = 500, condition = "Failed to save acls due to another error." ) } ) + public Response setFileAcls( @PathParam ( "pathId" ) String pathId, RepositoryFileAclDto acl ) { /* * [BISERVER-14294] Ensuring the owner is set to a non-null, non-empty string value to prevent any issues * that might cause problems with the repository. Then following it up with a user existence check @@ -766,6 +836,30 @@ public Response setFileAcls( @PathParam ( "pathId" ) String pathId, RepositoryFi } } + protected XMLStreamReader getSecureXmlStreamReader( StreamSource xmlSource ) throws XMLStreamException { + XMLInputFactory xif = XMLInputFactory.newFactory(); + xif.setProperty( XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false ); + xif.setProperty( XMLInputFactory.SUPPORT_DTD, false ); + XMLStreamReader xsr = xif.createXMLStreamReader( xmlSource ); + return xsr; + } + + protected Unmarshaller getUnmarshaller( Class> clazz ) throws JAXBException { + ContextResolver
* :path:to:file:id *- * @param contentCreator Repository file that created the file at the above pathId location + * @param contentCreatorXml Repository file that created the file at the above pathId location *
* <repositoryFileDto> * <createdDate>1402911997019</createdDate> @@ -820,11 +914,86 @@ public Response setFileAcls( @PathParam ( "pathId" ) String pathId, RepositoryFi */ @PUT @Path ( "{pathId : .+}/creator" ) - @Consumes ( { MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON } ) + @Consumes ( { MediaType.APPLICATION_XML } ) @StatusCodes ( { @ResponseCode ( code = 200, condition = "Successfully retrieved file." ), @ResponseCode ( code = 500, condition = "Failed to download file because of some other error." ) } ) @Facet ( name = "Unsupported" ) + public Response doSetContentCreator( @PathParam ( "pathId" ) String pathId, StreamSource contentCreatorXml ) { + try { + Unmarshaller unmarshaller = getUnmarshaller( RepositoryFileDto.class ); + XMLStreamReader xsr = getSecureXmlStreamReader( contentCreatorXml ); + RepositoryFileDto contentCreator = (RepositoryFileDto) unmarshaller.unmarshal( xsr ); + fileService.doSetContentCreator( pathId, contentCreator ); + return buildOkResponse(); + } catch ( FileNotFoundException e ) { + logger.error( getMessagesInstance().getErrorString( "FileResource.FILE_NOT_FOUND", pathId ), e ); + return buildStatusResponse( Response.Status.NOT_FOUND ); + } catch ( Throwable t ) { + logger.error( getMessagesInstance().getString( "SystemResource.GENERAL_ERROR" ), t ); + return buildStatusResponse( Response.Status.INTERNAL_SERVER_ERROR ); + } + } + + /** + * Store content creator for the given path of created content. + * + * @param pathId colon separated path for the repository file that was created by the contenCreator below + *+ * :path:to:file:id + *+ * @param contentCreator Repository file that created the file at the above pathId location + *+ * <repositoryFileDto> + * <createdDate>1402911997019</createdDate> + * <fileSize>3461</fileSize> + * <folder>false</folder> + * <hidden>false</hidden> + * <id>ff11ac89-7eda-4c03-aab1-e27f9048fd38</id> + * <lastModifiedDate>1406647160536</lastModifiedDate> + * <locale>en</locale> + * <localePropertiesMapEntries> + * <localeMapDto> + * <locale>default</locale> + * <properties> + * <stringKeyStringValueDto> + * <key>file.title</key> + * <value>myFile</value> + * </stringKeyStringValueDto> + * <stringKeyStringValueDto> + * <key>jcr:primaryType</key> + * <value>nt:unstructured</value> + * </stringKeyStringValueDto> + * <stringKeyStringValueDto> + * <key>title</key> + * <value>myFile</value> + * </stringKeyStringValueDto> + * <stringKeyStringValueDto> + * <key>file.description</key> + * <value>myFile Description</value> + * </stringKeyStringValueDto> + * </properties> + * </localeMapDto> + * </localePropertiesMapEntries> + * <locked>false</locked> + * <name>myFile.prpt</name></name> + * <originalParentFolderPath>/public/admin</originalParentFolderPath> + * <ownerType>-1</ownerType> + * <path>/public/admin/ff11ac89-7eda-4c03-aab1-e27f9048fd38</path> + * <title>myFile</title> + * <versionId>1.9</versionId> + * <versioned>true</versioned> + * </repositoryFileAclDto> + *+ * @return A jax-rs Response object with the appropriate status code, header, and body. + */ + @PUT + @Path ( "{pathId : .+}/creator" ) + @Consumes ( { MediaType.APPLICATION_JSON } ) + @StatusCodes ( { + @ResponseCode ( code = 200, condition = "Successfully retrieved file." ), + @ResponseCode ( code = 500, condition = "Failed to download file because of some other error." ) } ) + @Facet ( name = "Unsupported" ) public Response doSetContentCreator( @PathParam ( "pathId" ) String pathId, RepositoryFileDto contentCreator ) { try { fileService.doSetContentCreator( pathId, contentCreator ); @@ -1994,11 +2163,12 @@ public Response doRename( @PathParam( "pathId" ) String pathId, @QueryParam( "ne @PUT @Path ( "{pathId : .+}/metadata" ) @Produces ( { MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON } ) + @Consumes ( { MediaType.APPLICATION_JSON } ) @StatusCodes ( { - @ResponseCode ( code = 200, condition = "Successfully retrieved metadata." ), - @ResponseCode ( code = 403, condition = "Invalid path." ), - @ResponseCode ( code = 400, condition = "Invalid payload." ), - @ResponseCode ( code = 500, condition = "Server Error." ) } ) + @ResponseCode ( code = 200, condition = "Successfully retrieved metadata." ), + @ResponseCode ( code = 403, condition = "Invalid path." ), + @ResponseCode ( code = 400, condition = "Invalid payload." ), + @ResponseCode ( code = 500, condition = "Server Error." ) } ) public Response doSetMetadata( @PathParam ( "pathId" ) String pathId, Listmetadata ) { try { fileService.doSetMetadata( pathId, metadata ); diff --git a/extensions/src/test/java/org/pentaho/platform/web/http/api/resources/FileResourceTest.java b/extensions/src/test/java/org/pentaho/platform/web/http/api/resources/FileResourceTest.java index c31fd45d886..66c4a2bd9d5 100644 --- a/extensions/src/test/java/org/pentaho/platform/web/http/api/resources/FileResourceTest.java +++ b/extensions/src/test/java/org/pentaho/platform/web/http/api/resources/FileResourceTest.java @@ -14,12 +14,13 @@ * See the GNU Lesser General Public License for more details. * * - * Copyright (c) 2002-2021 Hitachi Vantara. All rights reserved. + * Copyright (c) 2002-2024 Hitachi Vantara. All rights reserved. * */ package org.pentaho.platform.web.http.api.resources; +import com.ctc.wstx.exc.WstxLazyException; import org.dom4j.Document; import org.dom4j.Element; import org.junit.After; @@ -61,6 +62,13 @@ import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import javax.ws.rs.core.StreamingOutput; +import javax.xml.bind.JAXBContext; +import javax.xml.bind.JAXBException; +import javax.xml.bind.Unmarshaller; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import javax.xml.transform.stream.StreamSource; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; @@ -84,9 +92,11 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doCallRealMethod; @@ -935,6 +945,240 @@ public void testSetFileAclsErrorNoOwner() { assertEquals( FORBIDDEN.getStatusCode(), fileResource.setFileAcls( PATH_ID, repository ).getStatus() ); } + /* + * [PPP-5021] Ensure external entities cannot be inserted into XML payloads. + * Test validates that the secure parser actually breaks on external enties + */ + @Test + public void testSecureXmlProcessing() throws XMLStreamException, JAXBException { + String xmlDocWithExternalEntitiesLol = + " \n" + + "" + + "" + + "" + + "" + + "" + + "]>" + + "\n" + + " " + + " "; + doCallRealMethod().when( fileResource ).getUnmarshaller( RepositoryFileAclDto.class ); + doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); + Unmarshaller unmarshaller = fileResource.getUnmarshaller( RepositoryFileAclDto.class ); + XMLStreamReader xsr = fileResource.getSecureXmlStreamReader( new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ); + try { + unmarshaller.unmarshal( xsr ); + fail(); + } catch ( WstxLazyException e ) { + assertTrue( e.getMessage().contains( "Undeclared general entity \"lol4\"" ) ); + } catch ( Exception e ) { + fail(); + } + } + + /* + * [PPP-5021] Ensure external entities cannot be inserted into XML payloads + */ + @Test + public void testSetFileAclsXxeError() throws JAXBException, XMLStreamException { + String xmlDocWithExternalEntitiesLol = + " \n" + + "" + + "" + + "" + + "" + + "" + + "]>" + + "\n" + + "true " + + "a56f5043-c5d7-4b6c-80d2-b5964bcfd860 " + + "admin&lol4; " + + "0 " + + "" + + " "; + + doCallRealMethod().when( fileResource ).setFileAcls( anyString(), any( StreamSource.class ) ); + doCallRealMethod().when( fileResource ).getUnmarshaller( any() ); + doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); + assertEquals( INTERNAL_SERVER_ERROR.getStatusCode(), + fileResource.setFileAcls( PATH_ID, new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ).getStatus() ); + } + + @Test + public void testSetFileAclsXxeErrorClean() throws JAXBException, XMLStreamException { + String xmlDocWithExternalEntitiesLol = + " \n" + + "true " + + "a56f5043-c5d7-4b6c-80d2-b5964bcfd860 " + + "admin&lol4; " + + "0 " + + "" + + " "; + + doCallRealMethod().when( fileResource ).setFileAcls( anyString(), any( StreamSource.class ) ); + doCallRealMethod().when( fileResource ).getUnmarshaller( any() ); + doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); + assertEquals( OK.getStatusCode(), + fileResource.setFileAcls( PATH_ID, new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ).getStatus() ); + } + + /* Note: tests commented out because we removed the Accepts: XML decorator from this method; should only + * accept JSON now. + * [PPP-5021] Ensure external entities cannot be inserted into XML payloads + */ +// @Test +// public void testDoSetMetadataXxeError() throws JAXBException, XMLStreamException { +// String xmlDocWithExternalEntitiesLol = +// "\n" +// + "" +// + "" +// + "" +// + "" +// + "" +// + "]>" +// + "\n" +// + "true " + + "a56f5043-c5d7-4b6c-80d2-b5964bcfd860 " + + "admin " + + "0 " + + ""; +// +// doCallRealMethod().when( fileResource ).doSetMetadata( anyString(), any( StreamSource.class ) ); +// doCallRealMethod().when( fileResource ).getUnmarshaller( any() ); +// doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); +// assertEquals( INTERNAL_SERVER_ERROR.getStatusCode(), +// fileResource.doSetMetadata( PATH_ID, new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ).getStatus() ); +// } + + /* + * [PPP-5021] Ensure external entities cannot be inserted into XML payloads + */ +// @Test +// public void testDoSetMetadataXxeErrorClean() throws JAXBException, XMLStreamException { +// String xmlDocWithExternalEntitiesLol = +// "\n" +// + " fooKey barValue&lol4; "; +// +// doCallRealMethod().when( fileResource ).doSetMetadata( anyString(), any( StreamSource.class ) ); +// doCallRealMethod().when( fileResource ).getUnmarshaller( any() ); +// doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); +// assertEquals( OK.getStatusCode(), +// fileResource.doSetMetadata( PATH_ID, new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ).getStatus() ); +// } + +// public void generateStringKeyAndValueXml() throws JAXBException { +// // save this in case someone needs to rebuild the XML +// StringKeyStringValueDto stringKeyStringValueDto1 = new StringKeyStringValueDto(); +// stringKeyStringValueDto1.setKey( "fooKey" ); +// stringKeyStringValueDto1.setValue( "barValue" ); +// StringKeyStringValueDto stringKeyStringValueDto2 = new StringKeyStringValueDto(); +// stringKeyStringValueDto2.setKey( "fooKey" ); +// stringKeyStringValueDto2.setValue( "barValue" ); +// List " +// + " fooKey barValue fooKey1 barValue1 stringKeyStringValueDtoList = new ArrayList<>(); +// stringKeyStringValueDtoList.add( stringKeyStringValueDto1 ); +// stringKeyStringValueDtoList.add( stringKeyStringValueDto2 ); +// JAXBContext jaxbContext = JAXBContext.newInstance( StringKeyStringValueDto.class, ArrayList.class ); +// ByteArrayOutputStream bos = new ByteArrayOutputStream(); +// jaxbContext.createMarshaller().marshal( stringKeyStringValueDtoList, bos ); +// System.out.println( bos.toString() ); +// } + + /* + * [PPP-5021] Ensure external entities cannot be inserted into XML payloads + */ + @Test + public void testDoSetContentCreatorXxeError() throws JAXBException, XMLStreamException { + String xmlDocWithExternalEntitiesLol = + "\n" + + "" + + "" + + "" + + "" + + "" + + "]>" + + "\n" + + " " + + " \n"; + + doCallRealMethod().when( fileResource ).doSetContentCreator( anyString(), any( StreamSource.class ) ); + doCallRealMethod().when( fileResource ).getUnmarshaller( any() ); + doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); + assertEquals( INTERNAL_SERVER_ERROR.getStatusCode(), + fileResource.doSetContentCreator( PATH_ID, new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ).getStatus() ); + } + + /* + * [PPP-5021] Ensure external entities cannot be inserted into XML payloads + */ + @Test + public void testDoSetContentCreatorXxeErrorClean() throws JAXBException, XMLStreamException { + String xmlDocWithExternalEntitiesLol = + "\n" + + "false " + + "01-01-2020 " + + "creatorFoo " + + "barDescription " + + "42 " + + "false " + + "false " + + "zardozId " + + "false " + + "fileNameField&lol4; " + + "false " + + "nobody " + + "-1 " + + "/home/nobody " + + "false " + + "" + + " \n"; + + doCallRealMethod().when( fileResource ).doSetContentCreator( anyString(), any( StreamSource.class ) ); + doCallRealMethod().when( fileResource ).getUnmarshaller( any() ); + doCallRealMethod().when( fileResource ).getSecureXmlStreamReader( any( StreamSource.class ) ); + assertEquals( OK.getStatusCode(), + fileResource.doSetContentCreator( PATH_ID, new StreamSource( new ByteArrayInputStream( xmlDocWithExternalEntitiesLol.getBytes() ) ) ).getStatus() ); + } + + private void generateRepositoryFileDtoXml() throws JAXBException { + // save this in case someone needs to rebuild the XML + RepositoryFileDto repositoryFileDto = new RepositoryFileDto(); + repositoryFileDto.setCreatorId( "creatorFoo" ); + repositoryFileDto.setFileSize( 42L ); + repositoryFileDto.setCreatedDate( "01-01-2020" ); + repositoryFileDto.setDescription( "barDescription" ); + repositoryFileDto.setFolder( false ); + repositoryFileDto.setAclNode( false ); + repositoryFileDto.setHidden( false ); + repositoryFileDto.setId( "zardozId" ); + repositoryFileDto.setName( "fileNameField" ); + repositoryFileDto.setOwner( "nobody" ); + repositoryFileDto.setPath( "/home/nobody" ); + JAXBContext jaxbContext = JAXBContext.newInstance( RepositoryFileDto.class ); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + jaxbContext.createMarshaller().marshal( repositoryFileDto, bos ); + System.out.println( bos.toString() ); + } + @Test public void testSetContentCreator() throws Exception {false " + + "01-01-2020 " + + "creatorFoo " + + "barDescription " + + "42 " + + "false " + + "false " + + "zardozId " + + "false " + + "fileNameField " + + "false " + + "nobody " + + "-1 " + + "/home/nobody " + + "false " + + "