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 1 commit
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
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
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 the condition is executed and fails. Make sure you understand the check precedence; this can be trickey, 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.
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.
Loading