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

[FLINK-20267][runtime] The JaasModule didn't support symbolic links. #14171

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -159,12 +159,20 @@ private static File generateDefaultConfigFile(String workingDir) {
checkArgument(workingDir != null, "working directory should not be null.");
final File jaasConfFile;
try {
Path path = Files.createDirectories(Paths.get(workingDir));
Path path = Paths.get(workingDir);
if (Files.notExists(path)) {
// We intentionally favored Path.toRealPath over Files.readSymbolicLinks as the latter one might return a
// relative path if the symbolic link refers to it. Path.toRealPath resolves the relative path instead.
Path parent = path.getParent().toRealPath();
Path resolvedPath = Paths.get(parent.toString(), path.getFileName().toString());

path = Files.createDirectories(resolvedPath);
XComp marked this conversation as resolved.
Show resolved Hide resolved
}
Path jaasConfPath = Files.createTempFile(path, "jaas-", ".conf");
try (InputStream resourceStream = JaasModule.class.getClassLoader().getResourceAsStream(JAAS_CONF_RESOURCE_NAME)) {
Files.copy(resourceStream, jaasConfPath, StandardCopyOption.REPLACE_EXISTING);
}
jaasConfFile = jaasConfPath.toFile();
jaasConfFile = new File(workingDir, jaasConfPath.getFileName().toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you do this change here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it in order to maintain the symlinks in the path?

Copy link
Contributor Author

@XComp XComp Nov 24, 2020

Choose a reason for hiding this comment

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

Yes, exactly. I wanted to avoid having a different path than the one the user specified in the configuration. The old implementation would have lead to the resolved path being exposed.

jaasConfFile.deleteOnExit();
} catch (IOException e) {
throw new RuntimeException("unable to generate a JAAS configuration file", e);
Expand Down
Expand Up @@ -29,8 +29,12 @@

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;

import static org.apache.flink.runtime.security.modules.JaasModule.JAVA_SECURITY_AUTH_LOGIN_CONFIG;
import static org.hamcrest.core.StringStartsWith.startsWith;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

/**
Expand Down Expand Up @@ -59,10 +63,33 @@ public void testJaasModuleFilePathIfWorkingDirNotPresent() throws IOException {
testJaasModuleFilePath(file.toPath().toString() + "/tmp");
}

@Test
public void testJaasModuleFilePathIfWorkingDirIsSymLink() throws IOException {
Path symlink = createSymLinkFolderStructure();
testJaasModuleFilePath(symlink.toString());
}

@Test
public void testJaasModuleFilePathIfWorkingDirNoPresentAndPathContainsSymLink() throws IOException {
Path symlink = createSymLinkFolderStructure();
testJaasModuleFilePath(symlink.toString() + "/tmp");
}

private Path createSymLinkFolderStructure() throws IOException {
File baseFolder = folder.newFolder();
File actualFolder = new File(baseFolder, "actual_folder");
assertTrue(actualFolder.mkdirs());

Path symlink = new File(baseFolder, "symlink").toPath();
Files.createSymbolicLink(symlink, actualFolder.toPath());

return symlink;
}

/**
* Test that the jaas config file is created in the working directory.
*/
private void testJaasModuleFilePath(String workingDir) {
private void testJaasModuleFilePath(String workingDir) throws IOException {
Configuration configuration = new Configuration();
// set the string for CoreOptions.TMP_DIRS to mock the working directory.
configuration.setString(CoreOptions.TMP_DIRS, workingDir);
Expand All @@ -79,7 +106,7 @@ private void testJaasModuleFilePath(String workingDir) {
* if we do not manually specify it.
*/
@Test
public void testCreateJaasModuleFileInTemporary() {
public void testCreateJaasModuleFileInTemporary() throws IOException {
Configuration configuration = new Configuration();
SecurityConfiguration sc = new SecurityConfiguration(configuration);
JaasModule module = new JaasModule(sc);
Expand All @@ -89,8 +116,12 @@ public void testCreateJaasModuleFileInTemporary() {
assertJaasFileLocateInRightDirectory(CoreOptions.TMP_DIRS.defaultValue());
}

private void assertJaasFileLocateInRightDirectory(String directory) {
assertTrue(System.getProperty(JAVA_SECURITY_AUTH_LOGIN_CONFIG).startsWith(directory));
private void assertJaasFileLocateInRightDirectory(String directory) throws IOException {
String resolvedExpectedPath = new File(directory).toPath().toRealPath().toString();
String resolvedActualPathWithFile = new File(System.getProperty(JAVA_SECURITY_AUTH_LOGIN_CONFIG)).toPath().toRealPath().toString();
assertThat("The resolved configured directory does not match the expected resolved one.", resolvedActualPathWithFile, startsWith(resolvedExpectedPath));

assertThat("The configured directory does not match the expected one.", System.getProperty(JAVA_SECURITY_AUTH_LOGIN_CONFIG), startsWith(directory));
}
}