Bug 736410

Summary: pem_Initialize should return CKR_CANT_LOCK as it isn't thread safe yet
Product: [Fedora] Fedora Reporter: Elio Maldonado Batiz <emaldona>
Component: nssAssignee: Elio Maldonado Batiz <emaldona>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: emaldona, kdudka, kengert, rrelyea
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: nss-3.12.11-3.fc17 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-08-16 22:30:43 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Return CKR_CANT_LOCK early on pem_Initialize
none
Return CKR_CANT_LOCK to tell module isn't thread safe
rrelyea: review-
Return CKR_CANT_LOCK to tell module isn't thread safe rrelyea: review+

Description Elio Maldonado Batiz 2011-09-07 15:57:06 UTC
Description of problem: The pem module isn't thread safe yet. NSS provides a mechanism for PKCS 11 modules to let the the cryptokey framework, ckfw, is they are thread-safe or not. The pem module doesn't use it and shouldn't.


Version-Release number of selected component (if applicable):


How reproducible: Often

Steps to Reproduce:
1. See Bug 701678

  
Actual results: slapd crashes as per quoted bug 701678
Expected results: it should crash

Additional info:

Comment 1 Elio Maldonado Batiz 2011-09-07 15:58:59 UTC
Typo: Expected results: it should NOT crash

Comment 2 Elio Maldonado Batiz 2011-09-07 16:38:57 UTC
From Bob Relyea: 
In the init function, pem_Initialize, check :
if (fwInstance->LockingState != SingleThreaded) {
   return CKR_CANT_LOCK;
}
At the very top (before you start allocating things).

Comment 3 Elio Maldonado Batiz 2011-09-07 17:34:09 UTC
I correct Comment 0 a bit. This is per prescription of the PKCS #11 specification and only incidentally related to the ckfw framework.

Comment 4 Elio Maldonado Batiz 2011-09-07 17:38:13 UTC
Created attachment 521956 [details]
Return CKR_CANT_LOCK early on pem_Initialize

Comment 5 Bob Relyea 2011-09-07 17:42:02 UTC
Elio, please attach a patch with more context (diff -up 15) thanks.

bob

Comment 6 Elio Maldonado Batiz 2011-09-07 17:56:20 UTC
Comment on attachment 521956 [details]
Return CKR_CANT_LOCK early on pem_Initialize

Withdrawing, make it compile first. This looks like an opaque type.

Comment 7 Kamil Dudka 2011-09-09 14:06:24 UTC
I tried to return CKR_CANT_LOCK from pem_Initialize() in a debugger and it was indeed called again with fwInstance->LockingState == SingleThreaded.  But I am afraid there is no sane way to access fwInstance->LockingState from the module.  Is there any other PKCS #11 module that uses the hack described in comment #2 ?

I thought another way to force secmod_LoadPKCS11Module() to see the module as not thread-safe would be to return {2, 0} from the NSSCKMDInstance::GetCryptokiVersion, but:

(1) I suspect that changing the version might have some side-effect.

(2) The logic in secmod_LoadPKCS11Module() seems to be broken anyway.  Although mod->isThreadSafe is correctly set to PR_FALSE in secmod_LoadPKCS11Module(), it is then never read before overwriting it first.  Or am I missing something?

Comment 8 Elio Maldonado Batiz 2011-09-09 16:10:06 UTC
The NSSCKFWInstance is opaque, it's internal structure defined inside inst.c as struct NSSCKFWInstanceStr, so the module cannot access it. An option would be to create an accessor function for it.

