Skip to content

Commit 75cb6b3

Browse files
astefanastefan
authored
Account for a possible rolled over file while reading the audit log file (#34909)
1 parent 76b32e6 commit 75cb6b3

File tree

3 files changed

+91
-37
lines changed

3 files changed

+91
-37
lines changed

‎x-pack/plugin/sql/qa/security/build.gradle‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Project mainProject = project
77
group = "${group}.x-pack.qa.sql.security"
88

99
// Tests are pushed down to subprojects and will be checked there.
10-
testingConventions.enabled = false
10+
testingConventions.enabled = false
1111

1212
subprojects {
1313
// Use resources from the parent project in subprojects
@@ -41,8 +41,11 @@ subprojects {
4141
}
4242

4343
integTestRunner {
44+
def today = new Date().format('yyyy-MM-dd')
4445
systemProperty 'tests.audit.logfile',
4546
"${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }_audit.json"
47+
systemProperty 'tests.audit.yesterday.logfile',
48+
"${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }_audit-${today}.json"
4649
}
4750

4851
runqa {

‎x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java‎

Lines changed: 86 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ protected interface Actions {
8484
* {@code plugin-security.policy}. So we may as well have gradle set the property.
8585
*/
8686
private static final Path AUDIT_LOG_FILE = lookupAuditLog();
87+
private static final Path ROLLED_OVER_AUDIT_LOG_FILE = lookupRolledOverAuditLog();
8788

8889
@SuppressForbidden(reason="security doesn't work with mock filesystem")
8990
private static Path lookupAuditLog() {
@@ -95,6 +96,16 @@ private static Path lookupAuditLog() {
9596
}
9697
return Paths.get(auditLogFileString);
9798
}
99+
100+
@SuppressForbidden(reason="security doesn't work with mock filesystem")
101+
private static Path lookupRolledOverAuditLog() {
102+
String auditLogFileString = System.getProperty("tests.audit.yesterday.logfile");
103+
if (null == auditLogFileString) {
104+
throw new IllegalStateException("tests.audit.yesterday.logfile must be set to run this test. It should be automatically "
105+
+ "set by gradle.");
106+
}
107+
return Paths.get(auditLogFileString);
108+
}
98109

99110
private static boolean oneTimeSetup = false;
100111
private static boolean auditFailure = false;
@@ -107,7 +118,12 @@ private static Path lookupAuditLog() {
107118
/**
108119
* How much of the audit log was written before the test started.
109120
*/
110-
private long auditLogWrittenBeforeTestStart;
121+
private static long auditLogWrittenBeforeTestStart;
122+
123+
/**
124+
* If the audit log file rolled over. This is a rare case possible only at midnight.
125+
*/
126+
private static boolean auditFileRolledOver = false;
111127

112128
public SqlSecurityTestCase(Actions actions) {
113129
this.actions = actions;
@@ -164,7 +180,7 @@ public void setInitialAuditLogOffset() {
164180
return null;
165181
}
166182
if (false == Files.isRegularFile(AUDIT_LOG_FILE)) {
167-
throw new IllegalStateException("expected tests.audit.logfile [" + AUDIT_LOG_FILE + "]to be a plain file but wasn't");
183+
throw new IllegalStateException("expected tests.audit.logfile [" + AUDIT_LOG_FILE + "] to be a plain file but wasn't");
168184
}
169185
try {
170186
auditLogWrittenBeforeTestStart = Files.size(AUDIT_LOG_FILE);
@@ -556,51 +572,85 @@ public void assertLogs() throws Exception {
556572
if (sm != null) {
557573
sm.checkPermission(new SpecialPermission());
558574
}
559-
BufferedReader logReader = AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
575+
576+
BufferedReader[] logReaders = new BufferedReader[2];
577+
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
560578
try {
561-
return Files.newBufferedReader(AUDIT_LOG_FILE, StandardCharsets.UTF_8);
579+
// the audit log file rolled over during the test
580+
// and we need to consume the rest of the rolled over file plus the new audit log file
581+
if (auditFileRolledOver == false && Files.exists(ROLLED_OVER_AUDIT_LOG_FILE)) {
582+
// once the audit file rolled over, it will stay like this
583+
auditFileRolledOver = true;
584+
// the order in the array matters, as the readers will be used in that order
585+
logReaders[0] = Files.newBufferedReader(ROLLED_OVER_AUDIT_LOG_FILE, StandardCharsets.UTF_8);
586+
}
587+
logReaders[1] = Files.newBufferedReader(AUDIT_LOG_FILE, StandardCharsets.UTF_8);
588+
return null;
562589
} catch (IOException e) {
563590
throw new RuntimeException(e);
564591
}
565592
});
566-
logReader.skip(auditLogWrittenBeforeTestStart);
593+
594+
// The "index" is used as a way of reading from both rolled over file and current audit file in order: rolled over file
595+
// first, then the audit log file. Very rarely we will read from the rolled over file: when the test happened to run
596+
// at midnight and the audit file rolled over during the test.
597+
int index;
598+
if (logReaders[0] != null) {
599+
logReaders[0].skip(auditLogWrittenBeforeTestStart);
600+
// start with the rolled over file first
601+
index = 0;
602+
} else {
603+
// the current audit log file reader should always be non-null
604+
logReaders[1].skip(auditLogWrittenBeforeTestStart);
605+
// start with the current audit logging file
606+
index = 1;
607+
}
567608

568609
List<Map<String, Object>> logs = new ArrayList<>();
569610
String line;
570-
while ((line = logReader.readLine()) != null) {
571-
try {
572-
final Map<String, Object> log = XContentHelper.convertToMap(JsonXContent.jsonXContent, line, false);
573-
if (false == ("access_denied".equals(log.get("event.action"))
574-
|| "access_granted".equals(log.get("event.action")))) {
575-
continue;
576-
}
577-
assertThat(log.containsKey("action"), is(true));
578-
if (false == (SQL_ACTION_NAME.equals(log.get("action"))
579-
|| GetIndexAction.NAME.equals(log.get("action"))
580-
|| FieldCapabilitiesAction.NAME.equals(log.get("action")))) {
581-
// TODO we may want to extend this and the assertions to SearchAction.NAME as well
582-
continue;
611+
while (index < 2) {
612+
line = logReaders[index].readLine();
613+
// when the end of the file is reached, either stop or move to the next reader
614+
if (line == null) {
615+
if (++index == 2) {
616+
break;
583617
}
584-
assertThat(log.containsKey("user.name"), is(true));
585-
List<String> indices = new ArrayList<>();
586-
if (log.containsKey("indices")) {
587-
indices = (ArrayList<String>) log.get("indices");
588-
if ("test_admin".equals(log.get("user.name"))) {
589-
/*
590-
* Sometimes we accidentally sneak access to the security tables. This is fine,
591-
* SQL drops them from the interface. So we might have access to them, but we
592-
* don't show them.
593-
*/
594-
indices.remove(".security");
595-
indices.remove(".security-6");
618+
}
619+
else {
620+
try {
621+
final Map<String, Object> log = XContentHelper.convertToMap(JsonXContent.jsonXContent, line, false);
622+
if (false == ("access_denied".equals(log.get("event.action"))
623+
|| "access_granted".equals(log.get("event.action")))) {
624+
continue;
625+
}
626+
assertThat(log.containsKey("action"), is(true));
627+
if (false == (SQL_ACTION_NAME.equals(log.get("action"))
628+
|| GetIndexAction.NAME.equals(log.get("action"))
629+
|| FieldCapabilitiesAction.NAME.equals(log.get("action")))) {
630+
// TODO we may want to extend this and the assertions to SearchAction.NAME as well
631+
continue;
632+
}
633+
assertThat(log.containsKey("user.name"), is(true));
634+
List<String> indices = new ArrayList<>();
635+
if (log.containsKey("indices")) {
636+
indices = (ArrayList<String>) log.get("indices");
637+
if ("test_admin".equals(log.get("user.name"))) {
638+
/*
639+
* Sometimes we accidentally sneak access to the security tables. This is fine,
640+
* SQL drops them from the interface. So we might have access to them, but we
641+
* don't show them.
642+
*/
643+
indices.remove(".security");
644+
indices.remove(".security-6");
645+
}
596646
}
647+
// Use a sorted list for indices for consistent error reporting
648+
Collections.sort(indices);
649+
log.put("indices", indices);
650+
logs.add(log);
651+
} catch (final ElasticsearchParseException e) {
652+
throw new IllegalArgumentException("Unrecognized log: " + line, e);
597653
}
598-
// Use a sorted list for indices for consistent error reporting
599-
Collections.sort(indices);
600-
log.put("indices", indices);
601-
logs.add(log);
602-
} catch (final ElasticsearchParseException e) {
603-
throw new IllegalArgumentException("Unrecognized log: " + line, e);
604654
}
605655
}
606656
List<Map<String, Object>> allLogs = new ArrayList<>(logs);

‎x-pack/plugin/sql/qa/security/src/test/resources/plugin-security.policy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
grant {
22
// Needed to read the audit log file
33
permission java.io.FilePermission "${tests.audit.logfile}", "read";
4+
permission java.io.FilePermission "${tests.audit.yesterday.logfile}", "read";
45

56
//// Required by ssl subproject:
67
// Required for the net client to setup ssl rather than use global ssl.

0 commit comments

Comments
 (0)