Closed Bug 1416730 Opened 7 years ago Closed 7 years ago

selfserv doesn't read crypto policy

Categories

(NSS :: Tools, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ueno, Assigned: ueno)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch selfserv-policy.patch (obsolete) — Splinter Review
Even when RC4 is prohibited through policy configuration, selfserv accepts connection using RC4 in key exchange (see below for the reproducer).  This is because the policy configuration is applied too early in selfserv.  Normally, policy configuration is loaded on ssl_Init() through any SSL_* public functions.  However, selfserv calls it after NSS_Initialize() so it doesn't take effect.
The attached patch changes this order to respect policy configuration.

Reproducer:

mkdir nssdb
certutil -d sql:nssdb -N

# Locally enable the policy by modifying nssdb/pkcs11.txt
sed -i '$i\
config="disallow=ALL allow=HMAC-SHA1:HMAC-SHA256:HMAC-SHA384:HMAC-SHA512:SECP256R1:SECP384R1:SECP521R1:aes256-gcm:chacha20-poly1305:aes256-cbc:camellia256-cbc:aes128-gcm:aes128-cbc:camellia128-cbc:des-ede3-cbc:SHA256:SHA384:SHA512:SHA1:ECDHE-RSA:ECDHE-ECDSA:RSA:DHE-RSA:DHE-DSS:tls-version-min=tls1.0:dtls-version-min=dtls1.0:DH-MIN=1023:DSA-MIN=1023:RSA-MIN=1023"'  nssdb/pkcs11.txt

dd if=/dev/urandom of=noise bs=1 count=32
certutil -S -z noise -s "CN=NSS Test Cert, O=BOGUS NSS, L=Mountain View, ST=California, C=US" -d sql:nssdb -n rsa -t ',,' -m 3651 -x

selfserv -d sql:nssdb -p 4433 -V tls1.0: -c :0004 -H 1 -n rsa

gnutls-cli -p 4433 --insecure --priority NORMAL:+ARCFOUR-128:+DHE-DSS:+SIGN-DSA-SHA1:+SIGN-DSA-SHA224:+SIGN-DSA-SHA256:+VERS-TLS1.2 localhost
(connection succeeds)

Originally reported in Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1426267
Attachment #8927796 - Flags: review?(rrelyea)
Comment on attachment 8927796 [details] [diff] [review]
selfserv-policy.patch

Review of attachment 8927796 [details] [diff] [review]:
-----------------------------------------------------------------

I wonder if we have other applications that my have this issue? We may want to make the policy trigger work both ways (it surprises me that we can initialize SSL before NSS, which is why the current policy was set up the way it was).

bob

::: tests/ssl/ssl.sh
@@ +682,4 @@
>  setup_policy()
>  {
>    policy="$1"
> +  outfile="$2"

We could also just pass in the directory. pkcs11.txt is hard coded inside NSS (currently). OTOH, if we ever change that, this code is more general so it's OK to keep it.
Attachment #8927796 - Flags: review?(rrelyea) → review+
Thank you for the review.

(In reply to Robert Relyea from comment #1)

> ::: tests/ssl/ssl.sh
> @@ +682,4 @@
> >  setup_policy()
> >  {
> >    policy="$1"
> > +  outfile="$2"
> 
> We could also just pass in the directory. pkcs11.txt is hard coded inside
> NSS (currently).

Yes, that's a good point; fixed and pushed it as: https://hg.mozilla.org/projects/nss/rev/6039a5e4ab01
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.35
I have reverted this change:
https://hg.mozilla.org/projects/nss/rev/99aa737128e1
as it causes timeout in SSL tests, when built with ASAN:
https://tools.taskcluster.net/groups/2ZftJTCkTKGgnoVcP1Apjg/tasks/2ZftJTCkTKGgnoVcP1Apjg/runs/0/logs/public%2Flogs%2Flive.log

I will look into it later.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It turned out that the issue was memory leak when selfserv is launched with the -b option, which terminates the process immediately (see the log below).  The attached patch delays the call of NSS_Initialize() right after the -b handling.

selfserv -b -p 8443 2>/dev/null;

=================================================================
==198==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x4c5503 in calloc /scratch/llvm/clang-4/xenial/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74:3
    #1 0x7f8a8ef55477 in PR_Calloc /home/worker/nspr/Debug/pr/src/malloc/../../../../pr/src/malloc/prmem.c:443:40
    #2 0x7f8a90955e8e in error_get_my_stack /home/worker/nss/out/Debug/../../lib/base/error.c:110:17
    #3 0x7f8a9095650c in nss_ClearErrorStack /home/worker/nss/out/Debug/../../lib/base/error.c:246:23
    #4 0x7f8a909526d8 in NSSArena_Create /home/worker/nss/out/Debug/../../lib/base/arena.c:329:5
    #5 0x7f8a909352df in NSSTrustDomain_Create /home/worker/nss/out/Debug/../../lib/pki/trustdomain.c:34:13
    #6 0x7f8a9091b33d in STAN_LoadDefaultNSS3TrustDomain /home/worker/nss/out/Debug/../../lib/pki/pki3hack.c:117:10
    #7 0x7f8a907beda6 in nss_Init /home/worker/nss/out/Debug/../../lib/nss/nssinit.c:705:13
    #8 0x7f8a907bf5a1 in NSS_Initialize /home/worker/nss/out/Debug/../../lib/nss/nssinit.c:889:12
    #9 0x4fd33b in main /home/worker/nss/out/Debug/../../cmd/selfserv/selfserv.c:2498:10
    #10 0x7f8a8f66982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x4c5503 in calloc /scratch/llvm/clang-4/xenial/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74:3
    #1 0x7f8a8ef55477 in PR_Calloc /home/worker/nspr/Debug/pr/src/malloc/../../../../pr/src/malloc/prmem.c:443:40
    #2 0x7f8a8a88765e in error_get_my_stack /home/worker/nss/out/Debug/../../lib/base/error.c:110:17
    #3 0x7f8a8a887bcc in nss_ClearErrorStack /home/worker/nss/out/Debug/../../lib/base/error.c:246:23
    #4 0x7f8a8a885478 in NSSArena_Create /home/worker/nss/out/Debug/../../lib/base/arena.c:329:5
    #5 0x7f8a8a8460ba in nssCKFWInstance_Create /home/worker/nss/out/Debug/../../lib/ckfw/instance.c:175:13
    #6 0x7f8a8a86f377 in NSSCKFWC_Initialize /home/worker/nss/out/Debug/../../lib/ckfw/wrap.c:172:20
    #7 0x7f8a8a83c9e5 in builtinsC_Initialize /home/worker/dist/public/nss/nssck.api:81:10
    #8 0x7f8a908536fa in secmod_ModuleInit /home/worker/nss/out/Debug/../../lib/pk11wrap/pk11load.c:245:11
    #9 0x7f8a90855dbc in secmod_LoadPKCS11Module /home/worker/nss/out/Debug/../../lib/pk11wrap/pk11load.c:504:10
    #10 0x7f8a9088e0b6 in SECMOD_LoadModule /home/worker/nss/out/Debug/../../lib/pk11wrap/pk11pars.c:1672:10
    #11 0x7f8a9088e368 in SECMOD_LoadModule /home/worker/nss/out/Debug/../../lib/pk11wrap/pk11pars.c:1707:25
    #12 0x7f8a907c20d3 in nss_InitModules /home/worker/nss/out/Debug/../../lib/nss/nssinit.c:464:18
    #13 0x7f8a907bed62 in nss_Init /home/worker/nss/out/Debug/../../lib/nss/nssinit.c:689:18
    #14 0x7f8a907bf5a1 in NSS_Initialize /home/worker/nss/out/Debug/../../lib/nss/nssinit.c:889:12
    #15 0x4fd33b in main /home/worker/nss/out/Debug/../../cmd/selfserv/selfserv.c:2498:10
    #16 0x7f8a8f66982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 144 byte(s) leaked in 2 allocation(s).
RETRY: selfserv -b -p 8443 2>/dev/null;
Attachment #8927796 - Attachment is obsolete: true
Attachment #8930495 - Flags: review?(rrelyea)
Attachment #8930495 - Flags: review?(rrelyea) → review+
Keywords: checkin-needed
Re-landed as: https://hg.mozilla.org/projects/nss/rev/469f02c40104
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Assignee: nobody → dueno
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: