From 29d0727624f4f111ec3c15a23b12384b558a68ce Mon Sep 17 00:00:00 2001 From: Arshan Dabirsiaghi Date: Tue, 25 Jun 2024 10:19:43 -0400 Subject: [PATCH 1/6] add jndi helper tools --- .../java/io/github/pixee/security/JNDI.java | 85 +++++++++++++++++++ .../io/github/pixee/security/UrlProtocol.java | 11 ++- .../io/github/pixee/security/JNDITest.java | 65 ++++++++++++++ 3 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 src/main/java/io/github/pixee/security/JNDI.java create mode 100644 src/test/java/io/github/pixee/security/JNDITest.java diff --git a/src/main/java/io/github/pixee/security/JNDI.java b/src/main/java/io/github/pixee/security/JNDI.java new file mode 100644 index 0000000..41db1cd --- /dev/null +++ b/src/main/java/io/github/pixee/security/JNDI.java @@ -0,0 +1,85 @@ +package io.github.pixee.security; + +import javax.naming.Context; +import javax.naming.NamingException; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +/** Offers utilities to defend against JNDI attacks by controlling allowed resources. */ +public final class JNDI { + + private JNDI() {} + + /** + * Looks up a resource in the context, only allowing resources non-URL-based resources and "java:" resources. + */ + public static LimitedContext limitedContext(final Context context) { + return new ProtocolLimitedContext(context, J8ApiBridge.setOf(UrlProtocol.JAVA)); + } + + /** + * Looks up a resource in the context, only allowing resources from the specified protocols. + */ + public static LimitedContext limitedContextByProtocol(final Context context, final Set allowedProtocols) { + return new ProtocolLimitedContext(context, allowedProtocols); + } + + /** + * Looks up a resource in the context, only allowing resources with the given names. + */ + public static LimitedContext limitedContextByResourceName(final Context context, final Set allowedResourceNames) { + return new NameLimitedContext(context, allowedResourceNames); + } + + /** A lookalike method for {@link Context} that allows sandboxing resolution. */ + public interface LimitedContext { + /** + * Looks up a resource in the context, but only allows resources that are in the allowed set. + * + * @param resource the resource to look up + * @return the object bound to the resource + * @throws NamingException if the resource is not allowed or if the lookup fails as per {@link Context#lookup(String)} + */ + Object lookup(final String resource) throws NamingException; + } + + /** A context which limits protocols. */ + private static class ProtocolLimitedContext implements LimitedContext { + private final Context context; + private final Set allowedProtocols; + + private ProtocolLimitedContext(final Context context, final Set allowedProtocols) { + this.context = Objects.requireNonNull(context); + this.allowedProtocols = Objects.requireNonNull(allowedProtocols); + } + + @Override + public Object lookup(final String resource) throws NamingException { + Set allowedProtocolPrefixes = allowedProtocols.stream().map(UrlProtocol::getKey).map(p -> p + ":").collect(Collectors.toSet()); + String canonicalResource = resource.toLowerCase().trim(); + if (allowedProtocolPrefixes.stream().anyMatch(canonicalResource::startsWith)) { + return context.lookup(resource); + } + throw new SecurityException("Unexpected JNDI resource protocol: " + resource); + } + } + + /** A context which only allows pre-defined resource names. */ + private static class NameLimitedContext implements LimitedContext { + private final Context context; + private final Set allowedResourceNames; + + private NameLimitedContext(final Context context, final Set allowedResourceNames) { + this.context = Objects.requireNonNull(context); + this.allowedResourceNames = Objects.requireNonNull(allowedResourceNames); + } + @Override + public Object lookup(final String resource) throws NamingException { + if(allowedResourceNames.contains(resource)) { + return context.lookup(resource); + } + throw new SecurityException("Unexpected JNDI resource name: " + resource); + } + } +} \ No newline at end of file diff --git a/src/main/java/io/github/pixee/security/UrlProtocol.java b/src/main/java/io/github/pixee/security/UrlProtocol.java index 9b376b2..f537caa 100644 --- a/src/main/java/io/github/pixee/security/UrlProtocol.java +++ b/src/main/java/io/github/pixee/security/UrlProtocol.java @@ -49,7 +49,16 @@ public enum UrlProtocol { TELNET("telnet"), /** Classpath */ - CLASSPATH("classpath"); + CLASSPATH("classpath"), + + /** LDAP */ + LDAP("ldap"), + + /** Java */ + JAVA("java"), + + /** RMI */ + RMI("rmi"); private final String key; diff --git a/src/test/java/io/github/pixee/security/JNDITest.java b/src/test/java/io/github/pixee/security/JNDITest.java new file mode 100644 index 0000000..1b569d0 --- /dev/null +++ b/src/test/java/io/github/pixee/security/JNDITest.java @@ -0,0 +1,65 @@ +package io.github.pixee.security; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.naming.Context; +import javax.naming.NamingException; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.*; + +final class JNDITest { + + private Context context; + private final Object NAMED_OBJECT = new Object(); + private final Object JAVA_OBJECT = new Object(); + private final Object LDAP_OBJECT = new Object(); + private final Object RMI_OBJECT = new Object(); + + @BeforeEach + void setup() throws NamingException { + context = mock(Context.class); + when(context.lookup("simple_name")).thenReturn(NAMED_OBJECT); + when(context.lookup("java:comp/env")).thenReturn(JAVA_OBJECT); + when(context.lookup("ldap://localhost:1389/ou=system")).thenReturn(LDAP_OBJECT); + when(context.lookup("rmi://localhost:1099/evil")).thenReturn(RMI_OBJECT); + } + + @Test + void it_limits_resources_by_name() throws NamingException { + JNDI.LimitedContext limitedContext = JNDI.limitedContextByResourceName(context, J8ApiBridge.setOf("simple_name")); + assertThat(limitedContext.lookup("simple_name"), is(NAMED_OBJECT)); + assertThrows(SecurityException.class, () -> limitedContext.lookup("anything_else")); + verify(context, times(1)).lookup(anyString()); + } + + @Test + void it_limits_resources_by_protocol() throws NamingException { + JNDI.LimitedContext onlyJavaContext = JNDI.limitedContextByProtocol(context, J8ApiBridge.setOf(UrlProtocol.JAVA)); + assertThat(onlyJavaContext.lookup("java:comp/env"), is(JAVA_OBJECT)); + assertThrows(SecurityException.class, () -> onlyJavaContext.lookup("ldap://localhost:1389/ou=system")); + assertThrows(SecurityException.class, () -> onlyJavaContext.lookup("rmi://localhost:1099/evil")); + + JNDI.LimitedContext onlyLdapContext = JNDI.limitedContextByProtocol(context, J8ApiBridge.setOf(UrlProtocol.LDAP)); + assertThat(onlyLdapContext.lookup("ldap://localhost:1389/ou=system"), is(LDAP_OBJECT)); + assertThrows(SecurityException.class, () -> onlyLdapContext.lookup("java:comp/env")); + assertThrows(SecurityException.class, () -> onlyLdapContext.lookup("rmi://localhost:1099/evil")); + + JNDI.LimitedContext onlyLdapAndJavaContext = JNDI.limitedContextByProtocol(context, J8ApiBridge.setOf(UrlProtocol.JAVA, UrlProtocol.LDAP)); + assertThat(onlyLdapAndJavaContext.lookup("ldap://localhost:1389/ou=system"), is(LDAP_OBJECT)); + assertThat(onlyLdapAndJavaContext.lookup("java:comp/env"), is(JAVA_OBJECT)); + assertThrows(SecurityException.class, () -> onlyLdapAndJavaContext.lookup("rmi://localhost:1099/evil")); + } + + @Test + void default_limits_rmi_and_ldap() throws NamingException { + JNDI.LimitedContext defaultLimitedContext = JNDI.limitedContext(context); + assertThat(defaultLimitedContext.lookup("java:comp/env"), is(JAVA_OBJECT)); + assertThrows(SecurityException.class, () -> defaultLimitedContext.lookup("rmi://localhost:1099/evil")); + assertThrows(SecurityException.class, () -> defaultLimitedContext.lookup("ldap://localhost:1389/ou=system")); + } + +} \ No newline at end of file From 5d72fe7350b86d986ae3e3b03c2ad9387a5c366f Mon Sep 17 00:00:00 2001 From: Arshan Dabirsiaghi Date: Tue, 25 Jun 2024 10:33:25 -0400 Subject: [PATCH 2/6] add module import --- src/main/java/module-info.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 8384d9e..7d4ec45 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -4,6 +4,7 @@ requires org.apache.commons.io; requires java.xml; + requires java.naming; requires java.desktop; requires java.base; } \ No newline at end of file From 8aea291f9400556ca5db3f6e36648a1512a980fb Mon Sep 17 00:00:00 2001 From: Jacoco Coverage Update Action Date: Tue, 25 Jun 2024 14:35:20 +0000 Subject: [PATCH 3/6] Autogenerated JaCoCo coverage badge --- .github/badges/branches.svg | 2 +- .github/badges/jacoco.svg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/badges/branches.svg b/.github/badges/branches.svg index ea0fee7..eb5fe7b 100644 --- a/.github/badges/branches.svg +++ b/.github/badges/branches.svg @@ -1 +1 @@ -branches60.4% \ No newline at end of file +branches60.9% \ No newline at end of file diff --git a/.github/badges/jacoco.svg b/.github/badges/jacoco.svg index 856b235..65ea9eb 100644 --- a/.github/badges/jacoco.svg +++ b/.github/badges/jacoco.svg @@ -1 +1 @@ -coverage80.3% \ No newline at end of file +coverage81.2% \ No newline at end of file From 1eb98ad894b7c5a65a420702044d6170f3054c23 Mon Sep 17 00:00:00 2001 From: Arshan Dabirsiaghi Date: Tue, 25 Jun 2024 10:52:14 -0400 Subject: [PATCH 4/6] still support simple named resources when protocol restrictions are present --- src/main/java/io/github/pixee/security/JNDI.java | 10 +++++++--- src/test/java/io/github/pixee/security/JNDITest.java | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/github/pixee/security/JNDI.java b/src/main/java/io/github/pixee/security/JNDI.java index 41db1cd..a17f360 100644 --- a/src/main/java/io/github/pixee/security/JNDI.java +++ b/src/main/java/io/github/pixee/security/JNDI.java @@ -58,10 +58,14 @@ private ProtocolLimitedContext(final Context context, final Set all public Object lookup(final String resource) throws NamingException { Set allowedProtocolPrefixes = allowedProtocols.stream().map(UrlProtocol::getKey).map(p -> p + ":").collect(Collectors.toSet()); String canonicalResource = resource.toLowerCase().trim(); - if (allowedProtocolPrefixes.stream().anyMatch(canonicalResource::startsWith)) { - return context.lookup(resource); + if(canonicalResource.contains(":")) { + if (allowedProtocolPrefixes.stream().anyMatch(canonicalResource::startsWith)) { + return context.lookup(resource); + } else { + throw new SecurityException("Unexpected JNDI resource protocol: " + resource); + } } - throw new SecurityException("Unexpected JNDI resource protocol: " + resource); + return context.lookup(resource); } } diff --git a/src/test/java/io/github/pixee/security/JNDITest.java b/src/test/java/io/github/pixee/security/JNDITest.java index 1b569d0..e0c66b5 100644 --- a/src/test/java/io/github/pixee/security/JNDITest.java +++ b/src/test/java/io/github/pixee/security/JNDITest.java @@ -40,6 +40,9 @@ void it_limits_resources_by_name() throws NamingException { void it_limits_resources_by_protocol() throws NamingException { JNDI.LimitedContext onlyJavaContext = JNDI.limitedContextByProtocol(context, J8ApiBridge.setOf(UrlProtocol.JAVA)); assertThat(onlyJavaContext.lookup("java:comp/env"), is(JAVA_OBJECT)); + + // confirm protocols protections dont restrict simple name lookups + assertThat(onlyJavaContext.lookup("simple_name"), is(NAMED_OBJECT)); assertThrows(SecurityException.class, () -> onlyJavaContext.lookup("ldap://localhost:1389/ou=system")); assertThrows(SecurityException.class, () -> onlyJavaContext.lookup("rmi://localhost:1099/evil")); @@ -58,6 +61,9 @@ void it_limits_resources_by_protocol() throws NamingException { void default_limits_rmi_and_ldap() throws NamingException { JNDI.LimitedContext defaultLimitedContext = JNDI.limitedContext(context); assertThat(defaultLimitedContext.lookup("java:comp/env"), is(JAVA_OBJECT)); + + // confirm simple name lookups still work + assertThat(defaultLimitedContext.lookup("simple_name"), is(NAMED_OBJECT)); assertThrows(SecurityException.class, () -> defaultLimitedContext.lookup("rmi://localhost:1099/evil")); assertThrows(SecurityException.class, () -> defaultLimitedContext.lookup("ldap://localhost:1389/ou=system")); } From f9a37d7b56e68e2c9238a93c5fec0ead3c62f46d Mon Sep 17 00:00:00 2001 From: Jacoco Coverage Update Action Date: Tue, 25 Jun 2024 14:54:44 +0000 Subject: [PATCH 5/6] Autogenerated JaCoCo coverage badge --- .github/badges/branches.svg | 2 +- .github/badges/jacoco.svg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/badges/branches.svg b/.github/badges/branches.svg index eb5fe7b..47d6a86 100644 --- a/.github/badges/branches.svg +++ b/.github/badges/branches.svg @@ -1 +1 @@ -branches60.9% \ No newline at end of file +branches61.1% \ No newline at end of file diff --git a/.github/badges/jacoco.svg b/.github/badges/jacoco.svg index 65ea9eb..e6c7d3c 100644 --- a/.github/badges/jacoco.svg +++ b/.github/badges/jacoco.svg @@ -1 +1 @@ -coverage81.2% \ No newline at end of file +coverage81.3% \ No newline at end of file From f523a091bc11f30df046d200354300428ab2c2a0 Mon Sep 17 00:00:00 2001 From: Arshan Dabirsiaghi Date: Tue, 25 Jun 2024 11:00:16 -0400 Subject: [PATCH 6/6] bump version --- README.md | 4 ++-- build.gradle.kts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8691bc1..0808c8a 100644 --- a/README.md +++ b/README.md @@ -35,12 +35,12 @@ In Maven: io.github.pixee java-security-toolkit - 1.1.3 + 1.2.0 ``` In Gradle: ```kotlin -implementation("io.github.pixee:java-security-toolkit:1.1.3") +implementation("io.github.pixee:java-security-toolkit:1.2.0") ``` ## Contributing diff --git a/build.gradle.kts b/build.gradle.kts index 175ed44..ef87528 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -91,7 +91,7 @@ tasks.named(java11SourceSet.jarTaskName) { } group = "io.github.pixee" -version = "1.1.3" +version = "1.2.0" description = "java-security-toolkit"