Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Use 0644 permissions for request.bin in tmp folder. (read everyone, write owner only) #123

Merged
merged 2 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/containerscan/allowedlist.yaml
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
run-testcases.sh
run-testcases.sh
tmp.Dockerfile
results/*-tmp.txt
35 changes: 23 additions & 12 deletions test/suite/setup.sh
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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"
}
]