Also, in wrap.c, it's worth looking at 
/* figure out out locking semantics */
static CK_RV nssCKFW_GetThreadSafeState(CK_C_INITIALIZE_ARGS_PTR pInitArgs,
                           CryptokiLockingState *pLocking_state) {
Which has comments relating to the pkcs #11 rules from Secion 11.4, and is the one that sets the state when called by NSSCKFWC_Initialize when creating the framework instance.

The logic in secmod_LoadPKCS11Module(), isn't broken but it does look twisty and that's probably because of multi-initialization.

Comment 9 Kamil Dudka 2011-09-09 16:33:40 UTC
(In reply to comment #8)
> The NSSCKFWInstance is opaque, it's internal structure defined inside inst.c as
> struct NSSCKFWInstanceStr, so the module cannot access it. An option would be
> to create an accessor function for it.

This implies the hack from comment #2 has never been used anywhere else, right?
 
> Also, in wrap.c, it's worth looking at 
> /* figure out out locking semantics */
> static CK_RV nssCKFW_GetThreadSafeState(CK_C_INITIALIZE_ARGS_PTR pInitArgs,
>                            CryptokiLockingState *pLocking_state) {
> Which has comments relating to the pkcs #11 rules from Secion 11.4, and is the
> one that sets the state when called by NSSCKFWC_Initialize when creating the
> framework instance.

To put it simple, if we used the hack, nssCKFW_GetThreadSafeState() would be called with pInitArgs == NULL, which bypasses the "pkcs #11 rules from Secion 11.4" completely.

> The logic in secmod_LoadPKCS11Module(), isn't broken but it does look twisty
> and that's probably because of multi-initialization.

If you claim it is not broken, then tell me how can I use the downgrade of CryptokiVersion() to convince secmod_LoadPKCS11Module() that the module is not thread-safe.

Comment 10 Bob Relyea 2011-09-09 21:02:01 UTC
> The logic in secmod_LoadPKCS11Module(), isn't broken but it does look twisty
> and that's probably because of multi-initialization.

I needs to  handle all sorts of different cases. Not all PKCS #11 modules are alike.

> This implies the hack from comment #2 has never been used anywhere else, right?

return CKR_CANT_LOCK isn't a hack for a PKCS #11 module, Lots of them do so. It's the only way for a module to tell the upper level application that it's not thread safe.

Now using the given variable to test the state may not be the right way. ckfw already processes the arguments, so I was looking at the variable where it gets the result. Clearly that isn't accessable.

Instead we should try this:

CK_C_INITIALIZE_ARGS_PTR *initArgs;

initArgs = NSSCKFWInstance_GetInitArgs(fwInstance);
if (initArgs && 
   ((initArgs->flags & CKF_OS_LOCKING_OK) || (initArgs->CreateMutex != 0)) {
      return CKR_CANT_LOCK;
}


This is the normal check a PKCS #11 module that can't do thread locking would make.

bob

Comment 11 Bob Relyea 2011-09-09 21:20:19 UTC
> To put it simple, if we used the hack, nssCKFW_GetThreadSafeState() would be
> called with pInitArgs == NULL, which bypasses the "pkcs #11 rules from Secion
> 11.4" completely.

Right. Such tokens will be treated as not thread save (the application won't be accessing them from multiple threads simultaneously).

From seciont 11.4

"A call to C_Initialize with pInitArgs set to NULL_PTR is treated like C_Initialize with pInitArgs pointing to a CK_C_INITIALIZE_ARGS which has the CreateMultex, DestroyMutex, LockMutex, UnlockMutex, and pReserved fields set to NULL_PTR and the flags filed set to 0."

This corresponds to case 1.

secmod_LoadPKCS11Module() passes NULL at this point because it has already determined the module is not happy with being thread safe. This could be because the module parsed the data and decided it isn't happy being thread safe, or because the PKCS #11 module predates version 2.10 (I think that's when the CK_C_INITALIZE_ARGS were added to the spec). Those older modules would return CKR_ARGUMENTS_BAD if the argument wasn't NULL (and weren't considered thread safe). 


> If you claim it is not broken, then tell me how can I use the downgrade of
> CryptokiVersion() to convince secmod_LoadPKCS11Module() that the module is not
> thread-safe.

Exactly the code above does this... and has been working that way for a long time with other tokens. Note that this code implements exactly what 11.4 specifies.

bob

Comment 12 Bob Relyea 2011-09-09 21:45:15 UTC
> I thought another way to force secmod_LoadPKCS11Module() to see the module as
> not thread-safe would be to return {2, 0} from the
> NSSCKMDInstance::GetCryptokiVersion, but:

That would only work if the module acted like a 2.0 module. That would include returning CKR_ARGUMENTS_BAD if the init arg is not NULL.

NSS basically expects 2.0 modules to act like 2.0 modules, so it actually detects modules by actions, rather than by version number. The version number is used to control which new functions are safe to call in the function table. (So NSS won't try to call C_WaitForSlotEvent if the token isn't at least 2.1).

bob

Comment 13 Kamil Dudka 2011-09-11 10:17:29 UTC
(In reply to comment #12)
> That would only work if the module acted like a 2.0 module. That would include
> returning CKR_ARGUMENTS_BAD if the init arg is not NULL.

I meant the following code was broken:

    /* check the version number */
    if (PK11_GETTAB(mod)->C_GetInfo(&info) != CKR_OK) goto fail2;
    if (info.cryptokiVersion.major != 2) goto fail2;
    /* all 2.0 are a priori *not* thread safe */
    if (info.cryptokiVersion.minor < 1) {
        if (!loadSingleThreadedModules) {
            PORT_SetError(SEC_ERROR_INCOMPATIBLE_PKCS11);
            goto fail2;
        } else {
            mod->isThreadSafe = PR_FALSE;
        }
    }

It sets mod->isThreadSafe = PR_FALSE, but mod->isThreadSafe is then never read without overwriting it first.  secmod_ModuleInit() will always get mod->isThreadSafe == PR_TRUE when called from secmod_LoadPKCS11Module().

Which lock is actually the one that will prevent the module from multiple access with LockingState == SingleThreaded, but is disabled by default?

Comment 14 Bob Relyea 2011-09-12 23:50:24 UTC
> I meant the following code was broken:
> 
> It sets mod->isThreadSafe = PR_FALSE, but mod->isThreadSafe is then never read
> without overwriting it first. 

This code sets isThreadSafe to PR_FALSE if the module is pre- 2.1. It's been 10 years since I wrote the code, but I'm pretty sure it's paranoi for the case where a 2.0 PKCS #11 module didn't return an error when we passed in a non-null arg. By definition a 2.0 PKCS #11 module is not thread safe. So it looks like your idea of dropping back to 2.0 will work, but it's probably not a good idea in general to drop back to 2.0 and still present 2.1 or 2.2 functions elsewhere.

> secmod_ModuleInit() will always get
> mod->isThreadSafe == PR_TRUE when called from secmod_LoadPKCS11Module().

This is not true. 

    crv = PK11_GETTAB(mod)->C_Initialize(pInitArgs);
    if (CKR_CRYPTOKI_ALREADY_INITIALIZED == crv) {
          ...
    }
    if (crv != CKR_OK) {
        if (pInitArgs == NULL ||
                crv == CKR_NETSCAPE_CERTDB_FAILED ||
                crv == CKR_NETSCAPE_KEYDB_FAILED) {
            PORT_SetError(PK11_MapError(crv));
            return SECFailure;
        }
        if (!loadSingleThreadedModules) {
            PORT_SetError(SEC_ERROR_INCOMPATIBLE_PKCS11);
            return SECFailure;
        }
        mod->isThreadSafe = PR_FALSE;
        crv = PK11_GETTAB(mod)->C_Initialize(NULL);
        if ((CKR_CRYPTOKI_ALREADY_INITIALIZED == crv) &&
            (!enforceAlreadyInitializedError)) {
            *alreadyLoaded = PR_TRUE;
            return SECSuccess;
        }
        if (crv != CKR_OK)  {
            PORT_SetError(PK11_MapError(crv));
            return SECFailure;
        }
    }


Clearly if the first C_Initialize fails with any function code other then CKR_CRYPTOKI_ALREADY_INITIALIZED, CKR_NETSCAPE_CERTDB_FAILED, or CKR_NETSCAPE_KEYDB_FAILED, then we are going to set mod->isThreadSafe to PR_FALSE.

If the second initialize with NULL succeeds, then we continue. So exactly the case we are talking about with returning CKR_CANT_LOCK with the code I've given.

bob
-----------------------------------------------------------------------------

NOTE: this means we can't use the argument passing to libpem, but I expect that the code that makes argument passing work correctly would also fix the thread safety issues.

The issue that you can't do argument passing on a non-thread library could probably be made into an upstream bug. The fix would be realtively simple...

replace:
        crv = PK11_GETTAB(mod)->C_Initialize(NULL);

with:

        /* we we have library parameters that we are trying to set, and the
         * PKCS #11 module return CKR_CANT_LOCK, then we know 1) the module is
         * at least 2.1 (CKR_CANT_LOCK wasn't available until then), and that
         * the module is configured to take parameters. Initialize it as single
         * threaded, but keeping the parameters */
        if (mod->libraryParams && crv == CKR_CANT_LOCK) {
            PORT_Memset(&moduleArgs, 0, sizeof(moduleArgs));
            moduleArgs.libraryParams = mod->libraryParams;
            pInitArgs = &moduleArgs; /* redundant, here for clarity */
        } else {
            /* try initializing the module with not parameters to see if it's
             * either an old module or a single threaded one */
            pInitArgs = NULL;
        }
        crv = PK11_GETTAB(mod)->C_Initialize(pInitArgs);



bob

Comment 15 Kamil Dudka 2011-09-13 08:44:27 UTC
(In reply to comment #14)
> This code sets isThreadSafe to PR_FALSE if the module is pre- 2.1. It's been 10
> years since I wrote the code, but I'm pretty sure it's paranoi for the case
> where a 2.0 PKCS #11 module didn't return an error when we passed in a non-null
> arg. By definition a 2.0 PKCS #11 module is not thread safe. So it looks like
> your idea of dropping back to 2.0 will work, but it's probably not a good idea
> in general to drop back to 2.0 and still present 2.1 or 2.2 functions
> elsewhere.

Bob, I don't think you understand.  The code I listed above apparently is broken.  Just changing the version number to 2.0 has no effect on pInitArgs, neither on the value of fwInstance->LockingState.  I am not saying it needs to be fixed now, but justifying the broken code without at least trying out what it really does is ridiculous.

> > secmod_ModuleInit() will always get
> > mod->isThreadSafe == PR_TRUE when called from secmod_LoadPKCS11Module().
> 
> This is not true.

Please show me a code path starting from secmod_LoadPKCS11Module(), where mod->isThreadSafe could take effect.  There is only one call of secmod_ModuleInit() in that function and just before the call, you overwrite the value of isThreadSafe to PR_TRUE unconditionally.

>     crv = PK11_GETTAB(mod)->C_Initialize(pInitArgs);
>     if (CKR_CRYPTOKI_ALREADY_INITIALIZED == crv) {
>           ...
>     }
>     if (crv != CKR_OK) {
>         if (pInitArgs == NULL ||
>                 crv == CKR_NETSCAPE_CERTDB_FAILED ||
>                 crv == CKR_NETSCAPE_KEYDB_FAILED) {
>             PORT_SetError(PK11_MapError(crv));
>             return SECFailure;
>         }
>         if (!loadSingleThreadedModules) {
>             PORT_SetError(SEC_ERROR_INCOMPATIBLE_PKCS11);
>             return SECFailure;
>         }
>         mod->isThreadSafe = PR_FALSE;

This has no effect, the value would be overwritten on a subsequent attempt anyway.

>         crv = PK11_GETTAB(mod)->C_Initialize(NULL);

Initializing the module with NULL as argument does takes effect.

> Clearly if the first C_Initialize fails with any function code other then
> CKR_CRYPTOKI_ALREADY_INITIALIZED, CKR_NETSCAPE_CERTDB_FAILED, or
> CKR_NETSCAPE_KEYDB_FAILED, then we are going to set mod->isThreadSafe to
> PR_FALSE.
> 
> If the second initialize with NULL succeeds, then we continue. So exactly the
> case we are talking about with returning CKR_CANT_LOCK with the code I've
> given.

That works independently of mod->isThreadSafe.

> NOTE: this means we can't use the argument passing to libpem, but I expect that
> the code that makes argument passing work correctly would also fix the thread
> safety issues.

The code loading certificates given in PEM arguments is unmaintained and broken as pointed out in another bug report.  There is no known user of that feature, so we do not track it as a bug now.

Comment 16 Elio Maldonado Batiz 2011-09-13 15:51:21 UTC
Created attachment 522955 [details]
Return CKR_CANT_LOCK to tell module isn't thread safe

Comment 17 Elio Maldonado Batiz 2011-09-13 15:54:49 UTC
http://koji.fedoraproject.org/koji/taskinfo?taskID=3348637 is a debug scratch build with this patch and two others, from Nalin and Kamil, applied.

Comment 18 Bob Relyea 2011-09-13 22:54:25 UTC
> Bob, I don't think you understand.  The code I listed above apparently is
> broken.  Just changing the version number to 2.0 has no effect on pInitArgs,
> neither on the value of fwInstance->LockingState.  I am not saying it needs to
> be fixed now, but justifying the broken code without at least trying out what
> it really does is ridiculous.

Your right, I did not under stand that were talking about that state of the ckfw code, rather than NSS proper. I think part of the problem may be that you use NSS to broadly.

That being said, I myself would have not expectation that ckfw would act any differently. The variables you describe are internal variables for the specific PKCS #11 module (in this case libpem), and not anything NSS itself ever cares are sees.

One would make the argument that the *ckfw* code is broken. Probably a better way to say it is the ckfw code doesn't restrict it's semantics to 2.0 semantics if you change the version number. I'm not sure that's a big issue. In any case it doesn't matter that ckfw does not change it's view of LockingState or pInitArgs because that's not the effect we are talking about. We are talking about whether *NSS* proper will thread the PKCS #11 module as thread safe or not. The only issue if NSS treats the module as thread safe and it thinks that it is not. So setting 2.0 will cause NSS to lock around libpem, whether or not libpem thinks it needs to use it's own internal locks.

It seems to me if you want to support 2.0 semantics, the token dependent code should actually provide those semantics (which would involve returning CKR_BAD_ARGUMENTS if pInitArgs != NULL).

In any case I have no problem with an RFE for ckfw, but it's not really that high of a priority.

Just to be clear: LockingState and mod->isThreadSafe have nothing to do with each other specifically. They are used by two separate and independent pieces of code (the former being libpem, the latter being NSS). The only requirement is that if mod->isThreadSafe == PR_TRUE and LockingState == SingleThreaded, then we have a bug (probably in ckfw/libpem).



> Please show me a code path starting from secmod_LoadPKCS11Module(),
> where mod->isThreadSafe could take effect.  There is only one call of
> secmod_ModuleInit() in that function and just before the call, you
> overwrite the value of isThreadSafe to PR_TRUE unconditionally.


Sigh.... The code speaks for itself.....

    mod->isThreadSafe = PR_TRUE;

    /* Now we initialize the module */
    rv = secmod_ModuleInit(mod, oldModule, &alreadyLoaded);
    if (rv != SECSuccess) {
        goto fail;
    }

    /* module has been reloaded, this module itself is done,
     * return to the caller */
    if (mod->functionList == NULL) {
        mod->loaded = PR_TRUE; /* technically the module is loaded.. */
        return SECSuccess;
    }

    /* check the version number */
    if (PK11_GETTAB(mod)->C_GetInfo(&info) != CKR_OK) goto fail2;
    if (info.cryptokiVersion.major != 2) goto fail2;
    /* all 2.0 are a priori *not* thread safe */
    if (info.cryptokiVersion.minor < 1) {
        if (!loadSingleThreadedModules) {
            PORT_SetError(SEC_ERROR_INCOMPATIBLE_PKCS11);
            goto fail2;
        } else {
            mod->isThreadSafe = PR_FALSE;
        }
    }


NOTE 1: The explicit setting of mod->isThreadSafe is set to true *BEFORE* we call secmod_ModuleInit().
NOTE 2: mod is passed as the first argument to secmod_ModuleInit().
NOTE 3: pInitArgs is not NULL becaise it is only set to NULL if mod->isThreadSafe is set to FALSE (which you quite apply noted it is not at this point).
NOTE 4: After the C_Initialize() is called, it's error is checked and the code I pointed out in comment 14 becomes active and sets mod->isThreadSafe to FALSE if the appropriate error conditions occur.

I think what is confusing you is secmod_ModuleInit potentially calls C_Initialize *TWICE* and that mod is not a const pointer. secmod_ModuleInit can and does change it.


> This has no effect, the value would be overwritten on a subsequent attempt
> anyway.

Say what??? The code listed is the 'subsequent attempt'!

> That works independently of mod->isThreadSafe.

yes, and there interaction is embodied in the code I just described. Are you still confused about this. I can walk down a call chart if you would like?

> The code loading certificates given in PEM arguments is unmaintained and
> broken as pointed out in another bug report.  There is no known user of 
> that feature, so we do not track it as a bug now.

I partially agree. I agree it's not an issue for libpem, certainly at this time, maybe never. It's probably a good bug to file upstream, however, just so we don't loose it. I agree it's low priority (though it's also pretty easy to fix). (BTW, the described bug is in NSS proper, not libpem).

bob

Comment 19 Bob Relyea 2011-09-13 23:01:56 UTC
Comment on attachment 522955 [details]
Return CKR_CANT_LOCK to tell module isn't thread safe

r- Move the CKR_CANT_LOCK to the top. Let's not dobule initialize the PRNG and the log file! Go ahead and move the fwInstance check with it...

bob

Comment 20 Kamil Dudka 2011-09-14 00:30:43 UTC
Thanks for comprehensive explanation Bob.  I just mentioned here my observation from debugging another issue because the observation itself was not worth opening a separate bug.  As far as one can reach from libcurl, there is at least some dead code in secmod_ModuleInit(), which I mistakenly thought would be useful for what I needed:

    if (mod->isThreadSafe == PR_FALSE) {
        pInitArgs = NULL;
    } else ...

Looking once again at the NSS source code, I can see that SECMOD_CancelWait() and SECMOD_RestartModules() also call secmod_ModuleInit(), so the code above might be somehow useful on the way from them.  I do not know those functions.

Which lock is actually the one that attachment 522955 [details] enables?

Comment 21 Bob Relyea 2011-09-14 01:22:30 UTC
Initialy the logic in secmod_ModuleInit() was inline in pk11_loadModule(). It was split out to handle the two functions you meantioned.


SECMOD_CancelWait() closes out the corresponding SECMOD_WaitForAnySlotEvent() which uses the C_WaitForSlotEvent(). The PKCS #11 spec only gives one way to 'wake up' a sleeping C_WaitForSlotEvent() and that is to C_Finalize() the module. Once the C_WaitForSlotEvent() has returned, we reinitialize the existing module so it can be used again in the future. At this point the module already knows whether or not it is thread safe, so we only need to do one C_Initialize().

SECMOD_RestartModules() is for using NSS after a fork(). Some applications initialize NSS and then fork(). In the fork() portion, PKCS #11 specifies that PKCS #11 modules need to call C_Initialize again in the child. NSS already has the rest of it's state set, so it knows whether or not the particular module is threadsafe, so again we can reduce things down to a single C_Initialize().

I suspect that libcurl has neither a smartcard thread (more common in apps like Mozilla), nor forks() (more common in servers).

bob

Comment 22 Kamil Dudka 2011-09-14 06:24:34 UTC
(In reply to comment #21)
> I suspect that libcurl has neither a smartcard thread (more common in apps like
> Mozilla), nor forks() (more common in servers).

libcurl itself creates threads only to implement DNS resolving timeouts on top of the standard glibc's name resolving service, which does not support timeouts on its own.

However, libcurl is just a library.  We have absolutely no control about creating threads and processes by its user (which can be a higher-level library).  libcurl can be loaded/unloaded multiple times from different contexts of a single application etc.

Comment 23 Elio Maldonado Batiz 2011-09-14 16:00:01 UTC
Created attachment 523191 [details]
Return CKR_CANT_LOCK to tell module isn't thread safe

Moved the check early in the function just after checking for fwInstance.
Other fixes: moved variable declarations to the top we can't have C99-isms,
changed the modparms to char * and got rid of a set but unused variable.

Comment 24 Bob Relyea 2011-09-14 18:04:45 UTC
Comment on attachment 523191 [details]
Return CKR_CANT_LOCK to tell module isn't thread safe

r+ rrelyea

Comment 25 Kamil Dudka 2011-12-09 12:11:10 UTC
patch included in nss-3.12.11-3.fc17, moving to MODIFIED

Comment 26 Kai Engert (:kaie) (inactive account) 2012-03-20 20:39:35 UTC
can this be closed?