On Wed, May 27, 2020 at 07:59:09AM +0200, Landry Breuil wrote:
>
> Maybe opensc upstream have valid reasons for calling mlock() or not, i'm
> not in a position to judge that.
>
> Anyway, the corresponding code is probably there:
> https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/sc.c#L895
>
> Which was added 2 years ago in this PR, with subject 'Added memory
> locking for secrets': https://github.com/OpenSC/OpenSC/pull/1491/files
>
> And fwiw, i recently had exchanges with someone else trying to use an
> estonian id card, so it seems this usecase is 'common'. Maybe
> fixing/looking at opensc is the way to go.
>
Here a diff for security/opensc.
>From the PR description, the uses of mlock() is specially to avoid leaking
secrets to swap.
The diff switchs the function sc_mem_secure_alloc() to uses mmap(2) with
MAP_CONCEAL as we do for secrets (it excludes this chunk of memory from core
dumps), and to not uses mlock(2). And changes sc_mem_secure_free() too.
Could you try it (firefox with pledge(2) activated, and unveil still disabled) ?
Thanks.
--
Sebastien Marie
diff 753d46e2c6f99e4a69ce571e41ff1e16e4b9e615 /data/semarie/repos/openbsd/ports
blob - 8f081d8b44e2c20e9bed3b6786cf6069e8147ea3
file + security/opensc/Makefile
--- security/opensc/Makefile
+++ security/opensc/Makefile
@@ -3,7 +3,7 @@
COMMENT= set of libraries and utilities to access smart cards
V= 0.20.0
-REVISION = 0
+REVISION = 1
DISTNAME= opensc-${V}
SUBST_VARS += V
blob - /dev/null
file + security/opensc/patches/patch-src_libopensc_sc_c
--- security/opensc/patches/patch-src_libopensc_sc_c
+++ security/opensc/patches/patch-src_libopensc_sc_c
@@ -0,0 +1,42 @@
+$OpenBSD$
+use MAP_CONCEAL and avoid mlock(2)
+Index: src/libopensc/sc.c
+--- src/libopensc/sc.c.orig
++++ src/libopensc/sc.c
+@@ -892,6 +892,12 @@ void *sc_mem_secure_alloc(size_t len)
+ len = pages * page_size;
+ }
+
++#ifdef _OPENBSD_
++ p = mmap(NULL, len, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANON|MAP_CONCEAL, -1, 0);
++ if (p == MAP_FAILED) {
++ return NULL;
++ }
++#else
+ p = malloc(len);
+ if (p == NULL) {
+ return NULL;
+@@ -901,18 +907,23 @@ void *sc_mem_secure_alloc(size_t len)
+ #else
+ mlock(p, len);
+ #endif
++#endif
+
+ return p;
+ }
+
+ void sc_mem_secure_free(void *ptr, size_t len)
+ {
++#ifdef _OPENBSD_
++ munmap(ptr, len);
++#else
+ #ifdef _WIN32
+ VirtualUnlock(ptr, len);
+ #else
+ munlock(ptr, len);
+ #endif
+ free(ptr);
++#endif
+ }
+
+ void sc_mem_clear(void *ptr, size_t len)
