1049435 - importing an RSA private key fails if p < q

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[IIDA Yosiaki]

Attachment: data.tar.gz

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.0; Trident/4.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.0.30729; .NET CLR 3.5.30729; InfoPath.3)

Steps to reproduce:

Run 31.0, 31.0esr or 24.7.0esr of Firefox.
Select Tools(T) from menu bar.
Select Options(O) from menu items.
Select [Advanced] tab.
Select [Certificates] tab.
Push [View Certificates] button.
Select [Your Certificates] tab.
Push [Import...] button.
Select Unimportable.p12 file in attached tar ball.
Type c c w 3 3 t 4 a in password.
Push [OK].



Actual results:

Following message will appear in dialog:
  The PKCS #12 operation failed for unknown reasons.
On the other hand,
* "openssl pkcs12" can extract key and certificates.
* "openssl verify" can verify signature in EE certificate.



Expected results:

Key and certificates should be imported.

Comment 1

[Loic]

Dupe of bug 1048922?

Comment 2

[:Gijs (he/him)]

(In reply to Loic from comment #1)
> Dupe of bug 1048922?

Yes, but this has an actual example, so I'd rather forward-dupe in this case.

Comment 4

[:Gijs (he/him)]

Confirming per the dupe.

(In reply to IIDA Yosiaki from comment #0)
> Run 31.0, 31.0esr or 24.7.0esr of Firefox.

Out of curiosity, did this used to work on Firefox 30 or before?

Comment 5

[IIDA Yosiaki]

(In reply to :Gijs Kruitbosch from comment #4)
> Out of curiosity, did this used to work on Firefox 30 or before?

30 works fine with me.
FYI, I tried both en-US and ja-JP for all four versions which I mentioned and I found that language doen not matter.

Comment 6

[:Gijs (he/him)]

From the STR:

(In reply to IIDA Yosiaki from comment #0)
> Type c c w 3 3 t 4 a in password.

NB, this seems to be different - there's a txt file in the tarball which has the correct password.

Turning off PKIX verification doesn't seem to fix this issue... David, any idea what's happening here? :-)

Comment 7

[Loic]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Reg range:
good=2014-06-15
bad=2014-06-16
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6a457fe2a2a&tochange=80431d4fd0da

My guess is that the regression is in the new NSS lib which has been backported from FF33 to FF31:
Wan-Teh Chang — Bug 1020695: Update Mozilla to use NSS 3.16.2 Beta 4. Includes fixes for bug 1013088, bug 996237, bug 970539, bug 1016567, bug 485732, bug 334013, bug 959864, bug 1016836, bug 1016811, bug 1018536, bug 996250, bug 1009227, bug 963150, bug 1007126, bug 1021102.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Adding Wan-Teh as CC.
Tracking because security matters.

Does it affect 32, 33 and 34 too?

Comment 9

[Loic]

(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> Does it affect 32, 33 and 34 too?

I tested with FF34 and same issue.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

OK, Merci Loïc. I guess we can extrapolate that 32 and 33 are affected too.

Comment 11

[:Gijs (he/him)]

Redirecting the needinfo. This might need to move to NSS?

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Looks like the private RSA key in that file has p < q, whereas bug 1021102 changed the implementation to return an error if this is the case (previously, the implementation would try to switch them, but this led to other errors due to aliasing).

Comment 13

[IIDA Yosiaki]

We have three other unimportable P12 files whose private key has p < q, while nine importable P12 files have keys with q < p.
Due to security reason, we cannot offer the above unimportable P12.

Comment 14

[Loic]

A user complained about this issue on the French support board too:
http://forums.mozfr.org/viewtopic.php?f=5&t=119489

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

David - Do you want to take this bug or can you find an owner? If we want a fix for Firefox 32 we need it before Thu.

Comment 16

[Dana Keeler (she/her) (use needinfo) [:keeler]]

This is an NSS bug. It's actually pretty straight-forward (see comment 12), but I don't think I would have a solution I'm confident in by Thursday (at this point, I'm not sure anyone would). Maybe Wan-Teh or Richard can have a look?

Comment 17

[Ryan Sleevi]

Such .p12 are not valid/conforming to RFC3447, which is what led to the stricter check.

It's really a question of balancing tradeoffs. For Web Crypto, we (Richard, myself) wanted to conform to the spec, and thus prevent invalid inputs.

If NSS wants to allow imports of such nonconforming keys, we may need to introduce an additional API. Or Mozilla could decide to allow them (Chromium won't, because its important to be spec conforming, especially this early in implementations)

Comment 18

[Lawrence Mandel [:lmandel] (use needinfo)]

From comment 16 and comment 17, this bug sounds like a won't fix for Firefox 32. I'm leaving this bug tracked for 33 and trust we can come to a decision on how to proceed in the next couple of weeks while 33 is Aurora.

David - Can you drive this one or should Wan-Teh or Richard do that?

Comment 19

[Richard Barnes [:rbarnes]]

+1 to WONTFIX, for the reasons rsleevi listed.

Comment 21

[GB]

Hello,

1) I have imported a not importable p12 using the command line pk12util
2) I have exported the p12 from firefox in pkcs#12 format
3) I have deleted the keys imported
4) I have successfully imported the exported p12 into firefox

I have check the two private keys by exporting a pem file using openssl pkcs12, and then dump output using:

openssl rsa -text -in failkey.pem
openssl rsa -text -in "working key.pem"

the reason it works when you follow the process is that firefox is changing the two primes (p,q) around when the export is done.

####Failed key######

$ openssl rsa -text -in failkey.pem

Enter pass phrase for failkey.pem:

Private-Key: (2048 bit)

modulus:

    00:c1:1f:95:4b:28:d1:f4:1e:25:1c:df:de:5d:dc:

    49:ce:2b:f7:8e:30:59:bd:cd:bf:71:9f:85:96:c3:

    a6:bd:3c:6a:20:54:48:c7:e0:92:b8:2f:40:60:a0:

    d8:5a:f3:01:ec:95:81:76:3a:91:e2:48:8d:47:57:

    4c:0b:41:c0:cb:70:45:38:4a:f3:ea:32:b6:2c:72:

    64:0f:08:f0:cd:a3:d3:f9:ff:20:15:6f:54:9d:8c:

    ab:b5:af:8b:d7:f4:9c:7c:f0:d9:2a:89:14:54:ab:

    ff:a5:0f:73:b6:98:0f:14:13:ad:6a:a8:0e:68:fc:

    cf:ca:75:47:6f:f9:20:28:0d:3a:4f:d8:df:20:53:

    d3:ee:be:72:38:88:2c:66:64:21:2e:4b:46:3c:d6:

    37:44:d2:44:fe:6d:2f:8b:ce:1a:a6:b9:34:85:bc:

    4a:e4:cc:e9:46:91:08:23:21:f6:89:fd:88:21:dd:

    77:15:53:ab:d3:5a:f3:7f:c4:a8:b5:dd:60:96:4f:

    c1:84:5f:44:df:ff:a9:05:84:03:34:7c:20:fb:a9:

    2a:eb:84:b3:2c:61:30:d3:3c:f7:0b:f6:e0:55:fc:

    c0:2c:08:36:9b:88:59:01:84:e4:8a:ce:f6:01:8d:

    80:91:26:95:f5:15:9b:21:76:ba:11:7f:cf:64:91:

    2b:45

publicExponent: 65537 (0x10001)

privateExponent:

    26:fe:aa:95:87:83:85:48:44:f4:24:9b:f0:d5:d7:

    2d:43:21:74:5b:7c:f3:5f:45:05:bb:51:2d:15:0a:

    68:f1:76:f8:5a:2c:6b:f7:83:88:9f:39:df:88:fa:

    c6:ba:84:ab:f1:b0:37:60:32:2e:bf:b9:8f:2b:28:

    56:a9:8b:35:48:d1:21:41:b9:28:93:de:c6:f0:be:

    15:6f:17:b8:5e:19:22:44:8a:84:e9:7a:eb:69:09:

    0e:e1:88:5a:2d:b1:1f:65:3e:64:61:53:72:99:5c:

    40:46:f3:75:6e:14:b5:58:17:cf:0e:6a:02:76:f1:

    ba:0e:9e:43:14:c9:92:6c:66:d5:73:9c:58:33:43:

    0d:54:03:31:98:b0:b3:2b:fe:3e:7a:18:11:80:f5:

    39:81:50:72:22:d0:a3:d7:cc:20:73:fe:8e:0f:24:

    75:21:22:70:99:56:c0:2d:e9:81:cd:d9:56:40:25:

    7e:51:c6:15:3a:26:59:97:86:83:f7:90:73:f9:77:

    f2:6f:dd:6a:25:46:6c:06:00:12:4d:68:c3:ef:9c:

    55:1d:a1:6f:49:46:5b:68:ac:1e:8c:1d:1d:41:3e:

    68:96:92:2f:32:57:0b:87:1b:ac:c4:8f:21:9a:04:

    d7:85:9d:e1:18:6f:5c:3f:7c:6c:53:37:c4:32:66:

    2d

prime1:

    00:c4:10:0b:06:49:b6:bd:55:34:62:74:05:0b:00:

    12:6e:90:a1:55:ec:3f:00:b5:a5:dd:21:ff:ff:2a:

    6b:79:bb:76:df:64:b7:b1:58:be:65:c9:e8:0f:f3:

    50:6a:ba:ee:7d:e1:73:7f:60:c4:b2:27:9d:f0:83:

    a8:e8:7e:1c:ca:57:46:80:ee:5a:44:ca:f3:55:c6:

    2e:1f:a1:ae:79:f4:c6:a7:c3:a5:15:cb:10:16:a4:

    5b:62:9a:c1:e9:6b:b4:ab:4b:32:c7:b3:f3:15:c0:

    b1:32:73:5f:56:51:47:92:0d:e5:af:e4:3f:8f:9d:

    63:5c:ea:57:b4:75:bb:51:07

prime2:

    00:fc:29:82:67:39:b5:93:2c:81:a2:dc:0e:db:9a:

    80:05:b7:22:a9:f6:75:89:a8:ae:0a:96:5a:0f:7f:

    e2:80:e0:bb:97:27:86:20:b5:99:7b:31:5f:10:07:

    a4:f0:f8:ea:6a:88:59:cc:b5:de:6b:76:25:86:4e:

    bc:5b:11:d0:67:48:ba:d3:ac:18:b2:6d:ca:d4:55:

    ab:e5:c5:e2:bc:e1:76:94:02:d3:d2:99:f7:5d:74:

    2d:2c:11:8e:cf:d9:6c:b5:90:dc:08:d2:90:49:45:

    b2:f5:83:66:2f:62:67:81:cc:a9:3d:af:fc:6f:e7:

    ff:9e:50:00:8a:af:2f:6a:53

exponent1:

    62:95:52:85:c9:e0:d9:d8:92:eb:82:3c:da:e8:21:

    5c:78:da:b6:b2:80:87:61:ce:d1:9e:fb:f2:98:a8:

    cb:df:e8:08:b1:c2:ef:a8:98:ab:e3:d7:0c:d6:22:

    34:58:63:fc:e5:b4:c6:72:a8:d4:8f:b9:09:ab:99:

    ed:b5:23:d2:d6:09:7c:60:dd:00:c4:2f:90:8c:82:

    ba:a2:f8:71:18:14:1f:5c:ef:90:42:b7:87:3b:03:

    3c:54:66:76:71:12:ba:22:a8:98:e3:b1:b1:d9:5e:

    ff:e8:25:22:e8:e1:9c:dd:e2:05:0c:36:ad:86:cc:

    e4:76:6c:bd:2f:89:8d:57

exponent2:

    00:ba:48:74:04:8c:16:7e:8e:2f:8a:bf:a4:de:48:

    c9:f0:ee:f4:d5:b8:b3:e6:29:4d:c1:96:87:1b:d2:

    2e:e3:64:a2:50:ad:2b:22:38:e6:14:a3:49:86:0f:

    0f:a3:d1:4f:63:ba:2d:14:d4:fa:66:4a:d6:b7:dc:

    ac:bb:5e:72:a6:0a:8d:b2:57:fd:ba:ba:ef:4f:63:

    a6:e4:cd:06:8e:e0:c4:f7:dd:0a:dd:17:4f:2d:a3:

    e8:c7:18:85:77:39:39:5c:fc:92:00:96:85:6f:0b:

    e5:84:08:39:52:22:11:33:4b:9d:6f:6b:f6:42:39:

    92:96:42:a5:d3:ce:4d:69:ef

coefficient:

    02:34:f1:8b:03:93:7e:19:6e:ee:c2:52:81:32:5c:

    35:eb:5e:7b:9f:61:57:23:73:de:e5:79:59:09:95:

    f1:68:cc:ed:62:63:f7:2b:68:b4:f1:a3:7e:18:91:

    82:2e:eb:25:9d:63:92:84:e2:4b:61:1c:2d:40:8d:

    5f:91:68:ce:01:2c:f0:65:db:64:14:ef:17:11:35:

    88:83:e4:c3:3e:ba:0f:5a:dc:b9:73:55:5c:41:11:

    5c:da:69:43:a2:09:99:d3:fe:c9:8c:53:05:b6:e1:

    54:51:4c:f7:63:05:b5:8e:c1:4d:2e:bd:d7:8e:f1:

    73:ca:1f:9b:bd:a1:8d:41

writing RSA key

-----BEGIN RSA PRIVATE KEY-----

MIIEowIBAAKCAQEAwR+VSyjR9B4lHN/eXdxJziv3jjBZvc2/cZ+FlsOmvTxqIFRI

x+CSuC9AYKDYWvMB7JWBdjqR4kiNR1dMC0HAy3BFOErz6jK2LHJkDwjwzaPT+f8g

FW9UnYyrta+L1/ScfPDZKokUVKv/pQ9ztpgPFBOtaqgOaPzPynVHb/kgKA06T9jf

IFPT7r5yOIgsZmQhLktGPNY3RNJE/m0vi84aprk0hbxK5MzpRpEIIyH2if2IId13

FVOr01rzf8Sotd1glk/BhF9E3/+pBYQDNHwg+6kq64SzLGEw0zz3C/bgVfzALAg2

m4hZAYTkis72AY2AkSaV9RWbIXa6EX/PZJErRQIDAQABAoIBACb+qpWHg4VIRPQk

m/DV1y1DIXRbfPNfRQW7US0VCmjxdvhaLGv3g4ifOd+I+sa6hKvxsDdgMi6/uY8r

KFapizVI0SFBuSiT3sbwvhVvF7heGSJEioTpeutpCQ7hiFotsR9lPmRhU3KZXEBG

