Skip to content

Commit 9c33cfe

Browse files
LoCoBench Botclaude
andcommitted
feat: US-005 - Create cr-security-002: Security-adjacent review in Kafka Java
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 1bef1ee commit 9c33cfe

15 files changed

Lines changed: 717 additions & 4 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
FROM ubuntu:22.04
2+
3+
ENV DEBIAN_FRONTEND=noninteractive
4+
5+
# Install dependencies
6+
RUN apt-get update && apt-get install -y \
7+
git \
8+
openjdk-17-jdk \
9+
python3 \
10+
python3-pip \
11+
curl \
12+
jq \
13+
&& rm -rf /var/lib/apt/lists/*
14+
15+
WORKDIR /workspace
16+
17+
# Clone Kafka at pinned tag 3.8.0
18+
RUN git clone --branch 3.8.0 --depth 1 https://github.com/apache/kafka.git . \
19+
&& git config --global user.email "test@example.com" \
20+
&& git config --global user.name "Test User"
21+
22+
# Inject defects
23+
COPY inject_defects.sh /tmp/inject_defects.sh
24+
RUN chmod +x /tmp/inject_defects.sh && /tmp/inject_defects.sh
25+
26+
# Set Java home
27+
ENV JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64
28+
29+
CMD ["/bin/bash"]
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
#!/bin/bash
2+
set -e
3+
4+
# Defect 1 (critical): ScramSaslServer.java line 136 — invert iteration count check
5+
# Security impact: Allows weak credentials with low iteration counts to bypass SCRAM strength requirements
6+
FILE1="clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java"
7+
python3 -c "
8+
import sys
9+
with open('$FILE1', 'r') as f:
10+
content = f.read()
11+
12+
# Find and replace the iteration check (line 136)
13+
old = ''' if (scramCredential.iterations() < mechanism.minIterations())
14+
throw new SaslException(\"Iterations \" + scramCredential.iterations() + \" is less than the minimum \" + mechanism.minIterations() + \" for \" + mechanism);'''
15+
16+
new = ''' if (scramCredential.iterations() > mechanism.minIterations())
17+
throw new SaslException(\"Iterations \" + scramCredential.iterations() + \" is less than the minimum \" + mechanism.minIterations() + \" for \" + mechanism);'''
18+
19+
if old not in content:
20+
print('ERROR: Defect 1 pattern not found in ScramSaslServer.java', file=sys.stderr)
21+
sys.exit(1)
22+
23+
content = content.replace(old, new, 1)
24+
25+
with open('$FILE1', 'w') as f:
26+
f.write(content)
27+
print('Defect 1 injected: Inverted iteration count check')
28+
"
29+
30+
# Defect 2 (critical): ScramSaslServer.java line 226 — remove timing-safe comparison
31+
# Security impact: Enables timing attacks to extract credential hashes
32+
python3 -c "
33+
import sys
34+
with open('$FILE1', 'r') as f:
35+
content = f.read()
36+
37+
# Find and replace the timing-safe comparison (line 226-227)
38+
old = ''' if (!MessageDigest.isEqual(computedStoredKey, expectedStoredKey))
39+
throw new SaslException(\"Invalid client credentials\");'''
40+
41+
new = ''' if (!Arrays.equals(computedStoredKey, expectedStoredKey))
42+
throw new SaslException(\"Invalid client credentials\");'''
43+
44+
if old not in content:
45+
print('ERROR: Defect 2 pattern not found in ScramSaslServer.java', file=sys.stderr)
46+
sys.exit(1)
47+
48+
content = content.replace(old, new, 1)
49+
50+
with open('$FILE1', 'w') as f:
51+
f.write(content)
52+
print('Defect 2 injected: Replaced timing-safe MessageDigest.isEqual with Arrays.equals')
53+
"
54+
55+
# Defect 3 (high): SaslServerAuthenticator.java line 698 — remove session expiration calculation
56+
# Security impact: Sessions never expire even when credentials/tokens expire
57+
FILE2="clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java"
58+
python3 -c "
59+
import sys
60+
with open('$FILE2', 'r') as f:
61+
content = f.read()
62+
63+
# Find and comment out the sessionExpirationTimeNanos assignment (line 698)
64+
old = ''' sessionExpirationTimeNanos = authenticationEndNanos + 1000 * 1000 * retvalSessionLifetimeMs;'''
65+
66+
new = ''' // sessionExpirationTimeNanos = authenticationEndNanos + 1000 * 1000 * retvalSessionLifetimeMs;'''
67+
68+
if old not in content:
69+
print('ERROR: Defect 3 pattern not found in SaslServerAuthenticator.java', file=sys.stderr)
70+
sys.exit(1)
71+
72+
content = content.replace(old, new, 1)
73+
74+
with open('$FILE2', 'w') as f:
75+
f.write(content)
76+
print('Defect 3 injected: Commented out session expiration time calculation')
77+
"
78+
79+
# Defect 4 (critical): AclAuthorizer.scala line 522 — invert DENY ACL check
80+
# Security impact: Authorization bypass — DENY ACLs are ignored, allowing forbidden operations
81+
FILE3="core/src/main/scala/kafka/security/authorizer/AclAuthorizer.scala"
82+
python3 -c "
83+
import sys
84+
with open('$FILE3', 'r') as f:
85+
content = f.read()
86+
87+
# Find and invert the denyAclExists check in aclsAllowAccess (line 541)
88+
old = ''' isEmptyAclAndAuthorized(acls) || (!denyAclExists(acls) && allowAclExists(acls))'''
89+
90+
new = ''' isEmptyAclAndAuthorized(acls) || (denyAclExists(acls) && allowAclExists(acls))'''
91+
92+
if old not in content:
93+
print('ERROR: Defect 4 pattern not found in AclAuthorizer.scala', file=sys.stderr)
94+
sys.exit(1)
95+
96+
content = content.replace(old, new, 1)
97+
98+
with open('$FILE3', 'w') as f:
99+
f.write(content)
100+
print('Defect 4 injected: Removed negation from denyAclExists check')
101+
"
102+
103+
# Defect 5 (high): CredentialCache.java line 36 — remove credential class type validation
104+
# Security impact: Type confusion allows wrong credential type to be retrieved, bypassing auth checks
105+
FILE4="clients/src/main/java/org/apache/kafka/common/security/authenticator/CredentialCache.java"
106+
python3 -c "
107+
import sys
108+
with open('$FILE4', 'r') as f:
109+
content = f.read()
110+
111+
# Find and remove the credential class validation (lines 36-37)
112+
old = ''' if (cache.credentialClass() != credentialClass)
113+
throw new IllegalArgumentException(\"Invalid credential class \" + credentialClass + \", expected \" + cache.credentialClass());'''
114+
115+
new = ''' // Type check removed - cache can return any credential type
116+
// if (cache.credentialClass() != credentialClass)
117+
// throw new IllegalArgumentException(\"Invalid credential class \" + credentialClass + \", expected \" + cache.credentialClass());'''
118+
119+
if old not in content:
120+
print('ERROR: Defect 5 pattern not found in CredentialCache.java', file=sys.stderr)
121+
sys.exit(1)
122+
123+
content = content.replace(old, new, 1)
124+
125+
with open('$FILE4', 'w') as f:
126+
f.write(content)
127+
print('Defect 5 injected: Removed credential class type validation')
128+
"
129+
130+
echo "All 5 defects injected successfully"
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Code Review: Apache Kafka Security Defects
2+
3+
You are reviewing a pull request to the Apache Kafka project (`apache/kafka` tag 3.8.0). The PR modifies authentication and authorization logic in the security subsystem.
4+
5+
**Your task:** Find and fix **all injected defects** in the modified code.
6+
7+
## Modified Files
8+
9+
The PR touches 4 files in the Kafka security subsystem:
10+
11+
1. `clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java`
12+
2. `clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java`
13+
3. `clients/src/main/java/org/apache/kafka/common/security/authenticator/CredentialCache.java`
14+
4. `core/src/main/scala/kafka/security/authorizer/AclAuthorizer.scala`
15+
16+
These files handle SCRAM authentication, SASL server authentication, credential caching, and ACL-based authorization.
17+
18+
## What to Look For
19+
20+
Focus on security-critical defects including but not limited to:
21+
22+
- **Authentication bypass**: Missing or inverted credential validation checks
23+
- **Authorization bypass**: Missing or inverted ACL permission checks
24+
- **Credential validation**: Weakened iteration counts, disabled timing-attack protections
25+
- **Token/session management**: Missing expiry checks, unsafe credential caching
26+
- **Input validation**: Missing bounds checks, unsafe string comparisons
27+
28+
## Expected Output
29+
30+
1. **Code fixes**: Apply your fixes directly to the files in `/workspace`
31+
2. **Review report**: Create `/workspace/review.json` with this structure:
32+
33+
```json
34+
[
35+
{
36+
"file": "path/to/file.java",
37+
"line": 123,
38+
"severity": "critical",
39+
"description": "Brief description of the defect",
40+
"fix_applied": "Brief description of the fix"
41+
}
42+
]
43+
```
44+
45+
**Severity levels**: Use `critical` for authentication/authorization bypasses, `high` for credential validation weaknesses, `medium` for other security issues.
46+
47+
## Notes
48+
49+
- The workspace contains the full Kafka 3.8.0 source tree
50+
- All defects are realistic, subtle security bugs that could plausibly appear in code review
51+
- Some defects may require understanding interactions between multiple files
52+
- This is a **security-focused** review — prioritize finding all authentication and authorization defects
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
version = "1.0"
2+
[metadata]
3+
name = "cr-security-002"
4+
description = "Review apache/kafka for injected security-adjacent defects: authentication bypass, unsafe deserialization, missing authorization checks, credential validation bypass, and token expiry check"
5+
license = "Apache-2.0"
6+
7+
[task]
8+
id = "cr-security-002"
9+
repo = "kafka"
10+
category = "code-review"
11+
language = "java"
12+
difficulty = "hard"
13+
time_limit_sec = 1200
14+
15+
[verification]
16+
type = "test"
17+
command = "bash /tests/test.sh"
18+
19+
reward_type = "checklist"
20+
description = "F1 score for defect detection plus fix correctness with 2x weight for security-critical defects"
21+
[environment]
22+
build_timeout_sec = 1800.0
23+
24+
[environment.setup_scripts]
25+
mcp_config = """#!/bin/bash
26+
# Setup Sourcegraph MCP if credentials provided
27+
if [ -n "$SOURCEGRAPH_ACCESS_TOKEN" ] && [ -n "$SOURCEGRAPH_URL" ]; then
28+
echo "Setting up Sourcegraph MCP configuration..."
29+
mkdir -p /root/.config/claude
30+
31+
cat > /root/.config/claude/mcp.json << 'EOF'
32+
{
33+
"mcpServers": {
34+
"sourcegraph": {
35+
"command": "npx",
36+
"args": ["-y", "@sourcegraph/mcp-server"],
37+
"env": {
38+
"SRC_ACCESS_TOKEN": "$SOURCEGRAPH_ACCESS_TOKEN",
39+
"SOURCEGRAPH_URL": "$SOURCEGRAPH_URL"
40+
}
41+
}
42+
}
43+
}
44+
EOF
45+
46+
echo "PASS MCP configuration created"
47+
else
48+
echo "No Sourcegraph credentials provided, MCP disabled"
49+
fi
50+
exit 0
51+
"""
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
[
2+
{
3+
"id": "defect-1",
4+
"file": "clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java",
5+
"line_start": 136,
6+
"line_end": 137,
7+
"type": "security",
8+
"severity": "critical",
9+
"description": "Iteration count validation inverted (< changed to >) — allows weak SCRAM credentials with iteration counts below the minimum threshold to bypass authentication strength requirements, enabling brute-force attacks on low-iteration password hashes.",
10+
"cross_file": false
11+
},
12+
{
13+
"id": "defect-2",
14+
"file": "clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java",
15+
"line_start": 226,
16+
"line_end": 227,
17+
"type": "security",
18+
"severity": "critical",
19+
"description": "Timing-safe comparison replaced with Arrays.equals — MessageDigest.isEqual() prevents timing attacks by using constant-time comparison; Arrays.equals() leaks credential hash information through execution timing, allowing attackers to extract stored keys byte-by-byte.",
20+
"cross_file": false
21+
},
22+
{
23+
"id": "defect-3",
24+
"file": "clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java",
25+
"line_start": 698,
26+
"line_end": 698,
27+
"type": "security",
28+
"severity": "high",
29+
"description": "Session expiration time calculation commented out — without setting sessionExpirationTimeNanos, authenticated sessions never expire even when delegation tokens or credentials expire, allowing indefinite access with revoked credentials.",
30+
"cross_file": false
31+
},
32+
{
33+
"id": "defect-4",
34+
"file": "core/src/main/scala/kafka/security/authorizer/AclAuthorizer.scala",
35+
"line_start": 541,
36+
"line_end": 541,
37+
"type": "security",
38+
"severity": "critical",
39+
"description": "DENY ACL check negation removed — the condition changed from !denyAclExists to denyAclExists, inverting the logic so DENY ACLs are required instead of forbidden. This allows operations that should be explicitly denied to proceed, bypassing ACL-based authorization.",
40+
"cross_file": false
41+
},
42+
{
43+
"id": "defect-5",
44+
"file": "clients/src/main/java/org/apache/kafka/common/security/authenticator/CredentialCache.java",
45+
"line_start": 36,
46+
"line_end": 37,
47+
"type": "security",
48+
"severity": "high",
49+
"description": "Credential class type validation removed — without the credentialClass() != credentialClass check, the cache can return credentials of the wrong type (e.g., SCRAM credentials when PLAIN was requested), causing type confusion that bypasses authentication checks in callback handlers.",
50+
"cross_file": false
51+
}
52+
]
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--- a/clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java
2+
+++ b/clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java
3+
@@ -133,7 +133,7 @@ public class ScramSaslServer implements SaslServer {
4+
String authorizationIdFromClient = clientFirstMessage.authorizationId();
5+
if (!authorizationIdFromClient.isEmpty() && !authorizationIdFromClient.equals(username))
6+
throw new SaslAuthenticationException("Authentication failed: Client requested an authorization id that is different from username");
7+
8+
- if (scramCredential.iterations() > mechanism.minIterations())
9+
+ if (scramCredential.iterations() < mechanism.minIterations())
10+
throw new SaslException("Iterations " + scramCredential.iterations() + " is less than the minimum " + mechanism.minIterations() + " for " + mechanism);
11+
this.serverFirstMessage = new ServerFirstMessage(clientFirstMessage.nonce(),
12+
serverNonce,
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--- a/clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java
2+
+++ b/clients/src/main/java/org/apache/kafka/common/security/scram/internals/ScramSaslServer.java
3+
@@ -223,7 +223,7 @@ public class ScramSaslServer implements SaslServer {
4+
byte[] expectedStoredKey = scramCredential.storedKey();
5+
byte[] clientSignature = formatter.clientSignature(expectedStoredKey, clientFirstMessage, serverFirstMessage, clientFinalMessage);
6+
byte[] computedStoredKey = formatter.storedKey(clientSignature, clientFinalMessage.proof());
7+
- if (!Arrays.equals(computedStoredKey, expectedStoredKey))
8+
+ if (!MessageDigest.isEqual(computedStoredKey, expectedStoredKey))
9+
throw new SaslException("Invalid client credentials");
10+
} catch (InvalidKeyException e) {
11+
throw new SaslException("Sasl client verification failed", e);
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--- a/clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java
2+
+++ b/clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java
3+
@@ -695,7 +695,7 @@ public class SaslServerAuthenticator implements Authenticator {
4+
else
5+
retvalSessionLifetimeMs = zeroIfNegative(Math.min(credentialExpirationMs - authenticationEndMs, connectionsMaxReauthMs));
6+
7+
- // sessionExpirationTimeNanos = authenticationEndNanos + 1000 * 1000 * retvalSessionLifetimeMs;
8+
+ sessionExpirationTimeNanos = authenticationEndNanos + 1000 * 1000 * retvalSessionLifetimeMs;
9+
}
10+
11+
if (credentialExpirationMs != null) {
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--- a/core/src/main/scala/kafka/security/authorizer/AclAuthorizer.scala
2+
+++ b/core/src/main/scala/kafka/security/authorizer/AclAuthorizer.scala
3+
@@ -538,7 +538,7 @@ class AclAuthorizer extends Authorizer with Logging {
4+
// we allow an operation if no acls are found and user has configured to allow all users
5+
// when no acls are found or if no deny acls are found and at least one allow acls matches.
6+
val acls = matchingAcls(resource.resourceType, resource.name)
7+
- isEmptyAclAndAuthorized(acls) || (denyAclExists(acls) && allowAclExists(acls))
8+
+ isEmptyAclAndAuthorized(acls) || (!denyAclExists(acls) && allowAclExists(acls))
9+
}
10+
11+
// Evaluate if operation is allowed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--- a/clients/src/main/java/org/apache/kafka/common/security/authenticator/CredentialCache.java
2+
+++ b/clients/src/main/java/org/apache/kafka/common/security/authenticator/CredentialCache.java
3+
@@ -33,9 +33,8 @@ public class CredentialCache {
4+
public <C> Cache<C> cache(String mechanism, Class<C> credentialClass) {
5+
Cache<?> cache = cacheMap.get(mechanism);
6+
if (cache != null) {
7+
- // Type check removed - cache can return any credential type
8+
- // if (cache.credentialClass() != credentialClass)
9+
- // throw new IllegalArgumentException("Invalid credential class " + credentialClass + ", expected " + cache.credentialClass());
10+
+ if (cache.credentialClass() != credentialClass)
11+
+ throw new IllegalArgumentException("Invalid credential class " + credentialClass + ", expected " + cache.credentialClass());
12+
return (Cache<C>) cache;
13+
} else
14+
return null;

0 commit comments

Comments
 (0)