Implement secure_getenv(3) if not provided by stdlib #147

Merged
merged 4 commits into from Sep 19, 2016

Conversation

Projects
None yet
3 participants
Collaborator

zyga commented Sep 19, 2016

This patch changes the previous approach to falling back to unsecure
getenv if the standard library (e.g. musl) didn't provide the
secure_getenv() function. Snap confine now provides an internal copy of
the required library function instead.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

Implement secure_getenv(3) if not provided by stdlib
This patch changes the previous approach to falling back to unsecure
getenv if the standard library (e.g. musl) didn't provide the
secure_getenv() function. Snap confine now provides an internal copy of
the required library function instead.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Collaborator

zyga commented Sep 19, 2016

Spread failures are caused by out-of-date things in the image. Let me try to get spread to update everything first.

+#include <sys/auxv.h>
+
+#ifndef HAVE_SECURE_GETENV
+char *secure_getenv(const char *name)
@chipaca

chipaca Sep 19, 2016

Member

you could add __attribute__((nonnull)) so that gcc and clang warn you if you pass in null, i think?

@chipaca

chipaca Sep 19, 2016

Member

also warn_unused_result fwiw

@zyga

zyga Sep 19, 2016

Collaborator

Nice, I'll do it

src/secure-getenv.c
+char *secure_getenv(const char *name)
+{
+ unsigned long secure = getauxval(AT_SECURE);
+ if (secure != 0)
@chipaca

chipaca Sep 19, 2016

Member

always use braces around ifs that aren't single-liners? Or maybe just always use braces around ifs.

@zyga

zyga Sep 19, 2016

Collaborator

{ } are like seat belts, will do

+1 with a nit and some suggestions

Thanks for the review. I'll update the pull request.

+#include <sys/auxv.h>
+
+#ifndef HAVE_SECURE_GETENV
+char *secure_getenv(const char *name)
@chipaca

chipaca Sep 19, 2016

Member

you could add __attribute__((nonnull)) so that gcc and clang warn you if you pass in null, i think?

@chipaca

chipaca Sep 19, 2016

Member

also warn_unused_result fwiw

@zyga

zyga Sep 19, 2016

Collaborator

Nice, I'll do it

src/secure-getenv.c
+char *secure_getenv(const char *name)
+{
+ unsigned long secure = getauxval(AT_SECURE);
+ if (secure != 0)
@chipaca

chipaca Sep 19, 2016

Member

always use braces around ifs that aren't single-liners? Or maybe just always use braces around ifs.

@zyga

zyga Sep 19, 2016

Collaborator

{ } are like seat belts, will do

Contributor

lool commented Sep 19, 2016

Looks good to me too, thanks for adding this

zyga added some commits Sep 19, 2016

Add extra sanity attributes to secure_getenv
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Use extra { } on one-line if's
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga merged commit 30f7ca1 into master Sep 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zyga zyga deleted the secure-getenv branch Sep 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment