Open Bug 210941 Opened 21 years ago Updated 2 years ago

Redefine NSS nicknames to identify certs unambiguously

Categories

(NSS :: Libraries, enhancement, P2)

Product:
NSS
Component:
Libraries
Type:
enhancement

Tracking

(Not tracked)

People

(Reporter: nelson, Assigned: rrelyea)

Details

This RFE is filed on behalf of thomask@netscape.com, and the CMS team.

Today, NSS nicknames identify a subject name, not an individual cert.
If a user has multiple certs with the same subject name, there is only one
nickname that identifies all those certs.  The nickname does not unambiguously
identify any one of the certs that share that subject name.

When an NSS application calls functions such as CERT_FindCertByNickname or
CERT_FindUserCertByUsage to obtain a cert (say, to be used as an SSL server 
cert or SSL client auth cert) NSS picks the most recently issued cert (that 
is currently within its validity period) with that nickname.  
The cert chosen this way is ALWAYS a valid cert for use at the present time.
The cert returned by CERT_FindUserCertByUsage is always value for the specified
use at the present time.  But nonetheless, the user may wish to use a different
specific cert than the one chosen by the NSS functions named above.

So, Thomas proposes to change the operation of functions like those named 
above to make is possible to unambiguously identify a cert through those APIs.  
The idea is to effect this change without needing to change the products that
use NSS.  A new NSS shared library could allow users of existing server and
client products to simply change the strings they use, and be able to identify
specific particular certs.

The proposal is to have these functions first behave exactly as they do now, 
so as to maximize backward compatibility.  Then if the cert search fails to 
find a nickname match, parse the nickname according to an alternative syntax,
and look for a cert according to that information.  There have been several
proposals for alternative nickname syntax, including

Proposal 1) An RFC 2253 ASCII-encoded issuer name followed by an ASCII encoded
serial number.  
Proposal 2) An NSS nickname (as today) followed by " serial#" and a serial
number in ASCII encoded decimal.  
Proposal 3) An NSS nickname (as today) followed by an expiration date expressed
as an ASCII encoded generalTime.
See bug 210709 and the bugs on which it depends to see why I think Proposal 1
is not a good idea.  
Hmm, just looked at NES's code, look like they do their own version of find
certificate by nickname. I will work with to NES to solve this problem



CERTCertificate* SSLSocketConfiguration :: FindServerCertFromNickname(const
char* name) const
{
    CERTCertList* clist;
    clist = PK11_ListCerts(PK11CertListUser, NULL);

    CERTCertListNode *cln;

    for (cln = CERT_LIST_HEAD(clist); !CERT_LIST_END(cln,clist);
        cln = CERT_LIST_NEXT(cln))
    {
        CERTCertificate* cert = cln->cert;
        const char* nickname = (const char*) cln->appData;
        if (!nickname)
        {
            nickname = cert->nickname;
        };
        if (0==strcmp(name, nickname))
        {
            // we found our cert
            return cert;
        };
    };
    return NULL;
};

