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

Cannot execute protoc on /tmp with noexec #39

Closed
sandordargo opened this issue Jun 2, 2017 · 20 comments
Closed

Cannot execute protoc on /tmp with noexec #39

sandordargo opened this issue Jun 2, 2017 · 20 comments

Comments

@sandordargo
Copy link

Hi,

I'm using Ubuntu 16.04, in my project I include protoc-jar-maven-plugin 3.2.0.1.

The maven build fails, complaining that it cannot execute protoc.exe. It seems that protoc thinks it is on windows.

A workaround is that I add
/usr/local/bin/protoc
in my pom.xml.

Thanks for checking this.

Sandor

@os72
Copy link
Owner

os72 commented Jun 2, 2017

Very strange.. What platform does it show? (it prints this to console)

@sandordargo
Copy link
Author

Okay, I found the root cause.

I saw a permission denied message, but I thought it's misleading, because just before there are logs indicating that the tool tries to run an exe file on linux. Now I can see that the platform doesn't matter the protoc executable will be called exe.

So I went back to this permission denied message. It turns out that my /tmp is mounted with noexec which I don't intend to change for security and policy reasons.

It also means that anyone else following the Securing Debian Manual might have the same issues.

I might create PR next week proposing a solution for this issue if you don't mind.

@os72
Copy link
Owner

os72 commented Jun 3, 2017

Yes, please do. There must be some alternative location to extract to and execute?

@sandordargo sandordargo changed the title Wrong platform detected Cannot execute protoc on /tmp with noexec Jun 3, 2017
@sandordargo
Copy link
Author

sandordargo commented Jun 3, 2017

Next will, I will definitely find the time to do it.
Well one option is to define some alternative location in the pom, but I would prefer something different. Determine if it's executable from the /tmp, if not create somewhere some other temporary file.

To give the context:
We'd like to use this great tool mostly from Windows but there are some folks like me with Linux machines. So defining an alternative location in the pom is not so great.

For the moment my workaround is to clone, build and install protoc, then in the pom add this
<protocCommand>/usr/local/bin/protoc</protocCommand>

@os72
Copy link
Owner

os72 commented Jun 5, 2017

I don't know, /tmp is for temp files and creation is via File.createTempFile(). But I'm open for suggestions..

@sandordargo
Copy link
Author

For File.createTempFile() you can pass a third parameter which is the directory where the temp file will be created.

What I thought about is that in case of a permission denied exception, I check if the platform is linux, check if the mount has the option of noexec and if these conditions apply, I try to recover.

I'm still hesitating a bit if this should be part of a normal flow or a recovery action called from a try-catch block. However considering that this issue was no reported so far, it can be treated exceptional. Plus apparently there is easy/cheap way to determine if the mount is mounted as noexec, so it might be preferable to do it only if it's really needed.

@os72
Copy link
Owner

os72 commented Jun 6, 2017

What could be a good alternative directory? We could add it by default and use it on exception, not even necessary to change config

@sandordargo
Copy link
Author

I already already made some experiments, maybe tomorrow I can commit it. What I used is to get the user.home from system properties and create a tmp folder there which I delete on exit. What do you think?

@os72
Copy link
Owner

os72 commented Jun 6, 2017

Yes that should work. (Although I'm not clear how that's more secure than via /tmp)

@sandordargo
Copy link
Author

I'm not an expert on Linux security, I don't see it either. But still Securing Debian Manual suggests people to do so... Anyway let's do this way, I'll create the PR today.

@os72
Copy link
Owner

os72 commented Jun 10, 2017

Turns out temp dir location can be changed, system property java.io.tmpdir. How about that?

$ java -Djava.io.tmpdir=. -jar target/protoc-jar-3.3.0.1-SNAPSHOT.jar
protoc-jar: protoc version: 330, detected platform: windows 8/amd64
protoc-jar: executing: [C:\github\protoc-jar\.\protocjar7783982935981426806\bin\protoc.exe]
Missing input file.

Or we add a plugin parameter altTempDir or so? <altTempDir>${user.home}</altTempDir>
What do you think?

@os72
Copy link
Owner

os72 commented Jun 12, 2017

I see 3 options

  1. altTempDir
  2. Extract, do one dummy runProtoc, switch to user.home temp if needed, re-extract
  3. Do something like (2) during actual compilation (but only once). Most complex

@sandordargo
Copy link
Author

We can add altTempDir, but it is not enough. Using that means you can't have one config which works everywhere. Option three is nice, but is it worth it? As this shouldn't happen too many times, 2 seems good enough for me. What do you think?

@sandordargo
Copy link
Author

I haven't worked with maven plugins, so maybe my preconception is wrong. In the maven plugin don't you use the code from os72/protoc-jar?

@os72
Copy link
Owner

os72 commented Jun 13, 2017

The plugin uses protoc-jar but extracts protoc only once, then executes it multiple times. So we need changes here as well
https://github.com/os72/protoc-jar-maven-plugin/blob/master/src/main/java/com/github/os72/protocjar/maven/ProtocJarMojo.java#L319

I see what you mean about option (1). Will go with option (2)

@sandordargo
Copy link
Author

Okay, now I see why we have public static int runProtoc(String cmd, String[] args).

If we wanted to keep changes minimal we could reuse pretty much what we did.

Extracting the few lines at https://github.com/os72/protoc-jar/blob/master/src/main/java/com/github/os72/protocjar/Protoc.java#L62 into a separate method (such as attemptRecovery would let us call it in https://github.com/os72/protoc-jar/blob/master/src/main/java/com/github/os72/protocjar/Protoc.java#L101 if the IOE exception has permission denied as a cause.

What do you think?

@os72
Copy link
Owner

os72 commented Jun 14, 2017

Yes we need a dummy execution like this one: https://github.com/os72/protoc-jar-maven-plugin/blob/master/src/main/java/com/github/os72/protocjar/maven/ProtocJarMojo.java#L303

Will you be able to build and test a SNAPSHOT?

@os72
Copy link
Owner

os72 commented Jun 15, 2017

Could you try 3.3.0.1-SNAPSHOT?

@sandordargo
Copy link
Author

Yes, it's working fine! Thank you!

@os72
Copy link
Owner

os72 commented Jun 23, 2017

Released 3.3.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants