Project

General

Profile

Feature #23916

Allow multiple Subject Alternate Names in CAs and Certs

Added by Suraj Ravichandran over 1 year ago. Updated 6 months ago.

Status:
Done
Priority:
Nice to have
Assignee:
Waqar Ahmed
Category:
Middleware
Target version:
Estimated time:
Sprint:
Severity:
New
Backlog Priority:
Reason for Closing:
Reason for Blocked:
Needs QA:
No
Needs Doc:
Yes
Needs Merging:
Yes
Needs Automation:
No
Support Suite Ticket:
n/a
Hardware Configuration:

Description

Title pretty much states it all.

For detailed discussion please see Ticket: #23844

Screen Shot 2017-10-18 at 3.54.32 PM.png (72.3 KB) Screen Shot 2017-10-18 at 3.54.32 PM.png Bonnie Follweiler, 10/18/2017 12:56 PM
Screen Shot 2017-10-18 at 3.54.20 PM.png (57.7 KB) Screen Shot 2017-10-18 at 3.54.20 PM.png Bonnie Follweiler, 10/18/2017 12:56 PM
Screen Shot 2018-01-14 at 9.29.55 AM.png (178 KB) Screen Shot 2018-01-14 at 9.29.55 AM.png SAN behavior workaround David Riley, 01/14/2018 07:38 AM
12745
12746
13897

Related issues

Related to FreeNAS - Bug #23844: Subject Alternative Name support in SSL certificatesResolved2017-05-08

Associated revisions

Revision c60531c1 (diff)
Added by Nikola Gigic about 1 year ago

feat(middleware): Add ability to add multiple Subject Alternate Names to CA/Cert in addition to the default one which is chosen from the Common Name

Ticket: #23916

Revision d8727565 (diff)
Added by Nikola Gigic about 1 year ago

feat(middleware): Add ability to add multiple Subject Alternate Names to CA/Cert in addition to the default one which is chosen from the Common Name

Ticket: #23916

Revision 19e07094 (diff)
Added by Nikola Gigic about 1 year ago