void SSLSocketConfiguration :: set_cert_and_key()
{
    SECStatus secstatus = SECFailure;

    // check the certificate
    SECCertTimeValidity certtimestatus;

    // Get own certificate and private key

    // later, we may need to do do a PK11_FindCertsFromNickname (note the plural)
    // because there may be several certs under the same nickname,
    // * for example in the case of certs signed by distinct CA
    // * there can also be certs with different expiration dates, but the singular
    // PK11_FindCertFromNickname always returns the most recent cert so we are OK
    // for that case, except if a cert that isn't yet valid has been installed ...
    // * DH support : if both RSA and DSA certs need to be installed on the same
    // server, we'll actually need to set both types certs on the socket ...

    servercert = FindServerCertFromNickname((char*)servercertnickname.data())
  ...
For reference, here is the NES bug filed

https://blackflag.mcom.com/show_bug.cgi?id=623094

Independently, I suggest we put the change into NSS:

certutil is currently using PK11_FindCertsFromNickname()

In mozilla/security/nss/lib/pk11wrap/pk11cert.c

static long pk11_getSerialNumberFromNickname(char *nickname) {
    /* Alternative Nickname Format: <nickname> <serial># */
    int len = 0;
    int i;
    char *nicknameDup = NULL;
    long rv = -1;
    int isNum = 0;

    nicknameDup = PORT_Strdup(nickname);
    len = PORT_Strlen(nicknameDup);
    if (len <= 0) {
      goto done;
    }
    if (nicknameDup[len-1] == '#') {
      nicknameDup[len-1] = '\0';
      for (i=len-2; i >= 0; i--) {
        if (nicknameDup[i] == ' ') {
          if (isNum) {
            rv = atol(nicknameDup+i+1);
          }
          goto done;
        }
        if (nicknameDup[i] < '0' || nicknameDup[i] > '9') {
           goto done;
        } else {
          isNum = 1;
        }
      }
    }

done:
    if (nicknameDup)
       PORT_Free(nicknameDup);

    return rv;
}

static CERTCertificate *
pk11_FindCertFromNicknameWithSN(char *nickname, void *wincx) {

    char *nicknameDup = NULL;
    int i;
    long sn = -1;
    long certSN = -1;
    CERTCertificate *rvCert = NULL;
    CERTCertList *certList = NULL;
    CERTCertListNode *node = NULL;

    sn = pk11_getSerialNumberFromNickname(nickname);
    if (sn == -1)
        goto done; /* error */

    nicknameDup = PORT_Strdup(nickname);
    for (i = PORT_Strlen(nicknameDup); i >= 0; i--) {
       if (nicknameDup[i] == ' ') {
         nicknameDup[i] = '\0';
         goto next;
       }
    }

next:
    certList = PK11_FindCertsFromNickname(nicknameDup, wincx);
    if (certList == NULL) {
        goto done; /* error */
    }

    for (node = CERT_LIST_HEAD(certList);
         !CERT_LIST_END(node, certList);
         node = CERT_LIST_NEXT(node)) {
        CERTCertificate *cert = node->cert;
        certSN = DER_GetInteger(&cert->serialNumber);
        if (certSN == sn) {
           rvCert = CERT_DupCertificate(cert);
           goto done; /* success */
        }
    }

done:
    if (nicknameDup)
       PORT_Free(nicknameDup);
    if (certList)
       CERT_DestroyCertList(certList);

    return rvCert;
}

CERTCertificate *
PK11_FindCertFromNickname(char *nickname, void *wincx) {

...
    }
    /* Test if we are using the new serial number format */
    if (rvCert == NULL && pk11_getSerialNumberFromNickname(nickname) != -1) {
      rvCert = pk11_FindCertFromNicknameWithSN(nickname, wincx);
    }
    if (slot) {
        PK11_FreeSlot(slot);
    }
    if (nickCopy) PORT_Free(nickCopy);
    return rvCert;
loser:
    if (slot) {
        PK11_FreeSlot(slot);
    }
    if (nickCopy) PORT_Free(nickCopy);
    return NULL;
#endif
}







Thomas,

Before you "fix" NES, let me explain why I had to implement my own version of
findcertbynickname.

The problem was that an identical cert could exist in multiple tokens, along
with multiple private keys. Even though the NSS cert nickname specified the slot
(token:nickname), the NSS APIs could return the cert structure with an
undetermined slot, if there were multiple copies of the cert.

Only NSS 4.0 could solve this problem with the API. Of course, this would be a
major change for all NSS applications, and not just a simple recompile.

If we change the definition of an NSS nickname to map to a unique cert rather
than any cert matching a subject, we will still have to deal with the token
issue. This isn't just a corner case for servers. Currently for instance, I have
my email certs and private keys living both on a smartcard and in the softoken.

Also, I think it is likely that some applications that are aware of the NSS
indexing by subject, eg. PSM and possibly parts of the NES admin, will break if
we make this change.

Note that I'm not at all saying we shouldn't change it, but that this is a major
change that is probably best dealt with in NSS 4.0 . Since we are now working on
NSS 3.9, it seems like an appropriate time to start seriously scheduling NSS 4.0
 work.
Priority: -- → P2
Target Milestone: --- → 3.9
Bob, please work with Steve and Thomas to resolve this RFE.
Assignee: wchang0222 → rrelyea0264
We should not change the semantics of the current find cert by nickname call.

Adding a new function with a new name may be reasonable.

Applications can get this current functionality without changes to NSS by
creating their own FindCertByNicknameWithSerial(). 
Call CERT_CreateNicknameCertList() for the nickname part.
walk the CertList looking for a cert with the requested serial number.

NOTE: This API is *not* guarrenteed to identify a unique cert. nickname the
nickname of the subject, not the issuer. Two certs may have the same subject and
same serial number with different issuers.

bob
Bob, the purpose of this RFE is to avoid having to build new binaries of 
certain old NSS-based server products.  The argument is that if the syntax
accepted by NSS functions that use nicknames is extended to facilitate 
unique cert selection, then those old servers can be used with new NSS 
libs, without rebuilding the servers.  
The problem is the only way to get that is to also change PK11_ListCerts to
return these kinds of nicknames. That would break existing apps as well. Since
it already requires a rebuild of those servers, then the problem should be fixed
correctly.
I agree with Bob. There is no "fix" for this that can be done only in NSS. Some
changes will have to be made to the server applications.

Can we not localize the change to PK11_FindCertFromNickname? I understand that 
DS server team uses this function to retrieve the server certificate for SSL 
certificate. If we worry about consistent issue, then maybe we should introduce 
a new function PK11_FindCertFromSuperNickname() as suggested. Then, in the long 
run, all server should use this PK11_FindCertFromSuperNickname().

For server certificate selection in SSL configuration, we should provide 
automatic and manual configuration. Maybe by default and normal circumstances, 
we will use the automatic certificate selection provided in 
PK11_FindCertFromNickname(). But in some scenario, administrators do want to be 
able to pick one particular certificate for whatever reason. Currently, we do 
not provide that functionality in NSS. I agree that the application that 
implement this feature. But if this is useful to all server product, why dont we 
put this feature in NSS?

Providing a brand new infrastructure to support manual selection could be a lot 
of work. That is why I proposed to have a way to leverage the existing API.
Thomas,

Even if we localize the change to PK11_FindCertFromNickname, the web server
still won't work with that sole change, because it uses PK11_ListCerts to locate
certs, not PK11_FindCertFromNickname.

PK11_ListCerts returns a list of certs with their nickname. If we change it to
return a nickname + SN, existing applications, including NES, will be broken by
that change.

So we would have to change the web server anyway. It can't work with just an NSS
patch to PK11_FindCertFromNickname. DS may be different. I don't know about
other products. I suspect there is no one single rule here as far as the
certificate search goes. There are lots of different ways to find certificates
in NSS. nickname is only one of them. Other applications have moved away from
using nicknames completely.

It is not accurate to say that we don't provide the function you want in NSS.
We don't provide a one-line function call to do what you want. However, you can
can certainly already perform the search you want by getting a subject list of
all the certs under the given nickname, and then looking at their serial number
and filter to the specific one that you need. There is nothing in NSS that stops
you from implementing what you need today.

However, what you really want is to add functionality to a number of different
NSS-based applications, and you believe that a change to NSS will automatically
do that. My take on this, is that it will not. I have been trying to tell you
that in our previous meeting on the issue, as Bob is telling you now.

I have no problem with adding a one-line function that does the search that you
want, so that you can modify all the applications that need the new behavior to
call this new cert search function.

In standard NSS terminology, we would probably not modify the format of the
nickname string, but instead have a function called
"PK11_FindCertByNicknameAndSN" which would take 2 arguments - one for the
nickname and SN. If we add this new function to do the search, it would be my
preference to use that sort of prototype.
However, I don't have a problem with putting the 2 arguments in a single string
together as long as that argument is not called a nickname. I don't particularly
like "SuperNickname", but we can probably find something less confusing.

No, I don't think we never asked for any change to PK11_ListCerts. That function
should continue to return the plain nickname.

At least if we extend CERT_FindCertByNickname, users can override the nickname
in their applications config file to add the serial number.



By the way - if we were able to make more extensive changes to the servers
handling of certificate configuration, it would be to remove support for
nicknames completely. They seem to cause far more confusion than is necessary.

Steve,

Once more ...

NES uses PK11_ListCerts to locate certs, not PK11_FindCertFromNickname. It works
by enumerating the certs into a list, then matching the nickname string in the
list against the string in the config file.

This is why changes to CERT_FindCertFromNickname, or PK11_FindCertByNickname,
won't help this particular application. There may be other cases.
If we make the change, I don't think you will get all the results you expect.

IMO we should not be lazy about this problem, because it will not work as you
expect. We should look at each application that needs this new functionality,
see exactly which functions they call, and modify them to call the new search
function where needed. If we keep the 2 arguments in one string, the application
changes should not be expensive - essentially renaming a couple of function
calls. However the applications code does need to be reviewed.

My issue is PK11_FindCertByNickname() has a semantic and a meaning. You want a
new function with a different semantic. This new function needs to be distinct.

Where it goes is 6 of one half a dozen of another. There is definately server
specific code in NSS that clients never use. There is also shared code
servercore that implements functions unique to the way servers use NSS. This can
be implemented in either place.
--------------------
So here's one proposal:

We provide two new functions:
char *PK11_GetStringForUniqueCert(CERTCertificate *);
CERTCertificate *PK11_FindUniqueCertByString(const char *string);

under the covers the string is the nickname and one of:
Issue date
serial number
certificate hash

I personally prefer certificate hash, since it's the only one guarrenteed to be
unique, but we can discuss which it is. It is even possible that
PK11_FindUniqueCertByString() will accept any of the three properly identified.

Example: "ServerCert serial:05:06:7d:54:45:80:12" (assuming a verisign cert)

bob

RE: Steve and removing nicknames: That is certainly doable, and probably
desirable. It doesn't require any changes to NSS. Other apps have done this already.

bob
Is certificate hash being displayed by certutil? If it is not currently 
displayed by our utility, people may have a hard time coming up the hash to do 
the listing.
Yes, certificate hash can be displayed in certutil,
but certutil in all NSS releases <= 3.8.2 displays
the wrong hash (bug 220016).  This bug was recently
fixed by Nelson in the upcoming NSS 3.9 release.
Moved to NSS 3.10.

Should we resolve this bug WONTFIX or implement Bob's
alternate proposal in comment 16?
Target Milestone: 3.9 → 3.10
QA Contact: bishakhabanerjee → jason.m.reid
Target Milestone: 3.10 → 3.12
QA Contact: jason.m.reid → libraries
Target Milestone: 3.12 → 3.12.2
Unsetting target milestone in unresolved bugs whose targets have passed.
Target Milestone: 3.12.2 → ---
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.