Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#42718 closed defect (fixed)

certsync fails to verify macports.org certificate

Reported by: ryandesign (Ryan Schmidt) Owned by: landonf (Landon Fuller)
Priority: High Milestone:
Component: ports Version: 2.2.1
Keywords: Cc: raimue (Rainer Müller), llattanzi+McF@…, cooljeanius (Eric Gallager)
Port: certsync

Description

We recently got a new SSL certificate for macports.org, from a different organization, and certsync fails to verify it confirmed on multiple machines and OS X versions:

$ sudo port -v sync
--->  Updating the ports tree
Synchronizing local ports tree from file:///Users/rschmidt/macports/dports
Updating '/Users/rschmidt/macports/dports':
svn: E230001: Unable to connect to a repository at URL 'https://svn.macports.org/repository/macports/trunk'
svn: E230001: Server SSL certificate verification failed: certificate has expired
Command failed: /opt/local/bin/svn update --non-interactive /Users/rschmidt/macports/dports
Exit code: 1
$ curl https://www.macports.org/
curl: (60) SSL certificate problem: certificate has expired
More details here: http://curl.haxx.se/docs/sslcerts.html

curl performs SSL certificate verification by default, using a "bundle"
of Certificate Authority (CA) public keys (CA certs). If the default
bundle file isn't adequate, you can specify an alternate file
using the --cacert option.
If this HTTPS server uses a certificate signed by a CA represented in
the bundle, the certificate verification probably failed due to a
problem with the certificate (it might be expired, or the name might
not match the domain name in the URL).
If you'd like to turn off curl's verification of the certificate, use
the -k (or --insecure) option.
$ openssl s_client -connect www.macports.org:443 -CAfile /opt/local/etc/openssl/cert.pem
CONNECTED(00000004)
depth=2 C = BE, O = GlobalSign nv-sa, OU = Root CA, CN = GlobalSign Root CA
verify error:num=10:certificate has expired
notAfter=Jan 28 12:00:00 2014 GMT
verify return:0

Using curl-ca-bundle instead of certsync, there is no problem.

Analysis from Rainer:

I see in Keychain there are two certificates named "GlobalSign Root CA", and the one used here expired in January 2014, while the other one would be valid until January 2028. It's certainly using the wrong certificate, but I don't know yet why that happens.

Maybe certsync compares them by name in a dictionary instead of using a unique key identifier and that mixes them up?

Attachments (3)

splitcert.pl (683 bytes) - added by raimue (Rainer Müller) 7 years ago.
patch-mktemp-fixes-v0 (6.0 KB) - added by landonf (Landon Fuller) 7 years ago.
Proposed fix
patch-handle-expired-certs-v0 (5.3 KB) - added by landonf (Landon Fuller) 7 years ago.
*Actual* proposed fix.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by landonf (Landon Fuller)

Priority: NormalHigh
Status: newassigned

comment:2 Changed 7 years ago by raimue (Rainer Müller)

Cc: raimue@… removed

Cc Me!

comment:3 Changed 7 years ago by raimue (Rainer Müller)

Cc: raimue@… added

Cc Me!

comment:4 Changed 7 years ago by raimue (Rainer Müller)

The valid "GlobalSign Root CA" is actually a re-signed certificate with a longer lifetime using the same modulus/exponent from the older one that expired end of January 2014. They both have the identical public key.

After some more analysis, the curl-ca-bundle only contains the "GlobalSign Root CA" certificate that is valid throughout 2028, while certsync includes them both into the same bundle. It seems like OpenSSL cannot handle the same certificate twice in a bundle.

Side note: I will attach a small perl helper script which I used to split the certificate bundle into the original certificates, so they can be examined using openssl x509 -text -noout -in <file>.pem.

With experiments I got it to work when switching the order of the certificates, but it's not working again when adding another one. I guess it's up to some hash algorithm which one gets used, so a different order is not a reliable fix. It seems like the only fix would be to leave out that expired certificate...

I see two solutions:

Don't export any expired certificate

Which means using this CA in a chain would be reported as "untrusted" instead of "expired". That solves this immediate problem because the older "GlobalSign Root CA" certificate is expired now. It might not work in other cases.

This is relatively easy to accomplish as we only need to check the expiry date against the current date.

Only export one valid/non-expired certificate per public key

This means certsync needs a special case to check for duplicates and decide for the one with the later expiry date.

Needs a hash/dictionary with the key being the public key of the cert and some more checking.

Changed 7 years ago by raimue (Rainer Müller)

Attachment: splitcert.pl added

comment:5 Changed 7 years ago by llattanzi+McF@…

Cc: llattanzi+McF@… added

Cc Me!

Changed 7 years ago by landonf (Landon Fuller)

Attachment: patch-mktemp-fixes-v0 added

Proposed fix

comment:6 Changed 7 years ago by landonf (Landon Fuller)

I've attached patch-mktemp-fixes-v0, which contains a proposed fix; testing+review is much appreciated before I commit it to the repo.

Based on Raimue's comments, I implemented the first option; to check for certificate validity, I actually set up a SecTrustRef with the only anchor being the certificate being tested, and then evaluate self-trust of the certificate. If this fails, the certificate is expired or otherwise untrustable, even if it's marked as trusted.

This approach should resolve the observed problem. Longer-term, I think it's more reasonable to go with the second option (Only export one valid/non-expired certificate per public key), and evaluate certificates according to internal heuristics based on what OpenSSL/gnutls will actually require. However, that requires a better API/model for working with certificates, and probably has to wait for the larger work I'm doing on implementing a certsync Security.framework-backed PKCS#11 module: https://opensource.plausible.coop/src/projects/CRTS/repos/certsync

comment:7 Changed 7 years ago by neverpanic (Clemens Lang)

That doesn't seem to be the correct patch (or I don't understand where to apply it).

Changed 7 years ago by landonf (Landon Fuller)

*Actual* proposed fix.

comment:8 Changed 7 years ago by landonf (Landon Fuller)

I'm a space cadet. Attached the correct patch as patch-handle-expired-certs-v0

comment:9 Changed 7 years ago by raimue (Rainer Müller)

Thank you for working on this. I just installed the patched version and I am now able to verify svn.macports.org against the generated bundle. I also confirmed with a diff against the older cert.pem that the newly generated version only misses expired certificates as expected.

Just a minor note on one comment in the patch, "OpenSSL will simply use whichever certificate is listed first" is not quite correct, as in my experiments just swapping the order was not enough in all cases.

comment:10 Changed 7 years ago by landonf (Landon Fuller)

Resolution: fixed
Status: assignedclosed

Thanks! Comment updated and patch committed in r117895.

comment:11 Changed 7 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

Note: See TracTickets for help on using tickets.