Skip to content

Commit

Permalink
feat(security): activate Structurizr restricted mode
Browse files Browse the repository at this point in the history
Enable restricted parsing to disable `!script` and other dangerous methods to be executed during parsing
By default, Kroki will parse Structurizr diagrams in "restricted mode" unless `KROKI_STRUCTURIZR_SAFE_MODE` (or `KROKI_SAFE_MODE`) is set to `unsafe`.
  • Loading branch information
NicolaiSoeborg authored and ggrossetie committed Nov 11, 2024
1 parent 65e9231 commit 4e4e5ee
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 23 deletions.
12 changes: 11 additions & 1 deletion docs/modules/setup/pages/configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@ By default, Kroki is running in `SECURE` mode.
====
Some diagram libraries allow referencing external entities by URL or accessing resources from the filesystem.
For example PlantUML allows the `!import` directive to pull fragments from the filesystem or a remote URL or the standard library.
For example, PlantUML allows the `!import` directive to pull fragments from the filesystem or a remote URL or the standard library.
It is the responsibility of the upstream codebases to ensure that they can be safely used without risk.
Because Kroki does not perform code review of these services, our default setting is to be paranoid and block imports unless known safe.
We encourage anyone running their own Kroki server to review the services security settings and select the security mode appropriate for their use case.
====

=== PlantUML

While running in `SECURE` mode, Kroki will prevent PlantUML from including files using the `!include` or `!includeurl` directive.

If you want to enable this feature, you can set the safe mode using the environment variable `KROKI_SAFE_MODE`:
Expand All @@ -82,6 +84,14 @@ KROKI_PLANTUML_INCLUDE_WHITELIST:: The name of a file that consists of a list of
KROKI_PLANTUML_INCLUDE_WHITELIST_0, KROKI_PLANTUML_INCLUDE_WHITELIST_1, ... KROKI_PLANTUML_INCLUDE_WHITELIST___N__:: One regex to add to the include whitelist per environment variable. Search will stop at the first empty or undefined integer number.
KROKI_PLANTUML_ALLOW_INCLUDE:: Either `false` (default) or `true`. Determines if PlantUML will fetch `!include` directives that reference external URLs. For example, PlantUML allows the !import directive to pull fragments from the filesystem or a remote URL or the standard library.

=== Structurizr

Structurizr's restricted mode is activated unless Kroki is running in `UNSAFE` mode:

> Run this parser in restricted mode (this stops `!include`, `!docs`, `!adrs` from working).

If you want to enable this feature, you can set the safe mode using the global environment variable `KROKI_SAFE_MODE` or the specific environment variable `KROKI_STRUCTURIZR_SAFE_MODE` (i.e., the safe mode will only apply to Structurizr).

== Cross-origin resource sharing (CORS)

By default, the following headers are allowed:
Expand Down
27 changes: 16 additions & 11 deletions server/src/main/java/io/kroki/server/service/Structurizr.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.kroki.server.error.BadRequestException;
import io.kroki.server.error.DecodeException;
import io.kroki.server.format.FileFormat;
import io.kroki.server.security.SafeMode;
import io.vertx.core.AsyncResult;
import io.vertx.core.Handler;
import io.vertx.core.Vertx;
Expand All @@ -23,12 +24,14 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.stream.Collectors;