feat(middleware): Add ability to add multiple Subject Alternate Names… (#306)

Ticket: #23916

Revision 921e0ff5 (diff)
Added by Nikola Gigic about 1 year ago

feat(middleware): Add ability to add multiple Subject Alternate Names… (#306)

Ticket: #23916

Revision bd025572 (diff)
Added by Dru Lavigne about 1 year ago

Add Subject Alternate Names field.
Ticket: #23916

History

#1 Updated by Suraj Ravichandran over 1 year ago

  • Related to Bug #23844: Subject Alternative Name support in SSL certificates added

#2 Updated by Dru Lavigne about 1 year ago

  • Assignee changed from Suraj Ravichandran to William Grzybowski

William, do you think additional work is needed beyond the related ticket? If so, please load balance between Vladimir and Nikola.

#3 Updated by William Grzybowski about 1 year ago

  • Status changed from Screened to Unscreened
  • Assignee changed from William Grzybowski to Nikola Gigic

#4 Updated by Nikola Gigic about 1 year ago

  • Status changed from Unscreened to Screened

#5 Updated by Nikola Gigic about 1 year ago

  • Status changed from Screened to Needs Developer Review

#6 Updated by Dru Lavigne about 1 year ago

  • Assignee changed from Nikola Gigic to William Grzybowski

William: can you perform the review and determine if the target date should be 11.2 (or can be bumped up to 11.1)?

#7 Updated by William Grzybowski about 1 year ago

  • Status changed from Needs Developer Review to Reviewed by Developer
  • Assignee changed from William Grzybowski to Nikola Gigic
  • Target version changed from 11.2-BETA1 to 11.1

#8 Updated by Nikola Gigic about 1 year ago

  • Status changed from Reviewed by Developer to Ready For Release

#9 Updated by Dru Lavigne about 1 year ago

  • Subject changed from Add ability to add multiple Subject Alternate Names to CA/Cert in addition to the default one which is chosen from the Common Name to Allow multiple Subject Alternate Names in CAs and Certs
  • Target version changed from 11.1 to 11.1-BETA1

#10 Updated by Bonnie Follweiler 12 months ago

12745
12746

Test passed in FreeNAS-11-MASTER-201710180506
Screenshots provided

#11 Updated by Dru Lavigne 12 months ago

  • Status changed from Ready For Release to Resolved

#12 Updated by David Riley 9 months ago

13897

I'm not sure this quite works as intended... among other things, when I try to add multiple SANs (especially if IP addresses are mixed in), the certificate is generated incorrectly with the full string I specified for the SANs (plus the interpreted type of the CN, but not the CN itself) as the sole DNS SAN entry.

I can work around this by just specifying the thing as the CN again (to match the expected type) followed by the rest of the non-space-separated string expected by the PyOpenSSL library for the subjectAltName extension, I can generate a valid certificate that works, but it's kind of an ugly workaround.

I think there are a few things that need fixing:

  • The CN is not being inserted in the SAN list supplied to the extension generator (and indeed, the compiled SAN list appears to be getting dumped in the cert's CN field, which is the wrong thing to do). You probably want to:
    1. keep the old req.get_subject().CN = cert_info['common'] that was there before (because the CN should not be a whole SAN list)
    2. try to determine the type of each item in the SAN list and prepend that to each element when you generate cn_san, not just determine the type of the CN and prepend it to the joined list (which wouldn't have worked well anyway, since you've put the CN portion at the end of the list anyway); alternatively, it might be more prudent to let/make people specify the types themselves, because it's hard to guess if they're including other types than IP and DNS
    3. join the elements of cn_san_list without spaces, since the generator doesn't seem to like that (see documentation here: https://www.openssl.org/docs/manmaster/man5/x509v3_config.html#STANDARD-EXTENSIONS)
  • Currently, you supply cert_info['san'] to the extension constructur when I think you meant to supply the generated cn_san string; as mentioned above, you probably want to consider how you're prepending the type to the string, because the way you have it currently won't work well with the current ordering when you join up the cn_san string. If it's actually normal to have multiple names in the CN (I feel like it's not?), it won't work properly at all.

I've attached a screenshot with an example of how I finally made it work with my server (they're all internal addresses, and the IPv6 is not externally accessible, so I'm not overly concerned about security here).

I can modify my local code to make the necessary changes, and possibly file a PR against the main tree on Github; would that be helpful?

#13 Updated by Dru Lavigne 9 months ago

  • Status changed from Resolved to Unscreened
  • Assignee changed from Nikola Gigic to Vladimir Vinogradenko
  • Target version changed from 11.1-BETA1 to 11.1-U2

#14 Avatar?id=13649&size=24x24 Updated by Ben Gadd 9 months ago

  • Due date set to 02/02/2018

#15 Updated by Dru Lavigne 9 months ago

  • Status changed from Unscreened to Not Started

#16 Updated by Vladimir Vinogradenko 9 months ago

  • Status changed from Not Started to Broken
  • Reason for Blocked set to Waiting for feedback

David, sorry for the delay, yes, a PR would be great.

#17 Updated by Vladimir Vinogradenko 9 months ago

  • Status changed from Broken to Blocked

#18 Avatar?id=13649&size=24x24 Updated by Ben Gadd 9 months ago

  • Due date changed from 02/02/2018 to 02/12/2018

Due date updated to reflect the code freeze for 11.1U2.

#19 Updated by David Riley 9 months ago

OK, I'll see what I can do. My FreeNAS installation is in a bit of flux at the moment, but I'll try to get something in well before the due date.

#20 Avatar?id=13649&size=24x24 Updated by Ben Gadd 9 months ago

  • Severity set to New

#21 Updated by Dru Lavigne 8 months ago

  • Target version changed from 11.1-U2 to 11.2-BETA1

#22 Updated by Dru Lavigne 7 months ago

  • Target version changed from 11.2-BETA1 to 11.2

#23 Avatar?id=13649&size=24x24 Updated by Ben Gadd 7 months ago

  • Assignee changed from Vladimir Vinogradenko to Waqar Ahmed

#24 Avatar?id=13649&size=24x24 Updated by Ben Gadd 7 months ago

  • Due date deleted (02/12/2018)

#25 Updated by Waqar Ahmed 7 months ago

Hi David! Could you kindly confirm if you have any update regarding this? Many thanks

#26 Updated by David Riley 7 months ago

Hi, deepest apologies, but I haven't been able to get a PR out because I haven't had the chance to put together the development environment to test. If I can't in another few days, I'll try to at least leave more explicit instructions or maybe just send a diff (which would be pretty easy).

#27 Updated by Waqar Ahmed 7 months ago

Thank you David for your response. Looking forward to hearing from you.

#28 Updated by Dru Lavigne 6 months ago

  • Status changed from Blocked to Done
  • Target version changed from 11.2 to 11.1-U1
  • Reason for Blocked deleted (Waiting for feedback)

David: I'm going to mark this ticket back as resolved for 11.1. If you get a chance to make a PR or create instructions, please create a new ticket so we can track those additions. Thanks!

#29 Updated by Dru Lavigne 6 months ago

  • Target version changed from 11.1-U1 to 11.1-BETA1

#30 Updated by David Riley 6 months ago

Dru Lavigne wrote:

David: I'm going to mark this ticket back as resolved for 11.1. If you get a chance to make a PR or create instructions, please create a new ticket so we can track those additions. Thanks!

Sounds reasonable. Sorry for not being able to file a PR yet; it's been a busy few months but I'm slowly clearing my backlog. I haven't set up a proper build system yet, but I could probably check out the source tree and use that to file a PR against that after testing the script on my live machine.

Also available in: Atom PDF