Skip to main content

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

  • From: Erik Trauschke < >
  • To:
  • Cc: Yiteng Zhang < >
  • Subject: [pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e
  • Date: Mon, 21 Oct 2013 16:05:14 -0700



On 10/21/13 10:31 AM, Yiteng Zhang wrote:
On 10/21/13 10:29 AM, Erik Trauschke wrote:


On 10/21/13 10:24 AM, Yiteng Zhang wrote:
On 09/30/13 03:41 PM, Erik Trauschke wrote:


On 09/25/13 12:52 PM, Yiteng Zhang wrote:
Hi,

Please see the following link. Let me know if there is any problem.
Thanks.

https://ips.java.net/webrev/yitezhan/15507548/

Dude, have you actually tried that code?

You are changing the handling of expiring certificates which already
doesn't stop execution and will just print out a message.
Right now you changed it in a way that it will actually stop execution
if you have one publisher for which the certificate will expire soon.
That is even worse than the situation we had before.

What the bug is about is that expired (not expiring) certificates will
stop execution after the first ExpiredCertificate exception is raised.
However, if you refresh multiple publishers with expired certificates
you won't get informed that more than just the first certificate has
expired.

What you want to do is change
src/modules/client/image.py:check_cert_validity()
and instead of raising an ExpiredCertificate exception the first time
an expired cert is encountered, you save it to a temporary variable
and keep going.
After you're through with all the publishers check if any had an
expired cert, then raise the exception with information about all the
expired certs.

You could create a new exception class for it:
class ExpiredCertificate*s*(CertificateError):
    self.errors = []
        ...

Then put all the ExpiredCertificate exceptions in the errors list to
save their content. In the client you can then go through this list
and print the appropriate errors in one block.

Erik






Yiteng

Hi all,

I have used two expired certificates to do the test. Please see the
following link an let me know your comments.

https://ips.java.net/webrev/yitezhan/15507548_2/

Before I look at this, can you show me how the message looked like
before and how it looks now?

Erik

Sure.

When there are two expired certificates,
Before:
# pkg -R /var/tmp/test_image refresh
pkg: Certificate
'/var/tmp/test_image/.org.opensolaris,pkg/ssl/ddeb91e801b8be53ddd87db354ad5c7173564d97'
for publisher 'pkg5-nightly' needed to access
'https://supreme.us.oracle.com/repo2/', has expired.  Please install a
valid certificate.

After:
# pkg -R /var/tmp/test_image refresh
pkg: Certificate
'/var/tmp/test_image/.org.opensolaris,pkg/ssl/ddeb91e801b8be53ddd87db354ad5c7173564d97'
for publisher 'pkg5-nightly', has expired.  Please install a valid
certificate.
Certificate
'/var/tmp/test_image/.org.opensolaris,pkg/ssl/cd98ea9966fb673235725ba169b2119e0d787fd2'
for publisher 'solaris', has expired.  Please install a valid certificate.

Have you played around with alternate ways to print that message to make it more readable? I'm thinking it might be better to print them in a more structured way:

--
# pkg -R /var/tmp/test_image refresh
pkg: One or more certificates have expired. Please replace with a valid certificate.

PUBLISHER      CERTIFICATE LOCATION
pkg5-nightly   /var/.../ssl/ddeb91e801b8be53ddd87db354ad5c7173564d97
solaris        /var/.../ssl/cd98ea9966fb673235725ba169b2119e0d787fd2
--

However, this probably won't work very well due to the length of the cert file names. But I wonder if we even need to include those (except for making it easy for people to delete them).

Anybody have an opinion?

Erik


[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/23/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Xiaobo Shen 10/24/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/24/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/25/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/29/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/30/2013
 
 
Close
loading
Please Confirm
Close