Skip to content

Commit

Permalink
Bugfix: Use 0644 permissions for request.bin in tmp folder. Read by e…
Browse files Browse the repository at this point in the history
…veryone. Write by owner only. (#123)

* Bugfix: Use 0644 permissions for request.bin in tmp folder.
* Read by everyone. Write by owner only.
* Adapt allowedlist for vulnerabilities which won't affect us.
  • Loading branch information
GollyTicker committed Apr 11, 2023
1 parent 32136d4 commit feecd4b
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 25 deletions.
3 changes: 3 additions & 0 deletions .github/containerscan/allowedlist.yaml
Expand Up @@ -2,6 +2,9 @@ general:
vulnerabilities:
- CVE-2021-3999
- CVE-2022-2097 # only affects 32-bit x86 platforms. We don't offer this architecture in docker however.
- CVE-2023-0464 # only affects us, if user explicitly passes "-policy" to curl. We simply accept that case.
- CVE-2023-0465 # same as above (CVE-2023-0464)
- CVE-2023-0466 # only applies when X509_V_FLAG_POLICY_CHECK is set as a flag. curl cannot be invoked in protocurl this way.
bestPracticeViolations:
- DKL-LI-0001
- CIS-DI-0005
Expand Down
7 changes: 4 additions & 3 deletions TESTS.md
Expand Up @@ -33,12 +33,13 @@ from `test/suite/testcases.json` against the testserver. Each testcase is of the
"<These are split into an array to make it easier to write them in the JSON file.>",
"<All of these array elements will be concatenated with spaces.>"
],
"runAgainWithArg": "<--some-arg>"
"runAgainWithArg": "<--some-arg>",
"afterTestBash": "<bash statements>"
}
```

For each testcase, the `args` array will be concatenated and the concatenated string will be given to `protocurl` (via
docker run) as arguments. `beforeTestBash` and `runAgainWithArg` are optional - and are replaced with `""` if not given.
docker run) as arguments. `beforeTestBash`, `afterTestBash` and `runAgainWithArg` are optional - and are replaced with `""` if not given.
This happens via `test/suite/run-testcases.sh` - which is dynamically created from the JSON. This script contains lines
of the form

Expand All @@ -55,7 +56,7 @@ memory addresses in them are unstable.

If `beforeTestBash` is given, then the bash statements will be executed inside the client docker container before
invoking protocurl with the given arguments. This enables one to explicitly remove curl from the container for testing
purposes.
purposes. The same happens with `afterTestBash`, except they are run after protocurl was invoked.

If `runAgainWithArg` is given, then the test case will be run twice. It will be run once with the given normal arguments
and once more with the given `<--some-arg>` prepended to the arguments of protocurl. This is useful to run the testcases
Expand Down
2 changes: 1 addition & 1 deletion dev/generate-local.Dockerfile.sh
Expand Up @@ -2,4 +2,4 @@
set -e

# Concatenate the dev dockerfile and the final release dockerfile to get the combined one
cat dev/builder.local.Dockerfile release/final.Dockerfile >dev/generated.local.Dockerfile
cat dev/builder.local.Dockerfile <(echo "# ==================") release/final.Dockerfile >dev/generated.local.Dockerfile
2 changes: 1 addition & 1 deletion release/40-generate-Dockerfile.sh
@@ -1,3 +1,3 @@
#!/bin/bash
set -e
cat release/builder.Dockerfile release/final.Dockerfile >release/generated.Dockerfile
cat release/builder.Dockerfile <(echo "# ==================") release/final.Dockerfile >release/generated.Dockerfile
7 changes: 5 additions & 2 deletions src/httpRequest.go
Expand Up @@ -5,7 +5,6 @@ import (
"bytes"
"errors"
"fmt"
"github.com/kballard/go-shellquote"
"io/ioutil"
"net/http"
"net/http/httputil"
Expand All @@ -14,8 +13,12 @@ import (
"path/filepath"
"regexp"
"strings"

"github.com/kballard/go-shellquote"
)

const publicReadPermissions os.FileMode = 0644

func invokeInternalHttpRequest(requestBinary []byte) ([]byte, string) {
if CurrentConfig.Verbose {
fmt.Println("Invoking internal http request.")
Expand Down Expand Up @@ -55,7 +58,7 @@ func invokeCurlRequest(requestBinary []byte, curlPath string) ([]byte, string) {
defer func() { _ = os.RemoveAll(tmpDir) }()

requestBinaryFile := filepath.Join(tmpDir, "request.bin")
err = ioutil.WriteFile(requestBinaryFile, requestBinary, 0)
err = ioutil.WriteFile(requestBinaryFile, requestBinary, publicReadPermissions)
PanicOnError(err)

responseBinaryFile := filepath.Join(tmpDir, "response.bin")
Expand Down
9 changes: 9 additions & 0 deletions test/results/tmp-file-permissions-readable-expected.txt
@@ -0,0 +1,9 @@
######### STDOUT #########
=========================== Request Text =========================== >>>

-rw-r--r--
=========================== Response Text =========================== <<<
isHappyDay: true
formattedDate: "Thu, 01 Jan 1970 00:00:00 GMT"
######### STDERR #########
######### EXIT 0 #########
4 changes: 3 additions & 1 deletion test/suite/.gitignore
@@ -1 +1,3 @@
run-testcases.sh
run-testcases.sh
tmp.Dockerfile
results/*-tmp.txt
35 changes: 23 additions & 12 deletions test/suite/setup.sh
Expand Up @@ -30,19 +30,30 @@ buildProtocurl() {
docker build --target final -t $PROTOCURL_IMAGE_ORIGINAL $BUILD_ARGS . &&
echo "Done."
fi

echo "Building test image variant of protocurl including additonal executables ..."
touch tmp.Dockerfile
grep "^FROM " release/builder.Dockerfile >> tmp.Dockerfile
echo "FROM $PROTOCURL_IMAGE_ORIGINAL as final" >> tmp.Dockerfile
echo "COPY --from=builder /bin/* /bin/" >> tmp.Dockerfile
grep "^ENTRYPOINT " release/final.Dockerfile >> tmp.Dockerfile
remove-leading-spaces-inplace tmp.Dockerfile

cat tmp.Dockerfile | docker build --target final -t $PROTOCURL_IMAGE -f - .
TMP_DOCKERFILE="test/suite/tmp.Dockerfile"
echo "" >$TMP_DOCKERFILE
grep "^FROM " release/builder.Dockerfile >>$TMP_DOCKERFILE
# add inotify to binaries to test tmp-file permissions. also add pkill for cleanup
echo "RUN apt-get update && apt-get install -y inotify-tools procps" >>$TMP_DOCKERFILE
echo "# =============" >>$TMP_DOCKERFILE
echo "FROM $PROTOCURL_IMAGE_ORIGINAL as final" >>$TMP_DOCKERFILE
echo "COPY --from=builder /bin/* /bin/" >>$TMP_DOCKERFILE
echo "COPY --from=builder /usr/bin/* /usr/bin/" >>$TMP_DOCKERFILE
echo "
COPY --from=builder /lib/*-linux-gnu /lib/x86_64-linux-gnu/
COPY --from=builder /lib/*-linux-gnu /lib/x86_32-linux-gnu/
COPY --from=builder /lib/*-linux-gnu /lib/aarch_64-linux-gnu/
COPY --from=builder /usr/lib/*-linux-gnu /usr/lib/x86_64-linux-gnu/
COPY --from=builder /usr/lib/*-linux-gnu /usr/lib/x86_32-linux-gnu/
COPY --from=builder /usr/lib/*-linux-gnu /usr/lib/aarch_64-linux-gnu/
" >>$TMP_DOCKERFILE
grep "^ENTRYPOINT " release/final.Dockerfile >>$TMP_DOCKERFILE
remove-leading-spaces-inplace $TMP_DOCKERFILE

cat $TMP_DOCKERFILE | docker build --target final -t $PROTOCURL_IMAGE -f - .
echo "Done."

rm -f tmp.Dockerfile
}
export -f buildProtocurl

Expand Down Expand Up @@ -129,7 +140,7 @@ normaliseOutput() {
sed -i 's/, {/,{/g' "$1"
sed -i 's/, {/,{/g' "$1"

# remove lines with random tamporary folder names
# remove lines with random temporary folder names
sed -i "s|/tmp/protocurl-temp.*|<tmp>|g" "$1"

customNormaliseOutput "$1"
Expand Down
16 changes: 11 additions & 5 deletions test/suite/test.sh
Expand Up @@ -22,12 +22,13 @@ testSingleRequest() {
ARGS="$2"
BEFORE_TEST_BASH="$3"
RUN_AGAIN_WITH_ARG="$4"
AFTER_TEST_BASH="$5"

if [[ "$RUN_AGAIN_WITH_ARG" != "" ]]; then
NEW_ARGS="$RUN_AGAIN_WITH_ARG $ARGS"
NEW_FILENAME="${FILENAME}-${RUN_AGAIN_WITH_ARG#--}"
testSingleRequest "$FILENAME" "$ARGS" "$BEFORE_TEST_BASH" ""
testSingleRequest "$NEW_FILENAME" "$NEW_ARGS" "$BEFORE_TEST_BASH" ""
testSingleRequest "$FILENAME" "$ARGS" "$BEFORE_TEST_BASH" "" "$AFTER_TEST_BASH"
testSingleRequest "$NEW_FILENAME" "$NEW_ARGS" "$BEFORE_TEST_BASH" "" "$AFTER_TEST_BASH"
else

EXPECTED="test/results/$FILENAME-expected.txt"
Expand All @@ -41,12 +42,17 @@ testSingleRequest() {

set +e

if [[ "$BEFORE_TEST_BASH" == "" ]]; then
if [[ "$BEFORE_TEST_BASH" == "" && "$AFTER_TEST_BASH" == "" ]]; then
eval "$RUN_CLIENT --name $FILENAME $PROTOCURL_IMAGE $ARGS" 2>"$OUT_ERR" >>"$OUT"
EXIT_CODE="$?"
else
if [[ "$BEFORE_TEST_BASH" == "" ]]; then BEFORE_TEST_BASH="true"; fi
if [[ "$AFTER_TEST_BASH" == "" ]]; then AFTER_TEST_BASH="true"; fi
ARGS="$(echo "$ARGS" | sed 's/"/\\"/g')" # escape before usage inside quoted context
eval "$RUN_CLIENT --entrypoint bash --name $FILENAME $PROTOCURL_IMAGE -c \"$BEFORE_TEST_BASH && ./bin/protocurl $ARGS\"" 2>"$OUT_ERR" >>"$OUT"
eval "$RUN_CLIENT --entrypoint bash \
--name $FILENAME $PROTOCURL_IMAGE \
-c \"$BEFORE_TEST_BASH && ./bin/protocurl $ARGS && $AFTER_TEST_BASH\"" \
2>"$OUT_ERR" >>"$OUT"
EXIT_CODE="$?"
fi
echo "######### STDERR #########" >>"$OUT"
Expand Down Expand Up @@ -79,7 +85,7 @@ runAllTests() {

# Convert each element in the JSON to the corresponding call of the testSingleRequest function.
# Simply look at the produced run-testcases.sh file to see what it looks like.
CONVERT_TESTCASE_TO_SINGLE_TEST_INVOCATION=".[] | \"testSingleRequest \(.filename|@sh) \(.args|join(\" \")|@sh) \(.beforeTestBash // \"\"|@sh) \(.runAgainWithArg // \"\"|@sh)\""
CONVERT_TESTCASE_TO_SINGLE_TEST_INVOCATION=".[] | \"testSingleRequest \(.filename|@sh) \(.args|join(\" \")|@sh) \(.beforeTestBash // \"\"|@sh) \(.runAgainWithArg // \"\"|@sh) \(.afterTestBash // \"\"|@sh)\""
cat test/suite/testcases.json | jq -r "$CONVERT_TESTCASE_TO_SINGLE_TEST_INVOCATION" >./test/suite/run-testcases.sh

export -f testSingleRequest
Expand Down
9 changes: 9 additions & 0 deletions test/suite/testcases.json
Expand Up @@ -523,5 +523,14 @@
"-d \"includeReason: true, date: { seconds: 1648044939, nanos: 152000000 }\" -v",
"-n --no-curl"
]
},
{
"filename": "tmp-file-permissions-readable",
"beforeTestBash": "((inotifywait -q -m -e create /tmp | head -n 2 | tail -n 1 | while read DIRECTORY EVENT FILE; do ls -lah /tmp/protocurl* | grep request.bin | awk '{ print \\$1; }'; done) &) && sleep 5s",
"args": [
"-i ..HappyDayRequest -o ..HappyDayResponse -u http://localhost:8080/happy-day/verify",
"-d \"\""
],
"afterTestBash": "pkill -f inotifywait"
}
]

0 comments on commit feecd4b

Please sign in to comment.