fix: path traversal in keystore v2 DirectoryBackend.osPath()

[zenmpi]

Summary

The path traversal check in DirectoryBackend.osPath() (keystore/v2/keystore/filesystem/backend/filesystem.go) is ineffective and does not prevent key path resolution outside the keystore root directory.

Vulnerability Details

The function constructs an OS path from a logical key path and validates it:

fullPath := filepath.Join(b.root, pathSeparators.Replace(path))
if fullPath != filepath.Clean(fullPath) {
    return "", api.ErrInvalidPath
}

According to the Go documentation, filepath.Join already returns a Clean-ed result:

Join joins any number of path elements into a single path [...] The result is Cleaned.

Therefore fullPath != filepath.Clean(fullPath) is always false, and the check never fires — even when the resolved path escapes the root directory.

Example

root:     /var/lib/acra/keys
input:    client/../../other-instance/keys/client/victim/storage.keyring
resolved: /var/lib/other-instance/keys/client/victim/storage.keyring  (outside root!)
check:    resolved != filepath.Clean(resolved) → false → allowed through

Affected Path

The keystore v1 (keystore/filesystem/) calls ValidateID() which restricts clientID to [a-zA-Z0-9_\- ], effectively blocking .. in paths. The v2 keystore does not call ValidateID(), relying solely on the osPath() check which is ineffective.

Impact

When clientID is taken directly from the gRPC request (acratranslator_client_id_from_connection_enable=false, the default), a crafted clientID containing .. sequences can cause the keystore to read keyring files from outside the root directory. The file name suffix (e.g. storage.keyring) is appended by the calling code and cannot be controlled, so this is limited to reading *.keyring files from arbitrary directories — not arbitrary file read.

When acratranslator_client_id_from_connection_enable=true, the clientID is extracted from the TLS peer certificate and is not attacker-controlled. The HTTP API always extracts clientID from the TLS connection and is not affected.

Fix

Two changes:

1. Sanitize root in both constructors (CreateDirectoryBackend, OpenDirectoryBackend):

return &DirectoryBackend{
    root: filepath.Clean(root),
    log:  newLog,
    lock: lock,
}, nil

This ensures root is always in canonical form. This also fixes a latent issue in ListAll() where strings.TrimPrefix(path, b.root+separator) needs root to match filepath.Walk output format — Walk returns cleaned paths, so if root was not cleaned, TrimPrefix could silently fail to strip the prefix.

2. Replace the tautological filepath.Clean comparison in osPath() with strings.HasPrefix:

if !strings.HasPrefix(fullPath, b.root+string(os.PathSeparator)) {
    return "", api.ErrInvalidPath
}

The + string(os.PathSeparator) suffix prevents partial prefix matches — e.g. a root of /keys must not match a path under /keys-backup.

Testing

Verified with path traversal inputs (../, ../../, client/../../) that:

• Legitimate key paths within the root directory are allowed
• Paths resolving outside the root are rejected
• Edge cases (trailing slash on root, similar directory names like /keys-evil) are handled correctly
• All existing backend tests pass (TestFilesystem, TestInMemory — Get/Put, ListAll, Separators, Rename, RenameNX, Locking)