83VuFLVYF88OagJ28boOnkMUyZJsZtVznFgzQw1UAzGYsLMr/j56GBGA9TmBUHIi

0KPXzCBz/o4PJHUhInCZVsAt6YHN2VZAJX5RxhU6JlmXhoP3kHP5d/Jv3WolRmwG

ABJNaMPvnFUdoW9JRltorB6MHR1BPmiWki8yVwuHG6zEjyGaBNeFneEYb1w/fGxT

N8QyZi0CgYEAxBALBkm2vVU0YnQFCwASbpChVew/ALWl3SH//yprebt232S3sVi+

ZcnoD/NQarrufeFzf2DEsied8IOo6H4cyldGgO5aRMrzVcYuH6GuefTGp8OlFcsQ

FqRbYprB6Wu0q0syx7PzFcCxMnNfVlFHkg3lr+Q/j51jXOpXtHW7UQcCgYEA/CmC

Zzm1kyyBotwO25qABbciqfZ1iaiuCpZaD3/igOC7lyeGILWZezFfEAek8PjqaohZ

zLXea3Ylhk68WxHQZ0i606wYsm3K1FWr5cXivOF2lALT0pn3XXQtLBGOz9lstZDc

CNKQSUWy9YNmL2JngcypPa/8b+f/nlAAiq8valMCgYBilVKFyeDZ2JLrgjza6CFc

eNq2soCHYc7RnvvymKjL3+gIscLvqJir49cM1iI0WGP85bTGcqjUj7kJq5nttSPS

1gl8YN0AxC+QjIK6ovhxGBQfXO+QQreHOwM8VGZ2cRK6IqiY47Gx2V7/6CUi6OGc

3eIFDDathszkdmy9L4mNVwKBgQC6SHQEjBZ+ji+Kv6TeSMnw7vTVuLPmKU3Blocb

0i7jZKJQrSsiOOYUo0mGDw+j0U9jui0U1PpmSta33Ky7XnKmCo2yV/26uu9PY6bk

zQaO4MT33QrdF08to+jHGIV3OTlc/JIAloVvC+WECDlSIhEzS51va/ZCOZKWQqXT

zk1p7wKBgAI08YsDk34Zbu7CUoEyXDXrXnufYVcjc97leVkJlfFozO1iY/craLTx

o34YkYIu6yWdY5KE4kthHC1AjV+RaM4BLPBl22QU7xcRNYiD5MM+ug9a3LlzVVxB

EVzaaUOiCZnT/smMUwW24VRRTPdjBbWOwU0uvdeO8XPKH5u9oY1B

-----END RSA PRIVATE KEY-----

 

 

##### Same Key but working

 

$ openssl rsa -text -in "working key.pem"

Enter pass phrase for working key.pem:

Private-Key: (2048 bit)

modulus:

    00:c1:1f:95:4b:28:d1:f4:1e:25:1c:df:de:5d:dc:

    49:ce:2b:f7:8e:30:59:bd:cd:bf:71:9f:85:96:c3:

    a6:bd:3c:6a:20:54:48:c7:e0:92:b8:2f:40:60:a0:

    d8:5a:f3:01:ec:95:81:76:3a:91:e2:48:8d:47:57:

    4c:0b:41:c0:cb:70:45:38:4a:f3:ea:32:b6:2c:72:

    64:0f:08:f0:cd:a3:d3:f9:ff:20:15:6f:54:9d:8c:

    ab:b5:af:8b:d7:f4:9c:7c:f0:d9:2a:89:14:54:ab:

    ff:a5:0f:73:b6:98:0f:14:13:ad:6a:a8:0e:68:fc:

    cf:ca:75:47:6f:f9:20:28:0d:3a:4f:d8:df:20:53:

    d3:ee:be:72:38:88:2c:66:64:21:2e:4b:46:3c:d6:

    37:44:d2:44:fe:6d:2f:8b:ce:1a:a6:b9:34:85:bc:

    4a:e4:cc:e9:46:91:08:23:21:f6:89:fd:88:21:dd:

    77:15:53:ab:d3:5a:f3:7f:c4:a8:b5:dd:60:96:4f:

    c1:84:5f:44:df:ff:a9:05:84:03:34:7c:20:fb:a9:

    2a:eb:84:b3:2c:61:30:d3:3c:f7:0b:f6:e0:55:fc:

    c0:2c:08:36:9b:88:59:01:84:e4:8a:ce:f6:01:8d:

    80:91:26:95:f5:15:9b:21:76:ba:11:7f:cf:64:91:

    2b:45

publicExponent: 65537 (0x10001)

privateExponent:

    26:fe:aa:95:87:83:85:48:44:f4:24:9b:f0:d5:d7:

    2d:43:21:74:5b:7c:f3:5f:45:05:bb:51:2d:15:0a:

    68:f1:76:f8:5a:2c:6b:f7:83:88:9f:39:df:88:fa:

    c6:ba:84:ab:f1:b0:37:60:32:2e:bf:b9:8f:2b:28:

    56:a9:8b:35:48:d1:21:41:b9:28:93:de:c6:f0:be:

    15:6f:17:b8:5e:19:22:44:8a:84:e9:7a:eb:69:09:

    0e:e1:88:5a:2d:b1:1f:65:3e:64:61:53:72:99:5c:

    40:46:f3:75:6e:14:b5:58:17:cf:0e:6a:02:76:f1:

    ba:0e:9e:43:14:c9:92:6c:66:d5:73:9c:58:33:43:

    0d:54:03:31:98:b0:b3:2b:fe:3e:7a:18:11:80:f5:

    39:81:50:72:22:d0:a3:d7:cc:20:73:fe:8e:0f:24:

    75:21:22:70:99:56:c0:2d:e9:81:cd:d9:56:40:25:

    7e:51:c6:15:3a:26:59:97:86:83:f7:90:73:f9:77:

    f2:6f:dd:6a:25:46:6c:06:00:12:4d:68:c3:ef:9c:

    55:1d:a1:6f:49:46:5b:68:ac:1e:8c:1d:1d:41:3e:

    68:96:92:2f:32:57:0b:87:1b:ac:c4:8f:21:9a:04:

    d7:85:9d:e1:18:6f:5c:3f:7c:6c:53:37:c4:32:66:

    2d

prime1:

    00:fc:29:82:67:39:b5:93:2c:81:a2:dc:0e:db:9a:

    80:05:b7:22:a9:f6:75:89:a8:ae:0a:96:5a:0f:7f:

    e2:80:e0:bb:97:27:86:20:b5:99:7b:31:5f:10:07:

    a4:f0:f8:ea:6a:88:59:cc:b5:de:6b:76:25:86:4e:

    bc:5b:11:d0:67:48:ba:d3:ac:18:b2:6d:ca:d4:55:

    ab:e5:c5:e2:bc:e1:76:94:02:d3:d2:99:f7:5d:74:

    2d:2c:11:8e:cf:d9:6c:b5:90:dc:08:d2:90:49:45:

    b2:f5:83:66:2f:62:67:81:cc:a9:3d:af:fc:6f:e7:

    ff:9e:50:00:8a:af:2f:6a:53

prime2:

    00:c4:10:0b:06:49:b6:bd:55:34:62:74:05:0b:00:

    12:6e:90:a1:55:ec:3f:00:b5:a5:dd:21:ff:ff:2a:

    6b:79:bb:76:df:64:b7:b1:58:be:65:c9:e8:0f:f3:

    50:6a:ba:ee:7d:e1:73:7f:60:c4:b2:27:9d:f0:83:

    a8:e8:7e:1c:ca:57:46:80:ee:5a:44:ca:f3:55:c6:

    2e:1f:a1:ae:79:f4:c6:a7:c3:a5:15:cb:10:16:a4:

    5b:62:9a:c1:e9:6b:b4:ab:4b:32:c7:b3:f3:15:c0:

    b1:32:73:5f:56:51:47:92:0d:e5:af:e4:3f:8f:9d:

    63:5c:ea:57:b4:75:bb:51:07

exponent1:

    00:ba:48:74:04:8c:16:7e:8e:2f:8a:bf:a4:de:48:

    c9:f0:ee:f4:d5:b8:b3:e6:29:4d:c1:96:87:1b:d2:

    2e:e3:64:a2:50:ad:2b:22:38:e6:14:a3:49:86:0f:

    0f:a3:d1:4f:63:ba:2d:14:d4:fa:66:4a:d6:b7:dc:

    ac:bb:5e:72:a6:0a:8d:b2:57:fd:ba:ba:ef:4f:63:

    a6:e4:cd:06:8e:e0:c4:f7:dd:0a:dd:17:4f:2d:a3:

    e8:c7:18:85:77:39:39:5c:fc:92:00:96:85:6f:0b:

    e5:84:08:39:52:22:11:33:4b:9d:6f:6b:f6:42:39:

    92:96:42:a5:d3:ce:4d:69:ef

exponent2:

    62:95:52:85:c9:e0:d9:d8:92:eb:82:3c:da:e8:21:

    5c:78:da:b6:b2:80:87:61:ce:d1:9e:fb:f2:98:a8:

    cb:df:e8:08:b1:c2:ef:a8:98:ab:e3:d7:0c:d6:22:

    34:58:63:fc:e5:b4:c6:72:a8:d4:8f:b9:09:ab:99:

    ed:b5:23:d2:d6:09:7c:60:dd:00:c4:2f:90:8c:82:

    ba:a2:f8:71:18:14:1f:5c:ef:90:42:b7:87:3b:03:

    3c:54:66:76:71:12:ba:22:a8:98:e3:b1:b1:d9:5e:

    ff:e8:25:22:e8:e1:9c:dd:e2:05:0c:36:ad:86:cc:

    e4:76:6c:bd:2f:89:8d:57

coefficient:

    00:f9:52:eb:1a:be:c8:f6:07:52:86:6d:b2:06:ca:

    c4:b5:5c:0f:c9:75:fd:ee:27:89:9d:f1:f8:4d:d1:

    20:4c:c8:0b:c2:ef:66:60:73:6e:57:3a:b2:65:9f:

    82:5c:4a:7e:19:59:94:20:a7:98:d4:1d:fc:c5:a4:

    27:17:f3:6a:9c:f8:32:3a:14:c7:5f:7d:ee:d2:d5:

    48:91:cf:5e:e6:ff:57:f1:16:02:96:04:a4:eb:a2:

    23:8f:f1:88:5b:f0:72:d5:20:24:b3:c8:2d:43:2c:

    fc:ba:e9:13:e4:fc:43:92:c3:a7:2f:b3:65:5d:71:

    89:dc:c2:f0:7d:9e:41:eb:75

writing RSA key

-----BEGIN RSA PRIVATE KEY-----

MIIEpAIBAAKCAQEAwR+VSyjR9B4lHN/eXdxJziv3jjBZvc2/cZ+FlsOmvTxqIFRI

x+CSuC9AYKDYWvMB7JWBdjqR4kiNR1dMC0HAy3BFOErz6jK2LHJkDwjwzaPT+f8g

FW9UnYyrta+L1/ScfPDZKokUVKv/pQ9ztpgPFBOtaqgOaPzPynVHb/kgKA06T9jf

IFPT7r5yOIgsZmQhLktGPNY3RNJE/m0vi84aprk0hbxK5MzpRpEIIyH2if2IId13

FVOr01rzf8Sotd1glk/BhF9E3/+pBYQDNHwg+6kq64SzLGEw0zz3C/bgVfzALAg2

m4hZAYTkis72AY2AkSaV9RWbIXa6EX/PZJErRQIDAQABAoIBACb+qpWHg4VIRPQk

m/DV1y1DIXRbfPNfRQW7US0VCmjxdvhaLGv3g4ifOd+I+sa6hKvxsDdgMi6/uY8r

KFapizVI0SFBuSiT3sbwvhVvF7heGSJEioTpeutpCQ7hiFotsR9lPmRhU3KZXEBG

83VuFLVYF88OagJ28boOnkMUyZJsZtVznFgzQw1UAzGYsLMr/j56GBGA9TmBUHIi

0KPXzCBz/o4PJHUhInCZVsAt6YHN2VZAJX5RxhU6JlmXhoP3kHP5d/Jv3WolRmwG

ABJNaMPvnFUdoW9JRltorB6MHR1BPmiWki8yVwuHG6zEjyGaBNeFneEYb1w/fGxT

N8QyZi0CgYEA/CmCZzm1kyyBotwO25qABbciqfZ1iaiuCpZaD3/igOC7lyeGILWZ

ezFfEAek8PjqaohZzLXea3Ylhk68WxHQZ0i606wYsm3K1FWr5cXivOF2lALT0pn3

XXQtLBGOz9lstZDcCNKQSUWy9YNmL2JngcypPa/8b+f/nlAAiq8valMCgYEAxBAL

Bkm2vVU0YnQFCwASbpChVew/ALWl3SH//yprebt232S3sVi+ZcnoD/NQarrufeFz

f2DEsied8IOo6H4cyldGgO5aRMrzVcYuH6GuefTGp8OlFcsQFqRbYprB6Wu0q0sy

x7PzFcCxMnNfVlFHkg3lr+Q/j51jXOpXtHW7UQcCgYEAukh0BIwWfo4vir+k3kjJ

8O701biz5ilNwZaHG9Iu42SiUK0rIjjmFKNJhg8Po9FPY7otFNT6ZkrWt9ysu15y

pgqNslf9urrvT2Om5M0GjuDE990K3RdPLaPoxxiFdzk5XPySAJaFbwvlhAg5UiIR

M0udb2v2QjmSlkKl085Nae8CgYBilVKFyeDZ2JLrgjza6CFceNq2soCHYc7Rnvvy

mKjL3+gIscLvqJir49cM1iI0WGP85bTGcqjUj7kJq5nttSPS1gl8YN0AxC+QjIK6

ovhxGBQfXO+QQreHOwM8VGZ2cRK6IqiY47Gx2V7/6CUi6OGc3eIFDDathszkdmy9

L4mNVwKBgQD5Uusavsj2B1KGbbIGysS1XA/Jdf3uJ4md8fhN0SBMyAvC72Zgc25X

OrJln4JcSn4ZWZQgp5jUHfzFpCcX82qc+DI6FMdffe7S1UiRz17m/1fxFgKWBKTr

oiOP8Yhb8HLVICSzyC1DLPy66RPk/EOSw6cvs2VdcYncwvB9nkHrdQ==

-----END RSA PRIVATE KEY-----



If I am not wrong, In the RSA algorithm, p and q (prime1 and prime2) are interchangeable (as you can see from the dump before).
from what I have read here it seem that p<q is a correct check,
but if p and q are are interchangeable, why p<q is checked?

Regards
GB

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Brian, do you think it should be a wontfix? Thanks

Comment 24

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> Brian, do you think it should be a wontfix? Thanks

I don't have an opinion on this.

Comment 25

[lists]

The problem is that Microsoft's CA authority on Windows Active Directory genereates p12 files this way.  Now since FF >31, we can no longer import any of our client-side certificates into Firefox. 

I have no control over how Microsoft writes its p12 files, Rather than a won't fix, could you make it optional (strict) default, or (RSA-compaitble) optional?  

Does anyone know of a way to modify the p12 file to switch p & q so that this new restriction in FF can use P12 files that were previously valid, but are no longer?

Comment 26

[:Gijs (he/him)]

(In reply to lists from comment #25)
> Does anyone know of a way to modify the p12 file to switch p & q so that
> this new restriction in FF can use P12 files that were previously valid, but
> are no longer?

Download a copy of Firefox 30 or earlier, import the p12, and re-export it from there. You can probably use some kind of commandline tool provided by NSS or openssl, too, but I don't have the expertise for that.

Comment 27

[Sylvestre Ledru [:Sylvestre]]

Wontfix for 33. If there is a consensus on a general wontfix for this but, we should do it...

Comment 28

[AppleStock]

That is unexceptable.  We have to co-exist in a eco system of browsers and there is always a need for interoperability.  This has broken our enterprise solution where IE and Firefox were the premier browsers for the customer and now I have remove that as an option from the desktop.  I too have a CA that delivers the P12s in a format that on occasion have this flaw, but I have to support it until it is fixed.  Please make this an optional requirement.

Comment 29

[Wan-Teh Chang]

I remember we discussed this issue in a NSS conference call and
the decision was what I wrote in bug 1021102 comment 24:

  Let's be strict unless our tests show other major implementations accept
  such RSA private keys.

I wrote that comment at 2014-06-12 15:24:20 PDT. 2014-06-12 was
a Thursday. So most likely we discussed this issue in the NSS
conference call earlier that day.

So, now that we have found that Microsoft's CA authority on Windows
Active Directory genereates p12 files this way, we should revert the
strict check.

Comment 30

[Kaiwren]

I'm in total agreement with AppleStock.  This should be an optional requirement.  It doesn't make sense to deliver new functionality that breaks the norm of the community without supporting the current implementation.

Comment 31

[Wan-Teh Chang]

I will write a patch to address this issue. Can one of you provide
a sample PKCS #8 or PKCS #12 file generated by Microsoft's CA authority
on Windows Active Directory that the current version of Firefox and NSS
cannot import? Thanks.

Comment 32

[Wan-Teh Chang]

Attachment: Proposed patch

1. Change RSA_PrivateKeyCheck to not require p > q.

2. Change RSA_PrivateKeyCheck and rsa_build_from_primes (called
by RSA_NewKey and RSA_PopulatePrivateKey) to require p != q.

3. Continue to allow RSA_NewKey and RSA_PopulatePrivateKey to
force p > q, but add a comment to note that it is not necessary.

4. Remove unused variable prevbp in get_blinding_params.

Comment 33

[Wan-Teh Chang]

Rationale for removing the p > q check in RSA_PrivateKeyCheck:

1. PKCS #1 does not require p > q. The only requirement is that p and
q be distinct. For example,
http://tools.ietf.org/html/rfc3447#section-3.2 says:

   In a valid RSA private key with the first representation, the RSA
   modulus n is the same as in the corresponding RSA public key and is
   the product of u distinct odd primes r_i, i = 1, 2, ..., u, where u
   >= 2.  ...

   ...

   In a valid RSA private key with the second representation, the two
   factors p and q are the first two prime factors of the RSA modulus n
   (i.e., r_1 and r_2), ...

2. The p > q requirement is only necessary for implementations whose
bignum library has trouble handling a negative m_1 - m_2 in the CRT
calculation in RFC 3447, Section 5.1.2, Step 2.b.iii:

   Steps:

   1. If the ciphertext representative c is not between 0 and n - 1,
      output "ciphertext representative out of range" and stop.

   2. The message representative m is computed as follows.

      a. If the first form (n, d) of K is used, let m = c^d mod n.

      b. If the second form (p, q, dP, dQ, qInv) and (r_i, d_i, t_i)
         of K is used, proceed as follows:

         i.    Let m_1 = c^dP mod p and m_2 = c^dQ mod q.

         ii.   If u > 2, let m_i = c^(d_i) mod r_i, i = 3, ..., u.

         iii.  Let h = (m_1 - m_2) * qInv mod p.

         iv.   Let m = m_2 + q * h.

         v.    If u > 2, ...

If p > q, we can replace m_1 - m_2 with m_1 + p - m_2, which is
positive. This implementation trick is described in
http://www.di-mgt.com.au/rsa_alg.html (search for "p > q" in that page):

  [2008-09-02] Chris Becke has pointed out that most large integer
  packages will fail when computing h if m1 < m2. This can be easily
  fixed by computing

  h = qInv(m1 + p - m2) mod p

(OpenSSL uses this implementation trick. I don't know whether
OpenSSL's BN library has this issue or it's a performance
optimization.)

3. NSS doesn't use this implementation trick. See
http://mxr.mozilla.org/nss/ident?i=rsa_PrivateKeyOpCRTNoCheck:

932     /* 1. m1 = c**d_p mod p */
933     CHECK_MPI_OK( mp_mod(c, &p, &ctmp) );
934     CHECK_MPI_OK( mp_exptmod(&ctmp, &d_p, &p, &m1) );
935     /* 2. m2 = c**d_q mod q */
936     CHECK_MPI_OK( mp_mod(c, &q, &ctmp) );
937     CHECK_MPI_OK( mp_exptmod(&ctmp, &d_q, &q, &m2) );
938     /* 3.  h = (m1 - m2) * qInv mod p */
939     CHECK_MPI_OK( mp_submod(&m1, &m2, &p, &h) );
940     CHECK_MPI_OK( mp_mulmod(&h, &qInv, &p, &h)  );
941     /* 4.  m = m2 + h * q */
942     CHECK_MPI_OK( mp_mul(&h, &q, m) );
943     CHECK_MPI_OK( mp_add(m, &m2, m) );

By doing mp_submod instead of mp_sub on line 939, NSS ensures
the result of the subtraction is nonnegative.  Therefore NSS
doesn't need to require p > q.

Comment 34

[Richard Barnes [:rbarnes]]

Comment on attachment 8499932 [details] [diff] [review]
Proposed patch

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

::: lib/freebl/rsa.c
@@ +101,1 @@
>  		mp_int *e, PRBool needPublicExponent, 

Nit: Space at end of line.

@@ +117,5 @@
>      CHECK_MPI_OK( mp_init(&qsub1) );
>      CHECK_MPI_OK( mp_init(&tmp)   );
> +    /* p and q must be distinct. */
> +    if (mp_cmp(p, q) == 0) {
> +	PORT_SetError(SEC_ERROR_NEED_RANDOM);

Maybe SEC_ERROR_BAD_KEY?

@@ +1451,5 @@
>      /* d_q == d mod q-1 */
>      CHECK_MPI_OK( mp_mod(&d, &qsub1, &res) );
>      VERIFY_MPI_EQUAL(&res, &d_q);
>      /* q * q**-1 == 1 mod p */
> +    /* Should also check qInv is a positive integer less than p. */

Should this patch just implement this requirement?

> if ((mp_cmp_z(qInv) <= 0) || (mp_cmp(qInv, p) >= 0)) {
>   err = MP_NO;
>   goto cleanup;
> }

Comment 35

[Benjamin Kerensa [:bkerensa]]

Is there any workaround to this? It appears to be impacting our ESR users.

Comment 36

[Robert Relyea]

Comment on attachment 8499932 [details] [diff] [review]
Proposed patch

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

::: lib/freebl/rsa.c
@@ +117,5 @@
>      CHECK_MPI_OK( mp_init(&qsub1) );
>      CHECK_MPI_OK( mp_init(&tmp)   );
> +    /* p and q must be distinct. */
> +    if (mp_cmp(p, q) == 0) {
> +	PORT_SetError(SEC_ERROR_NEED_RANDOM);

If we are calling rsa_build_from_primes, then we are now just generating the key, which means we've generated p and q and they were the same, so we have a problem with the prng, so SEC_ERROR_NEED_RANDOM is appropriate.

@@ +288,5 @@
>  	CHECK_SEC_OK( generate_prime(&q, primeLen) );
> +	/* Assure p > q */
> +	/* NOTE: PKCS #1 does not require p > q, and NSS doesn't use any
> +	 * implementation optimization that requires p > q. We can remove
> +	 * this code in the future.

For primes we generate, we should still keep this relationship for compatibility reasons. We should verify that other standards don't require it (there are several including ISO etc.).

@@ +1451,5 @@
>      /* d_q == d mod q-1 */
>      CHECK_MPI_OK( mp_mod(&d, &qsub1, &res) );
>      VERIFY_MPI_EQUAL(&res, &d_q);
>      /* q * q**-1 == 1 mod p */
> +    /* Should also check qInv is a positive integer less than p. */

I think we only need this check if some validation requires it. as long as q*qInv mod p == 1, then there isn't anything mathematically wrong with qInv as far as NSS is concern.

Comment 38

[Mike Kaply [:mkaply]]

Can someone explain how this broke between ESR 24.5 and ESR 24.7?

Was NSS upgraded in the ESR? Why?

Comment 39

[Wan-Teh Chang]

Comment on attachment 8499932 [details] [diff] [review]
Proposed patch

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

::: lib/freebl/rsa.c
@@ +117,5 @@
>      CHECK_MPI_OK( mp_init(&qsub1) );
>      CHECK_MPI_OK( mp_init(&tmp)   );
> +    /* p and q must be distinct. */
> +    if (mp_cmp(p, q) == 0) {
> +	PORT_SetError(SEC_ERROR_NEED_RANDOM);

The SEC_ERROR_NEED_RANDOM error code is specifically checked for by one of the callers (RSA_NewKey), so it is important to use SEC_ERROR_NEED_RANDOM here. I agree SEC_ERROR_NEED_RANDOM is a little confusing for the other caller (RSA_PrivateKeyCheck), which doesn't generate the p and q.

@@ +288,5 @@
>  	CHECK_SEC_OK( generate_prime(&q, primeLen) );
> +	/* Assure p > q */
> +	/* NOTE: PKCS #1 does not require p > q, and NSS doesn't use any
> +	 * implementation optimization that requires p > q. We can remove
> +	 * this code in the future.

I did a Web search for "RSA ISO standard" but didn't find the ISO standard you referred to. If you give me specific standards, I can check them.

I checked FIPS 186-4. As far as I can tell, there is no p > q requirement. Note: I did find a requirement in Appendix B.3.1 that |p - q| (the absolute value of the difference between p and q) must not be too small. Search for "The primes p and q shall be selected with the following constraints:".

@@ +1451,5 @@
>      /* d_q == d mod q-1 */
>      CHECK_MPI_OK( mp_mod(&d, &qsub1, &res) );
>      VERIFY_MPI_EQUAL(&res, &d_q);
>      /* q * q**-1 == 1 mod p */
> +    /* Should also check qInv is a positive integer less than p. */

I removed this comment. To minimize the risk of this patch, I won't add this check now.

Comment 40

[Wan-Teh Chang]

Attachment: Proposed patch, v2

I made the changes that Richard and Bob suggested.

Patch checked in: https://hg.mozilla.org/projects/nss/rev/358730f7261d

Comment 41

[Mike Kaply [:mkaply]]

[Tracking Requested - why for this release]:

I believe this should go in the ESR31 as well, since enterprise customers are running into this (currently on ESR 24)

Comment 42

[Wan-Teh Chang]

(In reply to Mike Kaply (:mkaply) from comment #38)
>
> Was NSS upgraded in the ESR? Why?

The NSS change that broke this was in NSS 3.16.2. See
bug 1021102 and the two bugs named in bug 1021102 comment 0.
At that time I thought p > q was required, and any
incompatibility would be discovered quickly during testing
of mozilla-central nightly builds.

Dan: should we add this fix to an NSS 3.16.x patch release
for the ESR?

Note: the code in RSA_NewKey that ensures p > q was added in
year 2000: https://hg.mozilla.org/projects/nss/rev/db02a4b18663
The checkin message doesn't reference a bug, so I can't find
the reason behind it.

Comment 43

[Robert Relyea]

Comment on attachment 8501253 [details] [diff] [review]
Proposed patch, v2

r+ event though I've r+ the previous patch and this only incorperates some comments.

Comment 44

[Marc Meltzer]

Is there any possibility this fix can be incorporated into ESR 24 as well as ESR 31, since it was broken in ESR 24?

Comment 45

[Wan-Teh Chang]

I will advocate this at the NSS meeting tomorrow. ESR 24 and ESR 31
are using the same version of NSS right now, so this should not be
hard. I just need to get Mozilla's approval.

Comment 46

[Wan-Teh Chang]

Richard: do you know of any WebCrypto tests that should be updated
or removed?

Comment 47

[Sylvestre Ledru [:Sylvestre]]

(In reply to Marc Meltzer from comment #44)
> Is there any possibility this fix can be incorporated into ESR 24 as well as ESR 31, since it was broken in ESR 24?
Consider ESR 24 as dead (well, next Tuesday). All users should switch to ESR 31.
For ESR 31, yes, I think we should take a fix.

Comment 48

[Dana Keeler (she/her) (use needinfo) [:keeler]]

(In reply to Wan-Teh Chang from comment #46)
> Richard: do you know of any WebCrypto tests that should be updated
> or removed?

I don't think there are any webcrypto tests that rely on the relationship between p and q. We should probably add one to make sure this continues to work as expected.

Comment 49

[Sylvestre Ledru [:Sylvestre]]

Wan-Teh, any idea when you could land the patch? Thanks

Comment 50

[Kai Engert (:KaiE:)]

We're asking for your help in testing the correctness of the fix.

I have started an experimental build of Firefox 31 with this patch applied.
Within the next hours, I'll provide a link, from where you can download the build.

Could you please prepare to get this tested as soon as possible, after the build will be available, and give feedback in this bug report?
Thank you in advance.

(The try build is ongoing here:
 https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=414d13f6d569 )

Comment 51

[Wan-Teh Chang]

Comment on attachment 8501253 [details] [diff] [review]
Proposed patch, v2

Patch pushed to mozilla-inbound as part of NSS_3_17_2_BETA2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc04a1b51cb6

Comment 52

[Kai Engert (:KaiE:)]

(In reply to Kai Engert (:kaie) from comment #50)
> We're asking for your help in testing the correctness of the fix.
> 
> Could you please prepare to get this tested as soon as possible, after the
> build will be available, and give feedback in this bug report?
> Thank you in advance.

The tests build are appearing here:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-414d13f6d569/

The Linux builds are already completed and ready for download

The other platforms will show up at that address, as soon as the builds are completed (should happen within the next 2 hours).

Can you please test and give feedback? Thank you.

Comment 53

[Benjamin Kerensa [:bkerensa]]

As ESR owner I just wanted to chime in and say we will take a patch fixing this.

Comment 54

[Kai Engert (:KaiE:)]

(In reply to Kai Engert (:kaie) from comment #50)
> (The try build is ongoing here:
>  https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=414d13f6d569 )

FYI, the following test failures reported on above page can be ignored:
- Windows Mn 
- Web platform (WR, W1, etc.)
- M-e10s
- the X test, if the only mentioned failure is with test_cert_overrides
  (this was a false alert related to another bugfix included in this build,
   related to bug 1042889)

Comment 55

[Richard Barnes [:rbarnes]]

(In reply to Wan-Teh Chang from comment #46)
> Richard: do you know of any WebCrypto tests that should be updated
> or removed?

We don't currently test for failure when p < q, so this change should not break any current WebCrypto tests.  And it doesn't seem necessary to add a test with p < q, since WebCrypto just passes through to NSS.

Comment 56

[Kai Engert (:KaiE:)]

Who will test? Any volunteers?

Comment 57

[Kai Engert (:KaiE:)]

Download links to the experimental builds:

Windows:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-414d13f6d569/try-win32/firefox-31.2.0esr.en-US.win32.zip
(please download and extract to a new folder, then doubleclick firefox.exe inside that folder)

Mac:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-414d13f6d569/try-macosx64/firefox-31.2.0esr.en-US.mac.dmg
(mount disk image, then run firefox inside it)

Linux 64bit:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-414d13f6d569/try-linux64/firefox-31.2.0esr.en-US.linux-x86_64.tar.bz2

Linux 32bit:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-414d13f6d569/try-linux/firefox-31.2.0esr.en-US.linux-i686.tar.bz2

Comment 58

[Kai Engert (:KaiE:)]

Dear IIDA Yosiaki, because you reported the bug, may I ask you to please test, if the Firefox test version from comment 57 fixes this bug? Thank you in advance.

Comment 59

[lists]

Tested Windows and Linux(32) and was able to successfully import a client certificate that failed to import with release versions of Firefox 31 and 32.

So this fixes the issue for me.  

Thank you!

Comment 60

[Benjamin Kerensa [:bkerensa]]

(In reply to Kai Engert (:kaie) from comment #58)
> Dear IIDA Yosiaki, because you reported the bug, may I ask you to please
> test, if the Firefox test version from comment 57 fixes this bug? Thank you
> in advance.

Does feedback in comment 59 work? Hoping to get this approved tonight so we can land this early tomorrow.

Comment 61

[Kai Engert (:KaiE:)]

(In reply to Benjamin Kerensa [:bkerensa] from comment #60)
> Does feedback in comment 59 work? Hoping to get this approved tonight so we
> can land this early tomorrow.

Yes, that's good!

I'm waiting for feedback from Wan-Teh, who wants to double check the release tag.

The "uplift patch" will be created by running a script, which, in addition to the attached patch, will also change the NSS version number in a few files. There won't be any additional functional change.

I will run
  python client.py update_nss FINAL_RELEASE_TAG

If you don't mind the slight differences (only additional version numbers), then please feel free to approve the patch already attached here.

Comment 62

[Kai Engert (:KaiE:)]

I'm slightly confused by the wontfix + flags seen for 33/34.

Can you please tell me, which Firefox branches would you like the fix to land?

Comment 63

[Benjamin Kerensa [:bkerensa]]

(In reply to Kai Engert (:kaie) from comment #62)
> I'm slightly confused by the wontfix + flags seen for 33/34.
> 
> Can you please tell me, which Firefox branches would you like the fix to
> land?

We are landing this for ESR 31 right now AFAIK

Comment 64

[Sylvestre Ledru [:Sylvestre]]

and aurora (34).

Comment 65

[Benjamin Kerensa [:bkerensa]]

Kai,

Do you think you will have a patch for this today?

Comment 66

[Kai Engert (:KaiE:)]

Benjamin, the information from comment 61 still applies.

Comment 67

[Kai Engert (:KaiE:)]

I have a go from Wan-Teh, I'll provide the missing pieces shortly.

Comment 68

[Kai Engert (:KaiE:)]

Attachment: Patch to upgrade Mozilla 31 ESR to NSS 3.16.2.2

This patch is the same patch as the reviewed patch in this bug, plus the NSS release number bumps and touching the dependency files.

Comment 69

[Kai Engert (:KaiE:)]

I'll land into mozilla-central, and I'll attach another patch for aurora 34 approval.

Comment 70

[Kai Engert (:KaiE:)]

Attachment: Upgrade Mozilla 34 to NSS 3.17.2

This patch can be used to land NSS 3.17.2 into Mozilla 34.

Minutes ago I updated mozilla-inbound to NSS 3.17.2 in bug 1075686.

The patch was created by running:
  python client.py update_nss NSS_3_17_2_RTM
and manually editing configure.in to bump the nss version to 3.17.2

(If you consider to upgrade Firefox 33, too, instead of appliying the patch, you should run the script (which will automatically fix the coreconf.dep file.)

Comment 71

[Kai Engert (:KaiE:)]

Comment on attachment 8503284 [details] [diff] [review]
Patch to upgrade Mozilla 31 ESR to NSS 3.16.2.2

checked in to esr31
https://hg.mozilla.org/releases/mozilla-esr31/rev/f5690f9ca229

Comment 72

[Kai Engert (:KaiE:)]

I'm closing this bug. We usually close NSS bugs as soon as it has been landed into NSS HG.

Fixing of this bug in mozilla-central is tracked by bug 1075686 (upgrade to NSS 3.17.2), which will be marked fixed on the next mozilla-inbound merge.

Comment 73

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8503289 [details] [diff] [review]
Upgrade Mozilla 34 to NSS 3.17.2

This is find for 34. Aurora+

Comment 74

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e4a01dff6341

Sorry, I copy pasted from the other commit, and used the wrong approver name.

Comment 75

[Matt Wobensmith [:mwobensmith][:matt:]]

I've verified this fix using steps in the first comment. For what it's worth, the test instructions above have the wrong password. The correct password (2clntr1s) is contained inside the file named "UnimportablePwd.txt" within the first attachment. 

On Fx31.1.1esr, the import fails and gives the error detailed above.
On Fx31.2.0 build 3, the import succeeds with no error.

Tested on Mac OS 10.9.5, Windows 8.1 and Ubuntu Linux 13.04.

We'll confirm on other branches when they are fixed.

Comment 76

[Marc Meltzer]

Sorry for the late response, but I've been away for a few days.

I appreciate that ESR 24 is end-of-life, but the product was broken, it should be fixed.  Some of us are still undergoing internal testing of ESR 31 and haven't been able to move to it.  We are working on it, but it takes time.

I don't know if the decision has been made yet or not, so I will continue to advocate for 24 being fixed.

(In reply to Sylvestre Ledru [:sylvestre] from comment #47)
> (In reply to Marc Meltzer from comment #44)
> > Is there any possibility this fix can be incorporated into ESR 24 as well as ESR 31, since it was broken in ESR 24?
> Consider ESR 24 as dead (well, next Tuesday). All users should switch to ESR
> 31.
> For ESR 31, yes, I think we should take a fix.

Comment 77

[Sylvestre Ledru [:Sylvestre]]

(In reply to Marc Meltzer from comment #76)
> I don't know if the decision has been made yet or not, so I will continue to
> advocate for 24 being fixed.
Yes, it will be part of esr 31.2.0 released today (cf last comments).

Comment 78

[Mike Kaply [:mkaply]]

I agree with Marc.

There was still no explanation of how this bug ended up in ESR 24.

wtc, you said:

>> NSS upgraded in the ESR? Why?

>The NSS change that broke this was in NSS 3.16.2.

The question is still why was an entirely new version of NSS included in the Firefox 24 ESR.

The ESR is supposed to cherry pick security fixes. It shouldn't have gotten an entirely new NSS version unless that version only contained the relevant security fixes (which it clearly didn't).

Something was broke in the ESR 24 process that caused this bug to come in at all.

Comment 79

[Sylvestre Ledru [:Sylvestre]]

Mike, this update of NSS happened in bug 1020695. Not very clear why.

Comment 80

[Mike Kaply [:mkaply]]

The relevant comment is:

> Per bug 963150, this is going to need to land on everything down to esr24.

But for some reason 963150 is still security sensitive. Aren't the bugs supposed to be unmarked after they are fixed?

Comment 81

[Sylvestre Ledru [:Sylvestre]]

Kai, it seems that it didn't cause regression in ESR or aurora/beta. I think I am going to take this bug in 33.1. Could you fill the uplift request for mozilla-release? thanks

Comment 82

[IIDA Yosiaki]

(In reply to Kai Engert (:kaie) from comment #58)
> Dear IIDA Yosiaki, because you reported the bug, may I ask you to please
> test, if the Firefox test version from comment 57 fixes this bug? Thank you
> in advance.

Hello Kai and people.  Sorry for overlooking.  I've just made an in-house application for downloading executables.  For security reason, downloading executable files needs lots of red-tape procedures in my company...

Comment 83

[Kai Engert (:KaiE:)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #81)
> Kai, it seems that it didn't cause regression in ESR or aurora/beta. I think
> I am going to take this bug in 33.1. Could you fill the uplift request for
> mozilla-release? thanks

Sorry for the delay.

The Firefox 33 branch uses NSS 3.17.1

In order to pick up this fix, you should upgrade 33 to NSS 3.17.2

NSS 3.17.2 was a minimal patch release, which included only two fixes in the library section relevant for Firefox:
- this bugfix
- a regression fix (bug 1057161)

I'll prepare a patch for uplifting and request approval.

Comment 84

[Kai Engert (:KaiE:)]

Attachment: Patch to upgrade Mozilla 33 to NSS 3.17.2, which includes this bugfix. (For landing, please use: python client.py update_nss NSS_3_17_2_RTM)

Request approval for Firefox 33 as suggested.

Comment 85

[Wan-Teh Chang]

Comment on attachment 8511383 [details] [diff] [review]
Patch to upgrade Mozilla 33 to NSS 3.17.2, which includes this bugfix. (For landing, please use: python client.py update_nss NSS_3_17_2_RTM)

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

r=wtc.

(In reply to Kai Engert (:kaie) from comment #83)
>
> NSS 3.17.2 was a minimal patch release, which included only two fixes in the
> library section relevant for Firefox:
> - this bugfix
> - a regression fix (bug 1057161)

Nit: bug 1057161 is not a regression. (But it is a serious bug.)

Comment 86

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/mozilla-release/rev/8c16b644aaa7

Comment 87

[Satya]

Still doesn't work on FF 33.0.2

Comment 88

[Kai Engert (:KaiE:)]

(In reply to Satya from comment #87)
> Still doesn't work on FF 33.0.2

do you refer to a nightly build?

then you should wait with testing until tomorrow's nightly build

Comment 89

[Satya]

(In reply to Kai Engert (:kaie) from comment #88)
> (In reply to Satya from comment #87)
> > Still doesn't work on FF 33.0.2
> 
> do you refer to a nightly build?
> 
> then you should wait with testing until tomorrow's nightly build

This is not from a nightly build. I got auto update on my browser and that is the latest version I see.

Comment 90

[Kai Engert (:KaiE:)]

(In reply to Satya from comment #89)
> This is not from a nightly build. I got auto update on my browser and that
> is the latest version I see.

Ok. The change has landed into the release branch, but no updated release has been created yet. You'll have to wait until a new 33.x has been made and published.

But if you want to help testing, you could try a candidate build.
I'm guessing the right place is
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/33.1-candidates/

Make sure the timestamp of the build is newer than comment 86.

Comment 91

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Per bug 1084005 comment 16, we need this on b2g32 as well.

Comment 92

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/251eb3253863

Comment 93

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/251eb3253863

Comment 94

[John Hein]

Is there a workaround I can use to import a .p12 certificate that has this problem if I can't update from Firefox 31.0 for a couple months (private locked down network not connected to the internet)?

Comment 95

[Stefano]

I have the same problem with self signed certificates generated with java on ff 38. attached the failing p12 file-

Comment 96

[Stefano]

Attachment: ste.p12

Comment 97

[Oliver]

Hey folks, just ran into the same issue today, but for another reason. So I thought I'd leave a message here in case others run into this as well.

Since I find the whole key (and CSR) generation business a bit intransparent, I decided to create my own key and CSR using OpenSSL. In order to collect the certificate, however, the CA required that the private key be installed in the browser to identify me.

So I tried to export the key to PKCS12:

openssl pkcs12 -export -nocerts -inkey filename.key -out filename.p12

and tried to import that into FF (latest released as of this writing: 38.0.5). Consequently I ended up here, read the whole thread, but since the claim was that the problem was fixed, FF was unlikely to blame. So I thought to myself: the dialog is about certificates, but I'm trying to feed it a key only (also certmgr.msc in Windows complained about an empty cert). So I decided to create my own self-signed certificate to smuggle in that private key.

openssl req -new -key filename.key -out filename.csr
openssl x509 -req -days 365 -in filename.csr -signkey filename.key -out self-signed.crt

then combine both into PKCS12 format:

openssl pkcs12 -export -inkey filename.key -in self-signed.crt -out self-signed.p12

which FF imported without complaint. Turns out the certificate was what was missing all along.

After that I collected my certificate from the CA, removed the self-signed one and will now live happily ever after (well, perhaps ;)).

Comment 98

[Stefano]

(In reply to Stefano from comment #95)
> I have the same problem with self signed certificates generated with java on
> ff 38. attached the failing p12 file-

Commenting on my own post: the problem was that the keystore I was importing was not password protected. I created one password protected and it worked... I guess we should open a new issue for better error handling ?

Comment 99

[Michael Yoo]

I am also experiencing this issue. Firefox (Iceweasel) 38.3. Is there any workaround to this issue?

Comment 100

[Michael Yoo]

(In reply to Michael Yoo from comment #99)
> I am also experiencing this issue. Firefox (Iceweasel) 38.3. Is there any
> workaround to this issue?

Actually I have to retract that statement, I was getting the same "The PKCS #12 operation failed for unknown reasons." error message on a .pfx file so I was naturally led to this bug thread from Google.

Turns out it was an incorrect password issue, but I got a generic fallback error message...