Project

General

Profile

Actions
Copy link

Bug #12034

closed

Certificate Manager performs redundant escaping of special characters in certificate DN fields

Added by Viktor Gurov almost 3 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Viktor Gurov
Category:
Certificates
Target version:
2.6.0
Start date:
06/14/2021
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
22.01
Release Notes:
Default
Affected Version:
2.5.1
Affected Architecture:

Description

We are facing issue while generating Cert/CSR form Cert. Manager whenever there is comma (,) in Organization same.
The Cert/CSR generated inserts " \ " (backslash) character before comma due to which the SSL cert generation fails.

# openssl x509 -in commacert_test.crt -text -noout
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 2 (0x2)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = freeradius-temp-ca
        Validity
            Not Before: Jun 14 09:45:23 2021 GMT
            Not After : Jun 12 09:45:23 2031 GMT
        Subject: CN = commacert, O = "Comma \\, test" 
...

Files

testcommacert.crt (1.31 KB) testcommacert.crt Viktor Gurov, 06/14/2021 11:33 AM
Actions
Copy link
 #1

Updated by Viktor Gurov almost 3 years ago

according to https://datatracker.ietf.org/doc/html/rfc4514 "," (comma) must be escaped:

   Each octet of the character to be escaped is replaced by a backslash
   and two hex digits, which form a single octet in the code of the
   character.  Alternatively, if and only if the character to be escaped
   is one of

      ' ', '"', '#', '+', ',', ';', '<', '=', '>', or '\'
      (U+0020, U+0022, U+0023, U+002B, U+002C, U+003B,
       U+003C, U+003D, U+003E, U+005C, respectively)

   it can be prefixed by a backslash ('\' U+005C).

but here's the double backslash:

Subject: CN = commacert, O = "Comma \\, test"

which is issue

see https://github.com/pfsense/pfsense/blob/55dc00701011c2547a55dabf7716d2939cadc509/src/etc/inc/certs.inc#L1471

Actions
Copy link
 #2

Updated by Jim Pingle almost 3 years ago

  • Status changed from New to Feedback

I can't reproduce this here. The code is already doing the escaping so the user doesn't need to worry about it. If I enter the Org as "Comma, Test" it comes out with a single backslash in the certificate data as expected. It shows up properly in the GUI with the escaping hidden ("Comma, Test") and OpenSSL shows:

        Subject: CN=commatest, O=Comma\, Test

Exactly which inputs are given to achieve the stated result above?

Actions
Copy link
 #3

Updated by Viktor Gurov almost 3 years ago

Jim Pingle wrote:

I can't reproduce this here. The code is already doing the escaping so the user doesn't need to worry about it. If I enter the Org as "Comma, Test" it comes out with a single backslash in the certificate data as expected. It shows up properly in the GUI with the escaping hidden ("Comma, Test") and OpenSSL shows:

[...]

Exactly which inputs are given to achieve the stated result above?

Organization = "Test , comma"
it shows correct DN in the WebGUI:

DN: /CN=testcommacert/O=Test , comma

but double-backslash in the `openssl x509` output:

$ openssl x509 -in testcommacert.crt -text -noout | head -n 30
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 5 (0x5)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = freeradius-temp-ca
        Validity
            Not Before: Jun 14 16:30:45 2021 GMT
            Not After : Jun 12 16:30:45 2031 GMT
        Subject: CN = testcommacert, O = "Test \\, comma" 
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (2048 bit)
                Modulus:
...

see the attachment

Actions
Copy link
 #4

Updated by Jim Pingle almost 3 years ago

Interesting. Looks like the output varies by platform or OpenSSL version. Where I initially checked that was on an older Mac and it outputs only "\,", but on Linux and FreeBSD with openssl 1.1.x it shows "\\,".

Opening it in Windows also shows "Test \, comma".

Are you certain it's not actually correct and maybe the output from OpenSSL is what's not as expected?

Actions
Copy link
 #5

Updated by Marcos M almost 3 years ago

Here's some more details when examining certificates generated from different sources:

  1. Cert from third-party app default: O=Test\, INC
  2. Cert from Third-party app with escaped characters: O=Test\\\, INC
  3. Cert from pfSense 2.4.5p1/21.05 default: O=Test\\\, INC
  4. Cert from pfSense 2.4.5p1/21.05 with escaped characters: O=Test\\\, INC

From this, it seems that any special character defined in the pfSense cert_escape_x509_chars function will be passed as escaped to the php function openssl_csr_new which then seems to escape both \ and , resulting in \\\, entered into the certificate.

Additionally, I noticed that if php is built with LDAP support, it can use the ldap_escape function which includes all the characters in cert_escape_x509_chars as well as \r
https://github.com/php/php-src/blob/4c0231ebc38fa27fe2c037c7ca087de5e16bdc2a/ext/ldap/ldap.c#L3899

If cert_escape_x509_chars is to be used, perhaps it should also include \r?

Actions
Copy link
 #6

Updated by Viktor Gurov almost 3 years ago

it looks like `cert_escape_x509_chars()` is not needed - `openssl_csr_new()` automatically adds double quotes in case of a special character.

tests without `cert_escape_x509_chars()`:

$ openssl x509 -in commatest.crt -text -noout | head
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 7 (0x7)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = freeradius-temp-ca
        Validity
            Not Before: Jun 16 11:56:29 2021 GMT
            Not After : Jun 14 11:56:29 2031 GMT
        Subject: CN = commatest, O = "Comma , test" 

$ openssl x509 -in nocommatest.crt -text -noout | head
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 8 (0x8)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = freeradius-temp-ca
        Validity
            Not Before: Jun 16 12:01:02 2021 GMT
            Not After : Jun 14 12:01:02 2031 GMT
        Subject: CN = nocommatest, O = No comma test

Actions
Copy link
 #8

Updated by Jim Pingle almost 3 years ago

  • Status changed from Feedback to Pull Request Review
  • Target version set to 2.6.0
  • Plus Target Version set to 21.09
Actions
Copy link
 #9

Updated by Renato Botelho almost 3 years ago

  • Status changed from Pull Request Review to Feedback
  • Assignee set to Viktor Gurov

PR has been merged. Thanks!

Actions
Copy link
 #10

Updated by Viktor Gurov almost 3 years ago

  • % Done changed from 0 to 100
Actions
Copy link
 #11

Updated by Marcos M almost 3 years ago

  • Status changed from Feedback to Resolved

Looks good.

Performing the same tests that previously yielded extra escape characters now correctly shows just one: O=Test\, INC

Actions
Copy link
 #12

Updated by Jim Pingle over 2 years ago

  • Subject changed from Certificate Manager inserts a backslash before the comma in the certificate fields to Certificate Manager performs redundant escaping of special characters in certificate DN fields

Updating subject for release notes.

Actions
Copy link
 #13

Updated by Jim Pingle over 2 years ago

  • Plus Target Version changed from 21.09 to 22.01
Actions
Copy link

Also available in: Atom PDF