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

Add hints for failing conditions #172

Merged
merged 7 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,12 @@ You can build release files (e.g., `aeacus-linux.zip`). These will have auto-ran

## Documentation

All checks (with examples and notes) [are documented here](docs/checks.md).
All check condition types (with examples and notes) [are documented here](docs/checks.md).

Other documentation:
- [Non-Check Scoring Configuration](docs/config.md)
- [Condition Precedence](docs/conditions.md)
- [Adding Hints](docs/hints.md)
- [Crypto](docs/crypto.md)
- [Security Model](docs/security.md)
- [Remote Reporting](docs/remote.md)
Expand Down
17 changes: 13 additions & 4 deletions checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,20 @@ import (
// the conditions for a check, and its message and points (autogenerated or
// otherwise).
type check struct {
Message string
Points int
Message string
Mobmaker55 marked this conversation as resolved.
Show resolved Hide resolved
Hint string
Points int

Fail []cond
Pass []cond
PassOverride []cond
}

// cond, or condition, is the parameters for a given test within a check.
type cond struct {
Type string
Hint string
Type string

Path string
Cmd string
User string
Expand Down Expand Up @@ -55,6 +59,11 @@ func (c cond) requireArgs(args ...interface{}) {
continue
}

// Ignore hint fields, they only show up in the scoring report
if vType.Field(i).Name == "Hint" {
continue
}

required := false
for _, a := range args {
if vType.Field(i).Name == a {
Expand All @@ -68,7 +77,7 @@ func (c cond) requireArgs(args ...interface{}) {
fail(c.Type+":", "missing required argument '"+vType.Field(i).Name+"'")
}
} else if v.Field(i).String() != "" {
warn(c.Type+":", "specifying unnecessary argument '"+vType.Field(i).Name+"'")
warn(c.Type+":", "specifying unused argument '"+vType.Field(i).Name+"'")
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions checks_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,9 @@ func (c cond) PermissionIs() (bool, error) {
func (c cond) ProgramInstalled() (bool, error) {
c.requireArgs("Name")
result, err := cond{
Cmd: "dpkg -s " + c.Name,
}.Command()
Cmd: "dpkg -s " + c.Name,
Value: " install",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this change do?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this question also applies to line 215

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea lol I think that check was broken. After removing something with apt, dpkg just shows the status as deinstall. Does not return nonzero value for -s

}.CommandContains()

// If dpkg fails, use rpm
if err != nil {
Expand Down
9 changes: 7 additions & 2 deletions configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ func parseConfig(configContent string) {

// Print warnings for impossible checks and undefined check types.
for i, check := range conf.Check {
allConditions := append(append(append([]cond{}, check.Pass[:]...), check.Fail[:]...), check.PassOverride[:]...)
if len(allConditions) == 0 {
if len(check.Pass) == 0 && len(check.PassOverride) == 0 {
warn("Check " + fmt.Sprintf("%d", i+1) + " does not define any possible ways to pass!")
}
allConditions := append(append(append([]cond{}, check.Pass[:]...), check.Fail[:]...), check.PassOverride[:]...)
for j, cond := range allConditions {
if cond.Type == "" {
warn("Check " + fmt.Sprintf("%d condition %d", i+1, j+1) + " does not have a check type!")
Expand Down Expand Up @@ -169,6 +169,11 @@ func obfuscateConfig() {
if err := obfuscateData(&conf.Check[i].Message); err != nil {
fail(err.Error())
}
if conf.Check[i].Hint != "" {
if err := obfuscateData(&conf.Check[i].Hint); err != nil {
fail(err.Error())
}
}
for j := range check.Pass {
if err := obfuscateCond(&conf.Check[i].Pass[j]); err != nil {
fail(err.Error())
Expand Down
45 changes: 45 additions & 0 deletions docs/conditions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Check conditions and precedence

Using multiple conditions for a check can be confusing at first, but can greatly improve the quality of your images by accounting for edge cases and abuse.

Given no conditions, a check does not pass.

If any **Fail** conditions succeed, the check does not pass.

**PassOverride** conditions act as a logical OR. This means that any can succeed for the check to pass.

**Pass** conditions act as a logical AND with other pass conditions. This means they must ALL be true for a check to pass.

If the outcome of a check is decided, aeacus will NOT execute the remaining conditions (it will "short circuit"). For example, if a PassOverride succeeds, any Pass conditions are NOT executed.

So, it's like this: `check_passes = (NOT fails) AND (passoverride OR (AND of all pass checks))`.

For example:

```
[[check]]

# Ensure the scheduled task service is running AND
[[check.fail]]
type = 'ServiceUpNot'
name = 'Schedule'

# Pass if the user runnning those tasks is deleted
[[check.passoverride]]
type = 'UserExistsNot'
name = 'CleanupBot'

# OR pass if both scheduled tasks are deleted
[[check.pass]]
type = 'ScheduledTaskExistsNot'
name = 'Disk Cleanup'
[[check.pass]]
type = 'ScheduledTaskExistsNot'
name = 'Disk Cleanup Backup'

```

The evaluation of checks goes like this:
1. Check if any Fail are true. If any Fail checks succeed, then we're done, the check doesn't pass.
2. Check if any PassOverride conditions pass. If they do, we're done, the check passes.
3. Check status of all Pass conditions. If they all succeed, the check passes, otherwise it fails.
41 changes: 0 additions & 41 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,45 +86,4 @@ points = "-5"
name = '/lib/systemd/system/sshd.service'
```

## Combining check conditions

Using multiple conditions for a check can be confusing at first, but can greatly improve the quality of your images by accounting for edge cases and abuse.

Given no conditions, a check does not pass.

**Pass** conditions act as a logical AND with other pass conditions. This means they must ALL be true for a check to pass.

**PassOverride** conditions act as a logical OR. This means that any can succeed for the check to pass.

If any **Fail** conditions succeed, the check does not pass.

So, it's like: ``((all pass checks) OR passoverride) AND fails``.

For example:

```
[[check]]

# Pass only if both scheduled tasks are deleted
[[check.pass]]
type = 'ScheduledTaskExistsNot'
name = 'Disk Cleanup'
[[check.pass]]
type = 'ScheduledTaskExistsNot'
name = 'Disk Cleanup Backup'

# OR if the user runnning those tasks is deleted
[[check.passoverride]]
type = 'UserExistsNot'
name = 'CleanupBot'

# AND the scheduled task service is running
[[check.fail]]
type = 'ServiceUpNot'
name = 'Schedule'
```

The evaluation of checks goes like this:
1. Check if any Fail are true. If any Fail checks succeed, then we're done, the check doesn't pass.
2. Check if any PassOverride conditions pass. If they do, we're done, the check passes.
3. Check status of all Pass conditions. If they all succeed, the check passes, otherwise it fails.
4 changes: 1 addition & 3 deletions docs/crypto.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
# aeacus

## Adding Crypto
# Adding Crypto

The public releases of `aeacus` ship with weak crypto (cryptographic security), which means that the encryption and/or encoding of scoring data files is not very "secure".

Expand Down
45 changes: 45 additions & 0 deletions docs/hints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Hints

Hints let you provide information on failing checks.

![Hint Example](../misc/gh/ReadMe.png)

Hints are a way to help make images more approachable.

You can add a conditional hint or a check-wide hint. A conditional hint is printed when that specific condition is executed and fails. Make sure you understand the check precedence; this can be tricky, as sometimes your check is NOT executed ([read about conditions](conditions.md)).

Example conditional hint:
```
[[check]]
points = 5

[[check.pass]]
type = "ProgramInstalledNot"
name = "john"

[[check.pass]]
# This hint will NOT print unless the condition above succeeds.
# Pass conditions are logically AND-- they all need to succeed.
# If one fails, there's no reason to execute the other ones.
hint = "Removing just the binary is insufficient; use a package manager to remove all of a tool's files."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least by my logic, if a Program is NOT installed (so its no longer in the package manager DB saying it is or is not installed), then "using a package manager to remove the tool's files" would be useless since the package manager has no idea it exists -- if my understanding is right, shouldn't the hint be more like "The binary and its files need removed!"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example here is a "common" issue where people remove a package but don't purge or autoremove, so there's a hint for that (eg remove john, john-data still exists). I suppose the hint could be "Ensure that you use a package manager to remove ALL of a tool's files"

type = "PathExistsNot"
path = "/usr/share/john"
```

Check-wide hints are at the top level and always displayed if a check fails. Example check-wide hint:

```
[[check]]
hint = "Are there any 'hacking' tools installed?"
points = 5

[[check.pass]]
type = "ProgramInstalledNot"
name = "john"

[[check.pass]]
type = "PathExistsNot"
path = "/usr/share/john"
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you would end up with two hints to the same check? Which can be confusing when its based on points so
(Points: 5) (hint)
(Points: 5) (another hint on the same thing, but it could be interpreted as solving both hints give you 10 points)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When points are for the same item they're grouped in a list so I don't think that would be confusing. Maybe vice versa if there are two hints for 5 points someone could think they're the same underlying check but I think that's unlikely

You can combine check-wide and conditional hints. If the check fails, the check-wide hint is ALWAYS displayed, in addition to any conditional hints triggered.
2 changes: 1 addition & 1 deletion docs/regex.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Regular Expressions

Many of the checks in `aeacus` require regular expression (regex) strings as input. This may seem inconvenient if you want to score something simple, but we think it significantly increases the overall quality of checks. Each regex is applied to each line of the input file, so currently, no multi-line regexes are currently possible.
Many of the checks in `aeacus` use regular expression (regex) strings as input. This may seem inconvenient if you want to score something simple, but we think it significantly increases the overall quality of checks. Each regex is applied to each line of the input file, so currently, no multi-line regexes are currently possible.

> We're using the Golang Regular Expression package ([documentation here](https://godocs.io/regexp)). It uses RE2 syntax, which is also generally the same as Perl, Python, and other languages.

Expand Down
Binary file added misc/gh/hint.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 5 additions & 5 deletions output.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func debug(p ...interface{}) {
} else {
printStr = printer(color.FgMagenta, "DBUG", toPrint)
}
fmt.Printf(printStr)
fmt.Print(printStr)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it was Printf() before, this is just a sanity check, but it is intentional that it is Print() and not Println() yeah?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry had this change lying around for a while, it is supposed to be Print. Printf was kind of like a format string vulnerability lol, not that those exist in go. The newline is added in toPrint via Sprintln.

}
}

Expand All @@ -104,24 +104,24 @@ func info(p ...interface{}) {
} else {
printStr = printer(color.FgCyan, "INFO", toPrint)
}
fmt.Printf(printStr)
fmt.Print(printStr)
}
}

func blue(head string, p ...interface{}) {
toPrint := fmt.Sprintln(p...)
printStr := printer(color.FgCyan, head, toPrint)
fmt.Printf(printStr)
fmt.Print(printStr)
}

func red(head string, p ...interface{}) {
toPrint := fmt.Sprintln(p...)
fmt.Printf(printer(color.FgRed, head, toPrint))
fmt.Print(printer(color.FgRed, head, toPrint))
}

func green(head string, p ...interface{}) {
toPrint := fmt.Sprintln(p...)
fmt.Printf(printer(color.FgGreen, head, toPrint))
fmt.Print(printer(color.FgGreen, head, toPrint))
}

func printer(colorChosen color.Attribute, messageType, toPrint string) string {
Expand Down
Loading
Loading