Bug 13533 - Unlink in have_secure_mkstemp causes build failure
Summary: Unlink in have_secure_mkstemp causes build failure
Status: NEW
Alias: None
Product: TALLOC
Classification: Unclassified
Component: libtalloc (show other bugs)
Version: unspecified
Hardware: Other Linux
: P5 normal
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-16 09:48 UTC by ajburley
Modified: 2018-07-16 20:41 UTC (History)
1 user (show)

See Also:


Attachments
Patch to move unlink to only be run before exit (1.48 KB, patch)
2018-07-16 18:14 UTC, ajburley
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description ajburley 2018-07-16 09:48:01 UTC
I'm trying to build talloc 2.1.14 from source on my Debian arm64 (aarch64) machine. When running configure, the build failed due to exit code 1 from test have_secure_mkstemp.

I extracted the C source code from config.log and commented out the two "unlink" lines and the test passed. I was also able to verify that the two test files in /tmp had the correct permissions 0600 using a simple "ls -l".

By uncommenting unlink and adding a simple printf("%o", st.st_mode), I found that after unlink, the file permissions read as 0700 not 0600. This 0700 is not really any less "secure" for the purposes you are telling.

I am not sure why you need to run "unlink" so early in this config test. Why not do all your assertions first and then run "unlink" at the end of the function?

Furthermore, it seems wrong for the entire build to fail just because you determined that have_secure_mkstemp = no. For other tests (which presumably come from the default WAF), the build can proceed if the result was "no". In this case, you have provided a replacement mkstemp (based on mktemp) in libreplace, so if have_secure_mkstemp really gives result "no", you should use the replacement mkstemp rather than failing the entire build.
Comment 1 Jeremy Allison 2018-07-16 17:25:48 UTC
To be honest, I'm not even sure we should compile if we don't have mkstemp.

We should probably remove the fallback as it's promising something it doesn't deliver.
Comment 2 ajburley 2018-07-16 18:14:30 UTC
Created attachment 14325 [details]
Patch to move unlink to only be run before exit
Comment 3 ajburley 2018-07-16 18:15:03 UTC
Not sure I agree...mkstemp is just mktemp + chmod, so a replacement should be possible. In this case, the replacement (rep_mkstemp) appears to just use mktemp to get a unique file name, then use open to recreate the file with desired permissions. Not sure there is anything wrong with that. The README clearly states that we must at least have mktemp even if mkstemp is broken.

In any case, for me the problem is only caused by unlink and is unrelated to mkstemp itself, which meets the contract. I have attached a patch - when I apply this (unlink before exit rather than at the beginning), the test passes and mkstemp is reported available (the expected behaviour) and I'm able to build talloc.
Comment 4 Volker Lendecke 2018-07-16 20:41:15 UTC
This is about talloc. talloc does not need mkstemp. Is there a way to factor libreplace such that a pure libtalloc build does not fall over this?