Bug 246614 - certctl(8) silently overwrites certs with same subjects
Summary: certctl(8) silently overwrites certs with same subjects
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Kyle Evans
URL:
Keywords:
Depends on:
Blocks: 228913
  Show dependency treegraph
 
Reported: 2020-05-20 19:51 UTC by Michael Osipov
Modified: 2020-05-21 19:49 UTC (History)
4 users (show)

See Also:


Attachments
git(1) diff against base (1.70 KB, patch)
2020-05-21 18:37 UTC, Kyle Evans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2020-05-20 19:51:58 UTC
When additional trusted certificates are added with the same subject instead of increasing n in <hash>.n it does overwrite <hash>.0.

Here are two certs with the same subject hash:

subject: CN=Siemens Internet CA V1.0,OU=Copyright (C) Siemens AG 2011 All Rights Reserved,serialNumber=ZZZZZZV0,O=Siemens,C=DE
issuer: CN=Baltimore CyberTrust Root,OU=CyberTrust,O=Baltimore,C=IE
source: Siemens
subject hash: 8dc03e53
fingerprint (SHA-1): 1C:7D:56:40:9E:CB:A4:96:8B:8F:FF:41:78:31:86:06:DE:DB:05:32
fingerprint (SHA-256): 3E:BF:5F:FE:C5:82:D2:7C:69:3D:1B:C3:01:04:A6:3B:BB:FC:36:52:C7:8A:95:02:7E:91:B7:F8:8D:AC:63:45
-----BEGIN CERTIFICATE-----
MIIEkTCCA3mgAwIBAgIQDL0FAAzqeadFvWvsl9xaiDANBgkqhkiG9w0BAQsFADBa
MQswCQYDVQQGEwJJRTESMBAGA1UEChMJQmFsdGltb3JlMRMwEQYDVQQLEwpDeWJl
clRydXN0MSIwIAYDVQQDExlCYWx0aW1vcmUgQ3liZXJUcnVzdCBSb290MB4XDTE2
MDMwMTEyNDMzOFoXDTIxMDMwMTEyNDMzOFowgZExCzAJBgNVBAYTAkRFMRAwDgYD
VQQKDAdTaWVtZW5zMREwDwYDVQQFEwhaWlpaWlpWMDE6MDgGA1UECwwxQ29weXJp
Z2h0IChDKSBTaWVtZW5zIEFHIDIwMTEgQWxsIFJpZ2h0cyBSZXNlcnZlZDEhMB8G
A1UEAwwYU2llbWVucyBJbnRlcm5ldCBDQSBWMS4wMIIBIjANBgkqhkiG9w0BAQEF
AAOCAQ8AMIIBCgKCAQEAtOKu51fv/rScjbH0r0Uol/O96u8qGrtBQ+thN0A0ryzw
sgcU4QAp8nPYt6cRSI+29ysibk4xevVNfWCKOTquYntPOSXmyhXaPOKgJ588zr+1
F1//yODojIn+yDIRDR9mix/Znwa1K6ECXismikPP4GtqHv7Pj9T6QVonMHfntCFB
6fO8NN0akVJBZoS9ejtueypkKrYTAtzrA7R102kcp30UDPtrwtDXCFIvjfVmJmp3
0e2QX2kVhIYx7PtX+qLCVPOMujT3wJQ8tnLCTnRAv6MIgz7Ufp7AFF0TdUvVlHQ0
w6XqQJMgvlY4libFBZgW4hZ146STgsD+uRO1YODa8QIDAQABo4IBGTCCARUwHQYD
VR0OBBYEFBGqTpwKTU4XVhuUVzgB8rc1pEllMB8GA1UdIwQYMBaAFOWdWTCCR1jM
rPoIVDaGezq1BE3wMBIGA1UdEwEB/wQIMAYBAf8CAQEwDgYDVR0PAQH/BAQDAgGG
MDQGCCsGAQUFBwEBBCgwJjAkBggrBgEFBQcwAYYYaHR0cDovL29jc3AuZGlnaWNl
cnQuY29tMDoGA1UdHwQzMDEwL6AtoCuGKWh0dHA6Ly9jcmwzLmRpZ2ljZXJ0LmNv
bS9PbW5pcm9vdDIwMjUuY3JsMD0GA1UdIAQ2MDQwMgYEVR0gADAqMCgGCCsGAQUF
BwIBFhxodHRwczovL3d3dy5kaWdpY2VydC5jb20vQ1BTMA0GCSqGSIb3DQEBCwUA
A4IBAQA2D2N0Cnpk+cXutX6rfjebXl9C/rmOpxjHBEOJcUf4In/wZ8KO2ZaHlKdO
l6Hr8Ui4pZe6N/6lItx3aPhTBEd8buo+ApGtwDJIwB5ExsfDmCq9w7xo6ICb/TRP
Z867M411fhkh0pPqecVLSeqx5To0HM1Pixl7q8BmL4kyN4Oz0J/Uuy/UGuFCL7BF
nIkzUPL8oMdlwnUrWMPeHSOqgVinx3DEc4ysZBQ8lSYcAQj2xGLH+8Bict24VGV3
RsdJ2yCtXbg2H6Vj4R2Gtm4GdyK/kFgjd1aLYSxWD82G9IJPv4EvZsQyOtJqfhPn
Wp3ujLiX4hL9XpsnfGjDqzoU4y1K
-----END CERTIFICATE-----

subject: CN=Siemens Internet CA V1.0,OU=Copyright (C) Siemens AG 2011 All Rights Reserved,serialNumber=ZZZZZZV0,O=Siemens,C=DE
issuer: CN=Baltimore CyberTrust Root,OU=CyberTrust,O=Baltimore,C=IE
source: Siemens
subject hash: 8dc03e53
fingerprint (SHA-1): 04:E8:21:81:E0:9E:4E:DC:46:3C:1F:E2:67:41:60:5C:80:E8:18:11
fingerprint (SHA-256): 24:E5:6F:48:60:44:46:D8:A8:37:3B:43:CA:29:D1:A1:C4:97:72:E5:AA:BA:8B:A7:C1:76:62:BD:60:DA:8D:F6
-----BEGIN CERTIFICATE-----
MIIE4TCCA8mgAwIBAgIEBydWSzANBgkqhkiG9w0BAQUFADBaMQswCQYDVQQGEwJJ
RTESMBAGA1UEChMJQmFsdGltb3JlMRMwEQYDVQQLEwpDeWJlclRydXN0MSIwIAYD
VQQDExlCYWx0aW1vcmUgQ3liZXJUcnVzdCBSb290MB4XDTExMDcxNDE2MTE0MVoX
DTIzMDcxNDE2MTExN1owgZExCzAJBgNVBAYTAkRFMRAwDgYDVQQKDAdTaWVtZW5z
MREwDwYDVQQFEwhaWlpaWlpWMDE6MDgGA1UECwwxQ29weXJpZ2h0IChDKSBTaWVt
ZW5zIEFHIDIwMTEgQWxsIFJpZ2h0cyBSZXNlcnZlZDEhMB8GA1UEAwwYU2llbWVu
cyBJbnRlcm5ldCBDQSBWMS4wMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
AQEAtOKu51fv/rScjbH0r0Uol/O96u8qGrtBQ+thN0A0ryzwsgcU4QAp8nPYt6cR
SI+29ysibk4xevVNfWCKOTquYntPOSXmyhXaPOKgJ588zr+1F1//yODojIn+yDIR
DR9mix/Znwa1K6ECXismikPP4GtqHv7Pj9T6QVonMHfntCFB6fO8NN0akVJBZoS9
ejtueypkKrYTAtzrA7R102kcp30UDPtrwtDXCFIvjfVmJmp30e2QX2kVhIYx7PtX
+qLCVPOMujT3wJQ8tnLCTnRAv6MIgz7Ufp7AFF0TdUvVlHQ0w6XqQJMgvlY4libF
BZgW4hZ146STgsD+uRO1YODa8QIDAQABo4IBdTCCAXEwEgYDVR0TAQH/BAgwBgEB
/wIBATBbBgNVHSAEVDBSMEgGCSsGAQQBsT4BADA7MDkGCCsGAQUFBwIBFi1odHRw
Oi8vY3liZXJ0cnVzdC5vbW5pcm9vdC5jb20vcmVwb3NpdG9yeS5jZm0wBgYEVR0g
ADAOBgNVHQ8BAf8EBAMCAQYwgYUGA1UdIwR+MHyAFOWdWTCCR1jMrPoIVDaGezq1
BE3woV6kXDBaMQswCQYDVQQGEwJJRTESMBAGA1UEChMJQmFsdGltb3JlMRMwEQYD
VQQLEwpDeWJlclRydXN0MSIwIAYDVQQDExlCYWx0aW1vcmUgQ3liZXJUcnVzdCBS
b290ggQCAAC5MEcGA1UdHwRAMD4wPKA6oDiGNmh0dHA6Ly93d3cucHVibGljLXRy
dXN0LmNvbS9jZ2ktYmluL0NSTC8yMDI1MDEvY2RwLmNybDAdBgNVHQ4EFgQUEapO
nApNThdWG5RXOAHytzWkSWUwDQYJKoZIhvcNAQEFBQADggEBAAdvcLAkY0C51Hm6
hqgFctsCDXNy+CW0cuIwe2SV4apynjcMU7RB30cnOX7JGiL//o5VlpdGYeSIiI42
N3tZJQ6JURagL5mcBcCMAwSIE8cBa9RiSVW4KfMmun5ldN6q1FXJ28OnxxsJkZLZ
h2Y1J1R9WGxJbYRDJvrRH0icJKJmlD+k4h/EeyC75K2x2xQO5XIKJHUWH/+ChgpV
TcawNX3uH9XrPvaSRr+Troj6+iyE5nWXPolmnlyUrg1rJsMvY8iBtMUUxst10Y/V
nhIcGv8FWoPpaRJVVOgGRmwA7k4TsP3Yv4cQuANX0fQtCRrNq3BKMkUaznsHx90C
6sbFZEM=
-----END CERTIFICATE-----
Comment 1 Kyle Evans freebsd_committer 2020-05-21 18:37:03 UTC
Created attachment 214734 [details]
git(1) diff against base

Here's a tentative diff; there's some other WIP that I've already created one merge conflict with, I'm holding off on committing this until that work is complete (I wouldn't expect this to take long).
Comment 2 Michael Osipov 2020-05-21 19:11:42 UTC
There are several issues with the patch:

* The term "serial" is already taken: by the serial number embedded in the cert as well as serialNumber as part of the DN. c_rehash talks about decimal digit. Maybe "get_decimal" is maybe better?
* While links are created correctly as it seems:
> Reading siemens-cert-14.crt
> Adding 8dc03e53.0 to trust store
> Reading siemens-cert-15.crt
> Adding 8dc03e53.1 to trust store
* 'certctl list' does not show any of them because of:
> for CFILE in *.0; do
You likely will need to add *.1, *.2, ..., *.9
* There is another conceptional issue: *.n is only for the hashed links, not fo scanning, see https://www.openssl.org/docs/man1.1.1/man1/c_rehash.html.
* Please also note that the hashed links for CRLs need to be in <hash>.r<D>
Comment 3 Kyle Evans freebsd_committer 2020-05-21 19:49:42 UTC
(In reply to Michael Osipov from comment #2)

Acknowledged- I'll fix the patch after WIP lands and wrangle you into a formal review on Phabricator. Thanks for the review. =-)