public class Structurizr implements DiagramService {

private final Vertx vertx;
private final StructurizrPlantUMLExporter structurizrPlantUMLExporter;
private final SafeMode safeMode;
private final SourceDecoder sourceDecoder;
private final PlantumlCommand plantumlCommand;

Expand All @@ -48,6 +51,7 @@ public class Structurizr implements DiagramService {

public Structurizr(Vertx vertx, JsonObject config) {
this.vertx = vertx;
this.safeMode = SafeMode.get(config.getString("KROKI_STRUCTURIZR_SAFE_MODE", config.getString("KROKI_SAFE_MODE", "secure")), SafeMode.SECURE);
this.structurizrPlantUMLExporter = new StructurizrPlantUMLExporter();
this.sourceDecoder = new SourceDecoder() {
@Override
Expand Down Expand Up @@ -75,19 +79,20 @@ public String getVersion() {

@Override
public void convert(String sourceDecoded, String serviceName, FileFormat fileFormat, JsonObject options, Handler<AsyncResult<Buffer>> handler) {
vertx.executeBlocking(future -> {
try {
byte[] data = convert(sourceDecoded, fileFormat, options);
future.complete(data);
} catch (Exception e) {
future.fail(e);
}
}, res -> handler.handle(res.map(o -> Buffer.buffer((byte[]) o))));
vertx.executeBlocking(() -> convert(sourceDecoded, fileFormat, options), res -> handler.handle(res.map(Buffer::buffer)));
}

static byte[] convert(String source, FileFormat fileFormat, PlantumlCommand plantumlCommand, StructurizrPlantUMLExporter structurizrPlantUMLExporter, JsonObject options) throws IOException, InterruptedException {
static byte[] convert(
String source,
FileFormat fileFormat,
PlantumlCommand plantumlCommand,
StructurizrPlantUMLExporter structurizrPlantUMLExporter,
SafeMode safeMode,
JsonObject options
) throws IOException, InterruptedException {
StructurizrDslParser parser = new StructurizrDslParser();
try {
parser.setRestricted(safeMode != SafeMode.UNSAFE);
parser.parse(source);
ViewSet viewSet = parser.getWorkspace().getViews();
Collection<View> views = viewSet.getViews();
Expand Down Expand Up @@ -137,7 +142,7 @@ static byte[] convert(String source, FileFormat fileFormat, PlantumlCommand plan
if (outputOption != null) {
outputOption = outputOption.trim();
}

String diagramPlantUML;
if (outputOption == null || outputOption.equals("diagram")) {
diagramPlantUML = diagram.getDefinition();
Expand All @@ -161,7 +166,7 @@ static byte[] convert(String source, FileFormat fileFormat, PlantumlCommand plan
}

private byte[] convert(String source, FileFormat fileFormat, JsonObject options) throws IOException, InterruptedException {
return convert(source, fileFormat, this.plantumlCommand, this.structurizrPlantUMLExporter, options);
return convert(source, fileFormat, this.plantumlCommand, this.structurizrPlantUMLExporter, this.safeMode, options);
}

private static void applyTheme(ViewSet viewSet, StructurizrTheme theme) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import io.kroki.server.DownloadPlantumlNativeImage;
import io.kroki.server.error.BadRequestException;
import io.kroki.server.format.FileFormat;
import io.kroki.server.security.SafeMode;
import io.vertx.core.Vertx;
import io.vertx.core.json.JsonObject;
import io.vertx.junit5.Checkpoint;
Expand All @@ -25,6 +26,7 @@
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Objects;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -64,7 +66,7 @@ public void should_convert_getting_started_example(String output) throws IOExcep
options.put("output", output);
}

byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), options);
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, options);
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
Expand All @@ -78,7 +80,7 @@ public void should_convert_bigbank_example_container_view() throws IOException,
String expected = read("./bigbank.containers.expected.svg");
JsonObject options = new JsonObject();
options.put("view-key", "Containers");
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), options);
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, options);
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
Expand All @@ -92,7 +94,7 @@ public void should_convert_bigbank_example_systemcontext_view() throws IOExcepti
String expected = read("./bigbank.systemcontext.expected.svg");
JsonObject options = new JsonObject();
options.put("view-key", "SystemContext");
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), options);
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, options);
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
Expand All @@ -107,7 +109,7 @@ public void should_convert_bigbank_example_systemcontext_legend() throws IOExcep
JsonObject options = new JsonObject();
options.put("view-key", "SystemContext");
options.put("output", "legend");
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), options);
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, options);
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
Expand All @@ -119,7 +121,32 @@ public void should_convert_aws_example() throws IOException, InterruptedExceptio
if (Files.isExecutable(Paths.get("/usr/bin/dot"))) {
String source = read("./aws.structurizr");
String expected = read("./aws.expected.svg");
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), new JsonObject());
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, new JsonObject());
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
}
}

@Test
public void should_convert_docs_example() throws IOException, InterruptedException {
if (Files.isExecutable(Paths.get("/usr/bin/dot"))) {
String source = read("docs.structurizr");
String expected = read("./docs.expected.svg");
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, new JsonObject());
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
}
}

@Test
public void should_convert_docs_example_unsafe() throws IOException, InterruptedException {
if (Files.isExecutable(Paths.get("/usr/bin/dot"))) {
String source = read("docs.structurizr");
String expected = read("./docs.expected.svg");
// "docs" is not included when using the PlantUML exporter
byte[] result = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.UNSAFE, new JsonObject());
assertThat(stripComments(new String(result))).isEqualToIgnoringNewLines(expected);
} else {
logger.info("/usr/bin/dot not found, skipping test.");
Expand All @@ -132,7 +159,7 @@ public void should_throw_exception_when_view_does_not_exist() throws IOException
JsonObject options = new JsonObject();
options.put("view-key", "NonExisting");
assertThatThrownBy(() -> {
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), options);
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, options);
})
.isInstanceOf(BadRequestException.class)
.hasMessage("Unable to find view for key: NonExisting.");
Expand All @@ -142,7 +169,7 @@ public void should_throw_exception_when_view_does_not_exist() throws IOException
public void should_throw_exception_when_diagram_is_empty() throws IOException {
String source = read("./no-view.structurizr");
assertThatThrownBy(() -> {
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), new JsonObject());
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, new JsonObject());
})
.isInstanceOf(BadRequestException.class)
.hasMessage("Empty diagram, does not have any view.");
Expand All @@ -152,12 +179,23 @@ public void should_throw_exception_when_diagram_is_empty() throws IOException {
public void should_throw_exception_when_script_directive_used() throws IOException {
String source = read("./script.structurizr");
assertThatThrownBy(() -> {
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), new JsonObject());
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.UNSAFE, new JsonObject());
})
.isInstanceOf(BadRequestException.class)
.hasMessageStartingWith("Unable to parse the Structurizr DSL. Error running inline script, caused by java.lang.RuntimeException: Could not load a scripting engine for extension \"kts\" at line 5");
}


@Test
public void should_throw_exception_when_script_directive_used_safe() throws IOException {
String source = read("./script.structurizr");
assertThatThrownBy(() -> {
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, new JsonObject());
})
.isInstanceOf(BadRequestException.class)
.hasMessageStartingWith("Unable to parse the Structurizr DSL. Unexpected tokens");
}

@Test
public void should_throw_exception_when_unknown_output_specified() throws IOException {
String source = read("./bigbank.structurizr");
Expand All @@ -166,7 +204,7 @@ public void should_throw_exception_when_unknown_output_specified() throws IOExce
options.put("output", "invalid");

assertThatThrownBy(() -> {
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), options);
Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, options);
})
.isInstanceOf(BadRequestException.class)
.hasMessageStartingWith("Unknown output option: invalid");
Expand All @@ -175,7 +213,7 @@ public void should_throw_exception_when_unknown_output_specified() throws IOExce
@Test
public void should_preserve_styles_defined_in_workspace_while_applying_theme() throws IOException, InterruptedException {
String source = read("./workspace-style-with-theme.structurizr");
byte[] convert = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), new JsonObject());
byte[] convert = Structurizr.convert(source, FileFormat.SVG, plantumlCommand, new StructurizrPlantUMLExporter(), SafeMode.SAFE, new JsonObject());
assertThat(new String(convert)).isEqualTo(read("./workspace-style-with-theme.svg"));
}

Expand All @@ -184,7 +222,7 @@ private String stripComments(String xmlContent) {
}

private String read(String name) throws IOException {
try (BufferedReader buffer = new BufferedReader(new InputStreamReader(StructurizrServiceTest.class.getClassLoader().getResourceAsStream(name)))) {
try (BufferedReader buffer = new BufferedReader(new InputStreamReader(Objects.requireNonNull(StructurizrServiceTest.class.getClassLoader().getResourceAsStream(name))))) {
return buffer.lines().collect(Collectors.joining("\n"));
}
}
Expand Down
1 change: 1 addition & 0 deletions server/src/test/resources/docs.expected.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 18 additions & 0 deletions server/src/test/resources/docs.structurizr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
workspace {
model {
user = person "User"
softwareSystem = softwareSystem "Software System" {
!docs src/test/resources/docs
}
user -> softwareSystem "Uses"
}

views {
systemContext softwareSystem {
include *
autolayout
}

theme default
}
}
6 changes: 6 additions & 0 deletions server/src/test/resources/docs/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# System documentation


## Workspace

This is a *workspace*!

0 comments on commit 4e4e5ee

Please sign in to comment.