Closed Bug 356855 Opened 18 years ago Closed 8 years ago

Add support for a shared-secret HTTP Authentication scheme based on SRP

Categories

(Core :: Networking: HTTP, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: sayrer, Assigned: sayrer)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 8 obsolete files)

Informally described here:

<http://www.franklinmint.fm/blog/archives/000843.html>
This Python file shows the interactions between client and server. (skipping the actual HTTP traffic)

If you run it on the command line, you can insert print statements to see the various components of the calculations.
(In reply to comment #0)
> Informally described here:
> 
> <http://www.franklinmint.fm/blog/archives/000843.html>
> 

I'm not able to resolve the hostname of that URL
Oops, looks like my domain name registration lapsed. Here's the post, stripped of site-specific links.
Attached file server.py
This server implements a Digest variant that fixes most of the problems with RFC2617, except it doesn't incorporate SRP. It is roughly analogous to <http://tools.ietf.org/html/draft-sayre-http-hmac-digest-01>.
This extension implements the client portion of the spec, without SRP again.

The next step is to incorporate the SRP calculations into the digest, as demonstrated by attachment 253773 [details].
Attached patch include libtommath (obsolete) — Splinter Review
here it is with some crypto and bignum stuff. NSS can do all this as well, but I can't link to it as things stand.
Attachment #269323 - Attachment is obsolete: true
Attached patch in-tree version, with no bignums (obsolete) — Splinter Review
trying this to see what codesize is like. Might be able to get a bit more by refactoring nsHttpDigestAuth::MD5Hash to be more general.
Attached patch in-tree version, with no bignums (obsolete) — Splinter Review
Attachment #272063 - Attachment is obsolete: true
My codesize data is much different from the tinderbox, but it doesn't look like a big addition so far:

33140850
+14050 (+15024/-974)

Overall Change in Size
	Total:	     +14050 (+15024/-974)
	Code:	     +11977 (+12951/-974)
	Data:	      +2073 (+2073/+0)
Attachment #272065 - Attachment is obsolete: true
Attached patch add missing idlSplinter Review
Attachment #269559 - Attachment is obsolete: true
Attachment #272727 - Attachment is obsolete: true
OK, now that I understand the way NSS builds just a little, I see that I won't have MPI available. They are private in libfreebl, so I'm not sure what the best way forward is. I don't think all NSS users would want to build SRP, for a variety of reasons.
Attached file srp.c (WIP) (obsolete) —
Here's a seemingly correct implementation of the client portion of SRP, written as part of NSS.
I think the patch in attachment 273042 [details] has potential.  A little more work
would be needed to expose this function from the freebl shared library 
through the freebl interface, and potentially a bunch MORE work would be 
needed if we desired/chose to expose it though the PKCS#11 API.  But that's 
all feasible.

Here are some other issues to consider.  

Today, Mozilla's products (browsers, email clients) are used by many users,
including users in the US and foreign governments, including their respective
military organizations.  Many governments require that their employees use
*only* cryptographic software that uses approved standarized cryptographic 
algorithms and has undergone rigorous examination and testing for conformance 
to those approved standards.  In the USA those standards are called Federal
Information Processing Standards (FIPS).  Federal workers are required to use
ONLY certified implementations of FIPS-approved algorithms.  Many other 
governments recognize FIPS approval, and allow their own people to use FIPS
approved software as well.  (They figure, if it's good enough for the US 
military, then ...)

Mozilla's products are, today, recognized as approved for use by the US 
government, and others.  I believe we want to keep it that way.  This has 
implications on how we implement new cryptographic functionality.

To be recognized by those governments as acceptable software, a program 
(such as FireFox or ThunderBird) must do ALL of its cryptographic operations
(including random number generation) in a single (or limited set of) cryptographic "modules", which must ALL be FIPS approved, and it must access 
those modules only through the approved interfaces.  The cryptographic modules 
must be FIPS approved (or must have a FIPS-approved mode of operation)
and the application must not use ANY other cryptographic implementation(s) 
other than those approved modules.  

Today, NSS is (er, contains) a FIPS-approved module.  It has been FIPS 
approved 3 times before, and is currently undergoing its fourth FIPS 
evaluation.  Plans for the next FIPS evaluation are already underway.  

AFAIK, today, FireFox and ThunderBird do ALL of their cryptographic 
operations (including random number generation - when the random numbers
are to be used for security purposes) in NSS.  NSS has a "FIPS mode", and 
when used in FIPS mode, it does only FIPS-approved algorithms. (That is a
bit of an oversimplification, but suffices for this discussion.) A government
user can put his FireFox or Thunderbird into FIPS mode, and be assured that 
he is using software that meets the FIPS standards (and so he won't get in 
big trouble).

When NSS is put into FIPS mode, it stops doing operations that are not FIPS
approved.  (That is something of an oversimplification, but you get the idea.)
NSS acts as a gateway to all the cryptographic modules, so that it is able 
to control access to modules that are (or are not) FIPS approved.

Now, if other implementations of cryptographic algorithms are added to FireFox
or Thunderbird outside of NSS (or, not under the control of NSS), then it can
no longer be said that FireFox does all of its cryptographic operations inside
of FIPS approved modules.  In that case, even when the cryptographic modules
have all been put into FIPS mode, the presence of additional cryptographic
algorithm implementations outside of those FIPS-approved modules would 
disqualify the product for use by government users.  

So, I think that it is fairly important that all cryptographic algorithms be 
done in modules that can be put into a FIPS mode of operation, or can be
disabled when the product is operating in FIPS mode.  This argues strongly,
I think, for putting the complete SRP implementation (at least the 
cryptographic computations of SRP) into NSS.  

The point is NOT to say "don't implement non-FIPS algorithms", but rather 
is to ensure that the product has a mode of operation that uses ONLY those
approved algorithms, and that control of that mode is centralized in 
"modules" that can be inspected/approved.

Note that the interface to the freebl shared lib is considered a private 
interface of NSS, for use within NSS only.  The public APIs all exist at a
layer above PKCS#11.  That is, all public interfaces to cryptographic 
algorithms use the PKCS#11 cryptographic module API underneath to provide
access control to those cryptographic algorithms.  

Now, the decision about whether or not Mozilla software products, such as
FireFox and ThunderBird, want to retain their FIPS qualified status is not
up to the NSS team, but any change that would alter that FIPS-qualified 
status would seem to need to be made with the approval of Mozilla's 
powers-that-be.  

This is not an argument against implementing SRP.  I'm all in favor of 
implementing it.  It may even soon be standardized as an optional feature of
TLS (SSL 3.x).  But IMO, it would be best to implement it in such a way that
would allow FF and TB to retain their FIPS-qualified status.

The patch above seems like a good start in that direction.
(In reply to comment #14)  
> 
> AFAIK, today, FireFox and ThunderBird do ALL of their cryptographic 
> operations (including random number generation - when the random numbers
> are to be used for security purposes) in NSS. 

This is actually not /quite/ true, at least if I understand the definition of "cryptographic operations". Does it include things like HMAC? For instance, see

http://lxr.mozilla.org/mozilla/source/toolkit/components/url-classifier/content/url-crypto.js#325
http://lxr.mozilla.org/mozilla/source/mailnews/base/util/nsMsgUtils.cpp#949

> The presence of additional cryptographic
> algorithm implementations outside of those FIPS-approved modules would 
> disqualify the product for use by government users.  

That sounds like something we want to avoid, but have we already made trouble for ourselves?

> This is not an argument against implementing SRP.  I'm all in favor of 
> implementing it.  It may even soon be standardized as an optional feature of
> TLS (SSL 3.x).  But IMO, it would be best to implement it in such a way that
> would allow FF and TB to retain their FIPS-qualified status.

Agree.

Attached patch export srp from freebl (WIP) (obsolete) — Splinter Review
I think this is how its supposed to go. Missing versioning changes of course.
Attachment #273042 - Attachment is obsolete: true
In reply to comment 15:
Yes, HMAC is definitely a cryptographic algorithm for our purposes.

Mozilla may well have already gotten itself into trouble in this area, 
if it has allowed cryptographic code to be checked in in other areas.  
Perhaps not enough mozilla SRs know about these considerations.  :-/

There's some debate about whether other implementations are allowed in 
cases where the uses are not security sensitive (e.g. not for secrecy, 
confidentiality, authentication, integrity, etc.)  But I think clearly
all uses of SRP are security purposes.  
Attached patch export srp from freebl (WIP) (obsolete) — Splinter Review
Attachment #273069 - Attachment is obsolete: true
Comment on attachment 273069 [details] [diff] [review]
export srp from freebl (WIP)

Good stuff.  Here are some unsolicited review suggestions:   

>+SECStatus
>+SRP_CalculatePublicExponent(SECItem *prime,
>+                            SECItem *generator,
>+                            SECItem *privateExponent,
>+                            SECItem *publicExponent)

Declare input SECItems to be const (here, and in all new function 
declarations, definitions and typedefs).  e.g. 

>+SRP_CalculatePublicExponent(const SECItem *prime,
>+			      const SECItem *generator,
>+			      const SECItem *privateExponent,
>+			      SECItem *publicExponent);

>+{
>+    mp_int n, g, a, A;
>+    mp_err err = MP_OKAY;
>+    unsigned int len = 0;
>+    unsigned char *public = NULL;
>+    if (!prime || !generator || !privateExponent) {
>+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
>+        return SECFailure;
>+    }

Also check that publicExponent is non-NULL and that
publicExponent->data IS NULL.  

>+    /* number of bytes in the public exponent */
>+    len = mp_unsigned_octet_size(&A);
>+    /* allocate a buffer which can hold the exponent */
>+    public = PORT_Alloc(len);
>+    /* grab the exponent */
>+    err = mp_to_unsigned_octets(&A, public, len);
>+    if (err >= 0) err = MP_OKAY;
>+    SECITEM_AllocItem(NULL, publicExponent, len);
>+    memcpy(publicExponent->data, public, len);

I believe the entire block of code above is equivalent to this macro invocation
      MPINT_TO_SECITEM(&A, publicExponent, NULL) ;
and the macro obviates the pointer named "public", so the following 
will go away.

>+    if (public) {
>+        /* free the buffer allocated for the exponent. */
>+        PORT_ZFree(public, len);
>+    }

Similar changes can be made to SRP_CalculateM1.
Attachment #273069 - Attachment is obsolete: false
Attachment #273069 - Attachment is obsolete: true
Attachment #273074 - Attachment is obsolete: true
Comment on attachment 273090 [details] [diff] [review]
address nelson's comments

This is looking really good.  Two more changes:
1. You'll also need to bump the value of FREEBL_VERSION in loader.h
#define FREEBL_VERSION 0x0309
2. In the error paths of the new functions, after calling  
PORT_ZFree(sessionKey->data, set sessionKey->data back to NULL, 
so the caller won't attempt a double-free.

Also, I suggest adding another argument to the two new functions,
a PLArenaPool * that is passed as the third argument in your calls
to MPINT_TO_SECITEM.  This gives the caller an opportunity to 
allocate the data from an Arena Pool.  But this is PURELY optional.
(In reply to comment #21)
>  But this is PURELY optional.

It sounds totally required to me, if similar arguments have proven useful in other parts of NSS.

I will make the rest of the changes you mentioned. That said, I am totally bewildered by the NSS PKCS#11 code. What is the best way to become familiar with it? Even a recent bug that made additions to it could help.
This bug can probably be regarded as a duplicate of bug 405155?
(In reply to comment #23)
> This bug can probably be regarded as a duplicate of bug 405155?
> 

No.
The NSS portion of this bug has moved to bug 405155.
It seems to me that the work that remains is:
- protocol work in various protocols (http, IMAP, SMTP, etc.)
- UI work (to ask the user for that password)
- password manager integration (user might want FF to remember his SRP password).

I guess the remaining purpose of this bug is to track that other work.

Given the complaining that Mozilla is doing about the size of NSS, we 
definitely do not want to add SRP to NSS if Mozilla won't use it. 
I think the NSS team will do no more work on bug 405155 until there are some
signs of progress on the items I named just above.
Blocks: 405155
(In reply to comment #25)
> The NSS portion of this bug has moved to bug 405155.
> 
> Given the complaining that Mozilla is doing about the size of NSS, we 
> definitely do not want to add SRP to NSS if Mozilla won't use it. 

So, code size is always a trade-off. The complaining you are hearing concerns size vs. perceived benefit, and many Mozilla contributors disagree. It doesn't look like SRP will require anything close to the size of libpkix, the code needed to support EV and such. Am I mistaken?
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: