Closed Bug 996237 Opened 11 years ago Closed 11 years ago

Update TLS implementation to conform to WebRTC requirements

Categories

(Core :: WebRTC: Networking, defect, P3)

Product:
Core
Component:
WebRTC: Networking
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34

People

(Reporter: mt, Assigned: mt)

References

Details

Attachments

(6 files, 21 obsolete files)

0001-Bug-996237-Whitespace-cleanup.patch
47.64 KB, patch
wtc
: review+
wtc
: checkin+
Details | Diff | Splinter Review
0001-Bug-996237-Limiting-WebRTC-cipher-profiles.patch
6.64 KB, patch
mt
: review+
Details | Diff | Splinter Review
0002-Bug-996237-Unit-testing-cipher-suite-selection-happy.patch
6.46 KB, patch
mt
: review+
Details | Diff | Splinter Review
0003-Bug-996237-Tests-for-cipher-mismatch.patch
6.86 KB, patch
mt
: review+
Details | Diff | Splinter Review
0004-Bug-996237-Checking-for-DHE-negotiation-failure.patch
1.14 KB, patch
mt
: review+
Details | Diff | Splinter Review
0005-Bug-996237-Fixing-nss.def.patch
647 bytes, patch
mt
: review+
Details | Diff | Splinter Review
Currently, we negotiate a version of DTLS that is equivalent to TLS 1.1 for WebRTC. This is because NSS didn't support TLS 1.2 when that code was written. It should be easy enough to support TLS 1.2, it's a one character change in dtlstransportlayer.cpp. The change would need to be tested (against both Chrome as well as older versions of Firefox at a minimum).
There is more here to this than just TLS 1.2. If we can ensure that our usage is compliant with the security-arch document, that would be good: http://tools.ietf.org/html/draft-ietf-rtcweb-security-arch (currently -09).
Assignee: nobody → martin.thomson
Summary: Support RFC 6347 for WebRTC → Update TLS implementation to conform to WebRTC requirements
Priority: -- → P3
Target Milestone: --- → mozilla33
A basic untabify, remove trailing whitespace, realign patch.
Looks like only minimal changes were needed to get NSS to support DTLS 1.2. That is, unless the record layer is screwed up in ways I can't detect. I'll need to find a peer to test against. Chrome negotiates happily, but I'm not sure that's a good test candidate.
Simple hack that removes more code than it adds. It also saves a negligible number of precious run-time bytes.
Attachment #8430386 - Flags: review?(ekr)
Here's the big change to comply with the security-arch requirements. Turns out the tests don't like a straight disable of suites, because NSS doesn't turn ECC by default. That's not a problem in Firefox because we enable ECC separately.
Attachment #8430390 - Flags: review?(ekr)
Attachment #8430377 - Flags: review?(wtc)
Attachment #8430385 - Flags: review?(wtc)
Status: NEW → ASSIGNED
Comment on attachment 8430377 [details] [diff] [review] 0001-Bug-996237-Whitespace-cleanup.patch Review of attachment 8430377 [details] [diff] [review]: ----------------------------------------------------------------- I removed some spaces in dtlscon.c and checked in this patch. https://hg.mozilla.org/projects/nss/rev/33c165684527 ::: security/nss/lib/ssl/dtlscon.c @@ +953,4 @@ > SECStatus > dtls_HandleHelloVerifyRequest(sslSocket *ss, SSL3Opaque *b, PRUint32 length) > { > + int errCode = SSL_ERROR_RX_MALFORMED_HELLO_VERIFY_REQUEST; Some spaces were added before the '=' sign by mistake. I removed them. @@ +999,5 @@ > > /* Now re-send the client hello */ > rv = ssl3_SendClientHello(ss, PR_TRUE); > > + ssl_ReleaseXmitBufLock(ss); /*******************************/ Some spaces were added before the comment by mistake. I removed them.
Attachment #8430377 - Flags: review?(wtc)
Attachment #8430377 - Flags: review+
Attachment #8430377 - Flags: checkin+
Comment on attachment 8430385 [details] [diff] [review] 0002-Bug-996237-DTLS-1.2-support-for-NSS.patch Review of attachment 8430385 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/nss/lib/ssl/dtlscon.c @@ +90,3 @@ > } > > /* On this socket, Disable non-DTLS cipher suites in the argument's list */ Please see my "preliminary patch" in bug 959864. The only real difference is that I also changed the dtls_HandleHelloVerifyRequest function in this file. Do you think we need that change?
Depends on: 959864
Comment on attachment 8430385 [details] [diff] [review] 0002-Bug-996237-DTLS-1.2-support-for-NSS.patch Marking this obsolete, bug 959864 will deal with this.
Attachment #8430385 - Attachment is obsolete: true
Attachment #8430385 - Flags: review?(wtc)
Comment on attachment 8430386 [details] [diff] [review] 0003-Bug-996237-Hard-coding-SRTP-cipher-suite-selection.patch Review of attachment 8430386 [details] [diff] [review]: ----------------------------------------------------------------- I'm actually not really excited by this patch. I would prefer to leave this extension point open.
Attachment #8430386 - Flags: review?(ekr)
Comment on attachment 8430390 [details] [diff] [review] 0004-Bug-996237-Limiting-cipher-profiles-to-what-we-absol.patch Review of attachment 8430390 [details] [diff] [review]: ----------------------------------------------------------------- Please revise as below. Also, please add a unit test that verifies which cipher suite we negotiate. ::: media/mtransport/transportlayerdtls.cpp @@ -352,5 @@ > -static const uint16_t SrtpCiphers[] = { > - SRTP_AES128_CM_HMAC_SHA1_80, > - SRTP_AES128_CM_HMAC_SHA1_32 > -}; > - This needs to come back if we don't land patch 3. @@ -555,5 @@ > - // Set the SRTP ciphers > - rv = SSL_SetSRTPCiphers(ssl_fd, SrtpCiphers, > - sizeof(SrtpCiphers) / sizeof(uint16_t)); > - if (rv != SECSuccess) { > - MOZ_MTLOG(ML_ERROR, "Couldn't set SRTP cipher suite"); This too. @@ +594,5 @@ > + > +// Ciphers we explicitly want to disable. These are the modes we have to permit > +// for compatibility reasons in HTTPS. WebRTC has a narrower permitted set. > +// Anything outside this list is governed by the usual combination of policy and > +// user preferences. Please comment on why these have been removed. @@ +635,5 @@ > + } > + > + > + for (size_t i = 0; i < sizeof(EnabledCiphers) / sizeof(uint32_t); ++i) { > + MOZ_MTLOG(ML_NOTICE, LAYER_INFO << "Enabling: " << EnabledCiphers[i]); ML_INFO seems fine. @@ +645,5 @@ > + } > + } > + > + for (size_t i = 0; i < sizeof(DisabledCiphers) / sizeof(uint32_t); ++i) { > + MOZ_MTLOG(ML_NOTICE, LAYER_INFO << "Disabling: " << DisabledCiphers[i]); ML_INFO seems fine, @@ +651,5 @@ > + if (rv != SECSuccess) { > + MOZ_MTLOG(ML_NOTICE, LAYER_INFO << > + "Unable to disable suite: " << DisabledCiphers[i]); > + // ignore SECFailure, because that indicates that the suite we wanted to > + // disable wasn't even compiled in, which achieves the goal Ugh. Can you do SSL_CipherPrefGet() first to avoid this? ::: media/webrtc/signaling/test/mediapipeline_unittest.cpp @@ -75,3 @@ > } > ASSERT_EQ((nsresult)NS_OK, res); > This belongs in patch 3 but since I suggest we remove patch 3 I think we can lose this as well.
Attachment #8430390 - Flags: review?(ekr) → review-
Attached patch Limiting WebRTC cipher profiles (obsolete) — — Splinter Review
Attachment #8430390 - Attachment is obsolete: true
Attachment #8430386 - Attachment is obsolete: true
Comment on attachment 8443797 [details] [diff] [review] Limiting WebRTC cipher profiles Fine, have it your way.
Attachment #8443797 - Flags: review?(ekr)
Comment on attachment 8443797 [details] [diff] [review] Limiting WebRTC cipher profiles Let's wait until I have the tests in.
Attachment #8443797 - Attachment is obsolete: true
Attachment #8443797 - Flags: review?(ekr)
Attached patch Limiting WebRTC cipher profiles (obsolete) — — Splinter Review
Attachment #8444776 - Flags: review?(ekr)
Attachment #8444777 - Flags: review?(ekr)
Attached patch Tests for cipher mismatch (obsolete) — — Splinter Review
Attachment #8445276 - Flags: review?(ekr)
Target Milestone: mozilla33 → mozilla35
Blocks: 1046295
I'm going to go with ECDHE and RSA here for now. I've raised bug 1046295 to track any changes that might be needed once the IETF makes up their collective mind.
Attachment #8444776 - Attachment is obsolete: true
Attachment #8444776 - Flags: review?(ekr)
Attachment #8464892 - Flags: review?(ekr)
Attachment #8444777 - Attachment is obsolete: true
Attachment #8444777 - Flags: review?(ekr)
Attachment #8464893 - Flags: review?(ekr)
Attachment #8445276 - Attachment is obsolete: true
Attachment #8445276 - Flags: review?(ekr)
Attachment #8464894 - Flags: review?(ekr)
Comment on attachment 8464892 [details] [diff] [review] 0001-Bug-996237-Limiting-WebRTC-cipher-profiles.patch Review of attachment 8464892 [details] [diff] [review]: ----------------------------------------------------------------- This minimally needs to not break 1.1 and I think my cipher selection algorithm is simpler. ::: media/mtransport/transportlayerdtls.cpp @@ +578,4 @@ > return true; > } > > +// Ciphers we need to enable. These are on by default in standard firefox I'm a little unclear on the policy here. As I understand it the result of this is to give us the union of the ciphersuites on in the environment and those on here minus the ones turned off here? Would it be better to just disable everything and then turn stuff back on? The code would be simpler and more predictable. @@ +595,5 @@ > +static const uint32_t DisabledCiphers[] = { > + TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA, > + TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, > + TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, > + TLS_ECDHE_RSA_WITH_RC4_128_SHA, This actually has the impact of breaking 1.1 compat. That seems bad.
Attachment #8464892 - Flags: review?(ekr) → review-
Comment on attachment 8464893 [details] [diff] [review] 0002-Bug-996237-Unit-testing-cipher-suite-selection-happy.patch Review of attachment 8464893 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/mtransport/test/transport_unittests.cpp @@ +543,4 @@ > > size_t received() { return received_; } > > + uint16_t cipherSuite() { Can this be const? @@ +714,5 @@ > + SetDtlsPeer(); > + ConnectSocket(); > + > + // SRTP is on > + ASSERT_EQ(SRTP_AES128_CM_HMAC_SHA1_80, p1_->srtpCipher()); Please check the DTLS ciphersuite as well here.
Attachment #8464893 - Flags: review?(ekr) → review+
Comment on attachment 8464894 [details] [diff] [review] 0003-Bug-996237-Tests-for-cipher-mismatch.patch Review of attachment 8464894 [details] [diff] [review]: ----------------------------------------------------------------- I think this needs another round, especially if I am right about SSL_ImplementedCiphers. ::: media/mtransport/test/transport_unittests.cpp @@ +9,4 @@ > #include <iostream> > #include <string> > #include <map> > +#include <algorithm> Is algorthm really needed? @@ +824,5 @@ > TransferTest(1); > } > > +// For this next test, we need mutually exclusive sets of ciphersuites so we do > +// this the hard way... which is also brittle because new ciphersuites will This seems over-complicated. Couldn't you just do Side A: only cipher suite X Side B: only cipher suite Y != X @@ +828,5 @@ > +// this the hard way... which is also brittle because new ciphersuites will > +// cause this test to fail, if they are enabled by default. > + > +// Set A has all the first half ... > +static uint16_t AllCipherSuites[] { Can't you get this from SSL_ImplementedCiphers? @@ +900,5 @@ > + size_t count = sizeof(AllCipherSuites) / 2; > + uint16_t* middle = AllCipherSuites + (count / 2); > + uint16_t* end = AllCipherSuites + count; > + std::vector<uint16_t> setA(AllCipherSuites, middle); > + std::vector<uint16_t> setB(middle, end); Why not do odds/evens? The algorithm is simpler. Or just A and just B. @@ +914,5 @@ > +// one of the mandatory-to-implement suites, which should succeed > +static void ConfigureOneCipher(TransportTestPeer* peer, uint16_t suite) { > + > + std::vector<uint16_t> justGcm; > + justGcm.push_back(suite); Let's call this "justOne" since it's not just gcm
Attachment #8464894 - Flags: review?(ekr) → review-
Comment on attachment 8464892 [details] [diff] [review] 0001-Bug-996237-Limiting-WebRTC-cipher-profiles.patch Review of attachment 8464892 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/mtransport/transportlayerdtls.cpp @@ +586,5 @@ > + TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, > + TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA > +}; > + > +// All modes without PFS or with a non-AEAD record layer are disabled. You actually didn't disable non-AEAD AES
(In reply to Eric Rescorla (:ekr) from comment #20) > I'm a little unclear on the policy here. As I understand it the result > of this is to give us the union of the ciphersuites on in the environment > and those on here minus the ones turned off here? > > Would it be better to just disable everything and then turn stuff back > on? The code would be simpler and more predictable. I think that as much as possible we should respect user preferences and only deviate when we know that stuff sucks (disable) or we have to (MTI). > This actually has the impact of breaking 1.1 compat. That seems bad. Yeah: comment bad, code OK. That why I don't like comments. (In reply to Eric Rescorla (:ekr) from comment #22) > I think this needs another round, especially if I am right about > SSL_ImplementedCiphers. I looked, but obviously missed that. > ::: media/mtransport/test/transport_unittests.cpp > @@ +9,4 @@ > > #include <iostream> > > #include <string> > > #include <map> > > +#include <algorithm> > > Is algorthm really needed? I use it to remove the one suite. It's not really needed in the sense that I can write a for loop as well as the next person, but I don't like doing that. > @@ +900,5 @@ > > + size_t count = sizeof(AllCipherSuites) / 2; > > + uint16_t* middle = AllCipherSuites + (count / 2); > > + uint16_t* end = AllCipherSuites + count; > > + std::vector<uint16_t> setA(AllCipherSuites, middle); > > + std::vector<uint16_t> setB(middle, end); > > Why not do odds/evens? The algorithm is simpler. Or just A and just B. I don't think that odds/evens is any different. But reusing the "just one" code is obviously right.
Attachment #8464892 - Attachment is obsolete: true
Attachment #8465618 - Flags: review?(ekr)
r=ekr
Attachment #8464893 - Attachment is obsolete: true
Attachment #8465619 - Flags: review+
Can't believe I missed the SSL_ImplementedCiphers thing.
Attachment #8464894 - Attachment is obsolete: true
Attachment #8465620 - Flags: review?(ekr)
Comment on attachment 8465620 [details] [diff] [review] 0003-Bug-996237-Tests-for-cipher-mismatch.patch Review of attachment 8465620 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/mtransport/test/transport_unittests.cpp @@ +829,5 @@ > > +// test the default configuration against a peer that supports only > +// one of the mandatory-to-implement suites, which should succeed > +static void ConfigureOneCipher(TransportTestPeer* peer, uint16_t suite) { > + Extra whitespace @@ +841,5 @@ > + peer->SetCipherSuiteChanges(justOne, everythingElse); > +} > + > +TEST_F(TransportTest, TestCipherMismatch) { > + Extra whitespace. @@ +868,5 @@ > + > + ConnectSocket(); > + > + ASSERT_EQ(TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, p1_->cipherSuite()); > + ASSERT_EQ(TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, p2_->cipherSuite()); Suggest that you make an equality check part of ConnectSocket and then just assert p1_ here.
Attachment #8465620 - Flags: review?(ekr) → review+
Here is our ClientHello after the patch: Datagram Transport Layer Security DTLSv1.2 Record Layer: Handshake Protocol: Client Hello Content Type: Handshake (22) Version: DTLS 1.2 (0xfefd) Epoch: 0 Sequence Number: 1 Length: 128 Handshake Protocol: Client Hello Handshake Type: Client Hello (1) Length: 116 Message Sequence: 0 Fragment Offset: 0 Fragment Length: 116 Version: DTLS 1.2 (0xfefd) Random.gmt_unix_time: Feb 21, 2052 07:36:39.000000000 PST Random.bytes Session ID Length: 0 Cookie Length: 0 Cipher Suites Length: 18 Cipher Suites (9 suites) Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b) Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f) Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a) Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009) Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013) Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014) Cipher Suite: TLS_DHE_RSA_WITH_AES_128_CBC_SHA (0x0033) Cipher Suite: TLS_DHE_DSS_WITH_AES_128_CBC_SHA (0x0032) Cipher Suite: TLS_DHE_RSA_WITH_AES_256_CBC_SHA (0x0039) Compression Methods Length: 1 Compression Methods (1 method) Compression Method: null (0) Extensions Length: 56 Extension: renegotiation_info Type: renegotiation_info (0xff01) Length: 1 Data (1 byte) Extension: elliptic_curves Type: elliptic_curves (0x000a) Length: 8 Data (8 bytes) Extension: ec_point_formats Type: ec_point_formats (0x000b) Length: 2 Data (2 bytes) Extension: use_srtp Type: use_srtp (0x000e) Length: 7 Data (7 bytes) Extension: signature_algorithms Type: signature_algorithms (0x000d) Length: 18 Data (18 bytes) And the ServerHello: Datagram Transport Layer Security DTLSv1.2 Record Layer: Handshake Protocol: Server Hello Content Type: Handshake (22) Version: DTLS 1.2 (0xfefd) Epoch: 0 Sequence Number: 0 Length: 104 Handshake Protocol: Server Hello Handshake Type: Server Hello (2) Length: 92 Message Sequence: 0 Fragment Offset: 0 Fragment Length: 92 Version: DTLS 1.2 (0xfefd) Random.gmt_unix_time: May 20, 2069 02:18:16.000000000 PDT Random.bytes Session ID Length: 32 Session ID (32 bytes) Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f) Compression Method: null (0) Extensions Length: 20 Extension: renegotiation_info Type: renegotiation_info (0xff01) Length: 1 Data (1 byte) Extension: ec_point_formats Type: ec_point_formats (0x000b) Length: 2 Data (2 bytes) Extension: use_srtp Type: use_srtp (0x000e) Length: 5 Data (5 bytes)
As you note, since NSS doesn't support DHE on the server side, we need to ensure that we don't negotiate it. NSS crashes if you only set DHE on the server end.
Attachment #8465864 - Flags: review?(ekr)
Changed to include expanded set of disabled ciphers (see comment 29).
Attachment #8465618 - Attachment is obsolete: true
Attachment #8465618 - Flags: review?(ekr)
Attachment #8465875 - Flags: review?(ekr)
Attachment #8465864 - Attachment is obsolete: true
Attachment #8465864 - Flags: review?(ekr)
Attachment #8465876 - Flags: review?(ekr)
Comment on attachment 8465875 [details] [diff] [review] 0001-Bug-996237-Limiting-WebRTC-cipher-profiles.patch Review of attachment 8465875 [details] [diff] [review]: ----------------------------------------------------------------- lgtm with nitx below. ::: media/mtransport/transportlayerdtls.cpp @@ +588,5 @@ > +}; > + > +// All modes without PFS or with old and rusty ciphersuites are disabled. > +// These are the modes we have to permit for compatibility reasons in HTTPS, > +// but in WebRTC we can afford to be slightly more strict. Fix this comment since we are now removing things that may not be allowed. @@ +658,5 @@ > + return false; > + } > + } > + > + for (size_t i = 0; i < sizeof(EnabledCiphers) / sizeof(uint32_t); ++i) { Suggest PR_ARRAY_SIZE() here and below
Attachment #8465875 - Flags: review?(ekr) → review+
r=ekr
Attachment #8465875 - Attachment is obsolete: true
Attachment #8465895 - Flags: review+
Comment on attachment 8465876 [details] [diff] [review] 0004-Bug-996237-Checking-for-DHE-negotiation-failure.patch Review of attachment 8465876 [details] [diff] [review]: ----------------------------------------------------------------- lgtm ::: media/mtransport/test/transport_unittests.cpp @@ +884,5 @@ > ASSERT_EQ(0, p2_->srtpCipher()); > } > > +// NSS doesn't support DHE suites on the server end > +// this checks to see if we barf when that's the only option available Period at end of comment sentences
Attachment #8465876 - Flags: review?(ekr) → review+
r=ekr
Attachment #8465897 - Flags: review+
r=ekr
Attachment #8465620 - Attachment is obsolete: true
Attachment #8465876 - Attachment is obsolete: true
Attachment #8465900 - Flags: review+
All green, let's roll.
Keywords: checkin-needed
r=ekr
Attachment #8465897 - Attachment is obsolete: true
Attachment #8466463 - Flags: review+
NSS - as used in B2G and Firefox for Android - doesn't export all the symbols we need. Desktop builds don't encounter this issue because they link against libssl3.so; but B2G/Android builds symlink this to libnss3.so, probably to save space.
Attachment #8466477 - Flags: review?(wtc)
Attachment #8466477 - Flags: review?(dkeeler)
Comment on attachment 8466477 [details] [diff] [review] 0005-Bug-996237-Fixing-nss.def.patch Review of attachment 8466477 [details] [diff] [review]: ----------------------------------------------------------------- r=wtc. I verified that these two functions are exported in the standard NSS symbol export file for libssl3.so: http://mxr.mozilla.org/nss/source/lib/ssl/ssl.def
Attachment #8466477 - Flags: review?(wtc) → review+
Attachment #8466477 - Flags: review?(dkeeler)
Comment on attachment 8466477 [details] [diff] [review] 0005-Bug-996237-Fixing-nss.def.patch Wan-Teh, could you check this in when you get the chance? Thanks.
Attachment #8466477 - Flags: checkin?(wtc)
Actually, this looks like it's in the Firefox code base, though it applies to NSS. MT, wasn't it checked in in: https://hg.mozilla.org/integration/mozilla-inbound/rev/f0775f310a34
Comment on attachment 8466477 [details] [diff] [review] 0005-Bug-996237-Fixing-nss.def.patch This file is actually Firefox-specific, and doesn't go in the NSS repo.
Attachment #8466477 - Flags: checkin?(wtc)
r=ekr ... I'm re-uploading everything, because clearly the last attempt failed.
Attachment #8465895 - Attachment is obsolete: true
Attachment #8467219 - Flags: review+
r=ekr
Attachment #8465619 - Attachment is obsolete: true
Attachment #8467220 - Flags: review+
r=ekr
Attachment #8465900 - Attachment is obsolete: true
Attachment #8466463 - Attachment is obsolete: true
Attachment #8467222 - Flags: review+
r=wtc
Attachment #8466477 - Attachment is obsolete: true
Attachment #8467225 - Flags: review+
Keywords: checkin-needed
Can we please get a fresh Try run please given that it's been backed out twice now? I think builds-only is fine: try: -b do -p all -u none -t none
Keywords: checkin-needed
Target Milestone: mozilla35 → mozilla34
The current patch set is identical to what was run here: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9f7bb93575ed The error was in the uploading process. (Dreams of autolander removing these sorts of stupid PEBKAC errors.)
See comment 58.
Keywords: checkin-needed
Comment on attachment 8466477 [details] [diff] [review] 0005-Bug-996237-Fixing-nss.def.patch Review of attachment 8466477 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Martin Thomson [:mt] from comment #49) > > Wan-Teh, could you check this in when you get the chance? Thanks. Martin, sorry about the late reply. security/build/nss.def is not part of NSS, so you don't need an NSS developer to check in this patch for you.
Blocks: 798943
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: