[XSDLookUp] Updated entity resolver to not fallback to network lookup when xsd is not found

[kavya-shastri]

Environment

Liquibase Version:

Liquibase Integration & Version: <Pick one: CLI, maven, gradle, spring boot, servlet, etc.>

Liquibase Extension(s) & Version:

Database Vendor & Version:

Operating System Type & Version:

Pull Request Type

• Bug fix (non-breaking change which fixes an issue.)
• Enhancement/New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

• Updated Liquibase entity resolver to not fall back to network lookup in case the xsd is not found in the bundled jar.

Steps To Reproduce

• Liquibase xsd lookup outage that happened few days ago caused a crash in our hosted application.
• In the xml changelog file for the attribute (xsi:schemaLocation) of the element databaseChangeLog, we used the value : http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.0.xsd
  while using the 3.6 version of liquibase jar.
  This mismatch was causing our application to do a network lookup every time during deployment. This scenario worked well until there was a liquibase outage which ultimately caused a downtime for our application as well.

Actual Behavior

Liquibase falls back to network lookup if the xsd file is not found locally or with the bundled liquibase jar.

Expected/Desired Behavior

The fallback to the network lookup be controlled by a flag which controls the network lookup for xsd file.

• If the flag is set to true, then used the network lookup fallback.
• Default value to be false which will throw an exception instead of doing the network lookup.
  The latter scenario is preferred since this will allow us to catch issues during the development stage.

Screenshots (if appropriate)

If applicable, add screenshots to help explain your problem.

Additional Context

Add any other context about the problem here.

Fast Track PR Acceptance Checklist:

• Build is successful and all new and existing tests pass
• Added Unit Test(s)
• Added Integration Test(s)
• Added Test Harness Test(s)
• Documentation Updated

Need Help?

Come chat with us on our discord channel

[kataggart]

Thanks for submitting this. We are taking a look. Appreciate the PR and description.

[kavya-shastri]

Thanks for submitting this. We are taking a look. Appreciate the PR and description.

@kataggart
Thank you!
I wanted to know if there was a better way to define the property for "ALLOW_NETWORK_LOOKUP" flag so that it can be controlled by the user?

[kataggart]

That's a good question @kavya-shastri -- let me reach out to someone on this end with a deeper knowledge and get you an answer. Also, if you aren't already aware of Liquibase Forum, you might want to go over there and post the question. Some experts hang out there and are always answering questions.

[kataggart]

ha @kavya-shastri ignore my suggestion to go to the Forum! I just saw you already posted there! https://forum.liquibase.org/t/urgent-changelog-xsd-lookup/6432/3 :)

Working to get you an answer.

[nvoxland]

For controlling whether to enable or disable this behavior, I think the best spot is to use the new GlobalConfiguration.SECURE_PARSING.getCurrentValue() setting.

With 4.8.0, we added that, which allows us to be more secure by default in our XML parsing but let people disable that when they need. For that feature, we still had to allow http/https lookup for XSD or it broke the liquibase XSD references even though they are actually local. But your check inside the EntityResolver is a good solution to still allowing the local XSDs to work while blocking potentially insecure XSD lookups.

Could you update the PR, @kavya-shastri to use that setting instead of the new flag you introduced? And update the error message to be more descriptive about how it only will use locally packaged copies of XSDs unless liquibase.secureParsing is set to false?

[kavya-shastri]

insecure XSD lookups

@nvoxland
Thank you for the feedback! :)
Have amended the commit with the updates.
Please take a look at it and let me know if I need to modify anything else.

[kavya-shastri]

@kataggart
Any updates on this?

[kataggart]

@kavya-shastri It's in our queue for code review; thanks!
FYI @nvoxland

[nvoxland]

Thanks @kavya-shastri, it looks good. I just updated the error message slightly.

The changelog I used for testing looked like this:

<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
                   xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext"
                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext http://example.com/xml/ns/dbchangelog/dbchangelog-ext.xsd
                   http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.6.xsd">
    <changeSet author="example" id="1">
        <createTable tableName="foo">
            <column name="id" type="int"/>
            <ext:test/>
        </createTable>
    </changeSet>
</databaseChangeLog>

with the ext namespace at example.com.

With --secure-parsing=true, I get the error message. With it false, I get an expected "File not found" error since that's not at example.com

[XDelphiGrl]

PR 2384 enabled the Liquibase global configuration for SECURE_PROCESSING by default during XML parsing. This PR extends usage of that global configuration to prevent the entity resolver from falling back to looking on the network for missing XSDs. The default for SECURE_PROCESSING is true, but can be overridden in any of the Liquibase configuration locations.

Global Parameter : --secure-parsing=PARAM
See liquibase --help for details on how use JAVA_OPTS, liquibase.properties file or environment variables to change the setting of liquibase.secure-parsing.

---

I tested the first three cases by setting the dbchangelog.xsd to version 5.0, which does not exist.

	xmlns="http://www.liquibase.org/xml/ns/dbchangelog" 
	xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext" 
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
	xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext 
		http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd 
		http://www.liquibase.org/xml/ns/dbchangelog 
		http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-5.0.xsd">

• Verify validate does not look on the internet for a dbchangelog.xsd version not in the local jar. PASS

Unexpected error running Liquibase: liquibase.parser.core.xml.XSDLookUpException: Unable to resolve xml entity http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-5.0.xsd locally: liquibase.secureParsing is set to 'true' which does not allow remote lookups. Set it to 'false' to allow remote lookups of xsd files.

• Verify update does not look on the internet for a dbchangelog.xsd version not in the local jar. PASS

  • Same error message as validate command.

• Verify update with --secure-parsing false looks on the internet for the missing XSD. PASS

  • liquibase.org returns a 301 for files that do not exist.

[2022-03-01 08:13:15] SEVERE [liquibase.parser] s4s-elt-character: Non-whitespace characters are not allowed in schema elements other than 'xs:appinfo' and 'xs:documentation'. Saw '301 Moved Permanently'.

I tested this case with an XML changelog containing:

<!DOCTYPE person [<!ENTITY ssrf SYSTEM "https://localhost"> ]>

• Verify update fails for XML changelogs with external DTD references. PASS

I tested this case with an XML changelog containing:

<!DOCTYPE a_file[ <!ENTITY ref SYSTEM "file:///C:/Users/erz/dtd-test.txt" > ]>

• Verify update fails for XML changelogs with a DTD reference to a file. PASS

---

Test Environment
Liquibase Core: XSDLookUpUpdate/1662/54e15c, Pro: master/658/91c564/
Passing Functional Tests