236335 - signText lacks support for non detached signatures, standard armors and good error reporting

Status: RESOLVED WORKSFORME

(Core Graveyard :: Security: UI, defect)

Description

[Massimiliano Pala]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040207 Firefox/0.8
Build Identifier: Mozilla/5.0

the signText JS function lacks the ability to generate non-detatched signatures
and to add the default armours to generated PKCS7 signatures, i.e. '-----BEGIN
PKCS7-----' and '-----END PKCS7-----'.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Peter Van der Beken [:peterv]]

*** Bug 259737 has been marked as a duplicate of this bug. ***

Comment 2

[Peter Van der Beken [:peterv]]

Also need better error reporting (we tried to be compatible with the 4.x error
reporting which was minimal).

Comment 3

[Jose Vano]

In my opinion, this bug should be marked as "invalid" because it's not a bug
(armours can be added with other javascript functions and non-detatched
signatures are not useful).

On the other hand, I think that error reporting is quite good. Peter, do you
really think that we need better error reporting? Why?

Comment 4

[Antti Lankila]

The error reporting is hideously bad, because for any number of issues, the only response is "error:internalError". This is not easily debuggable. The method should throw exceptions describing the cause of error more clearly. Because the current function has users, and it has been previously felt that changing the interface is unacceptable, there should be a new function which works around the issues of the current one.

The current function works like this:

crypto.signText(textToSign, "ask", "issuer1", "issuer2", "issuer3", ...);

Several things are wrong here.

- the second argument value "ask" is the only one accepted. ("auto" is alias for "ask")
- the latter arguments are for valid CAs, but that's too inflexible model. We should probably offer filtering by other certificate properties, such as subject, issuer or key purpose. (Especially filtering for non-repudiation keys should be possible, for contract signing purposes.)

So, I'd propose an alternative method (no suggestion for name, let me use "betterSignText" here), something like:

crypto.betterSignText(textToSign, function(x509Certificate) {
     return x509Certificate.keyPurpose[0] == 'non-repudiation'
});

The latter argument is a javascript function callback that could be used to "filter" the list of available certificates, allowing the programmer full control of what he believes to be appropriate certificates.

The object given as argument should be a JS representation of the x509 certificate, at its simplest nothing more than a hash map with appropriate keys. Ideally, there would be a genuine x509 object, so more powerful ways to access the certificate content could be made available. CA should be available at x509Certificate.CA, which is another x509Certificate, until the chain runs out. The return value should be a javascript boolean.

Some other issues with the current implementation:

Currently, Firefox refuses to deal with certificates which it does not trust, and I think this is a bug. The programmer should be given more power here, and if the callback's x509 object had a method that indicates whether firefox trusts the certificate, then programmer could choose to reject the certificate when she so desires, or ignore the flag altogether.

Similarly, the CA needs to be trusted for some particular purpose before crypto.signText agrees to sign text with certificate signed by that CA. This should not be so. Firstly, it is the server-side program making decisions based on the certificate that should be the judge of whether the signature is good for the purpose it was made for, and nothing should depend on the settings of the client system making the signature.

Even worse, the user has already indicated his desire to sign the text with a particular certificate, and now Firefox overrides his decision! We should simply allow the callback to decide whether to include the certificate to the list, and user's explicit choice should never be second-guessed by Firefox.

Comment 5

[:Cykesiopka]

signText() was removed in Bug 1030963.