Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing / misleading statements, and formatting in CREATE and ALTER CERTIFICATE #1972

Merged
merged 9 commits into from Apr 24, 2019

Conversation

Projects
None yet
@srutzky
Copy link
Contributor

commented Apr 23, 2019

CREATE CERTIFICATE

  1. Fixed description of "WITH PRIVATE KEY" to account for "BINARY =" option introduced in SQL Server 2012.

  2. Fixed specification of BINARY for certificate (public key):

    1. added "BINARY ="
    2. moved above private key options to be properly grouped with public key options
    3. added "Applies to"
  3. Updated remarks to indicate that certificates can also be loaded from a binary constant (as of SQL Server 2012).

  4. Added note about "CLR strict security" server config option at the bottom of "Example C" (as of SQL Server 2017).

  5. Added links to CERT_ID and CERTPROPERTY functions in "See Also" section.

ALTER CERTIFICATE

  1. Fixed main description as it was both incomplete and misleading.

  2. Fixed syntax

  3. Fixed formatting in "Arguments" section to be consistent with formatting found in CREATE CERTIFICATE page (and I have been told to specifically not escape underscores unless they are causing formatting issues).

  4. Changed "password" in DECRYPTION BY PASSWORD and ENCRYPTION BY PASSWORD clauses to be "current_password" and "new_password", respectively, to hopefully be more intuitive to those who are less experienced with certificates.

  5. Removed "from a file" wording in "Remarks" section since the private key can be imported from multiple sources

  6. Changed "Example A" to be "Removing a private key". Previously it was "Changing the password of a certificate", which is identical to "Example B", except the title is slightly different and the order of the ENCRYPTION BY PASSWORD and DECRYPTION BY PASSWORD clauses is different. No need for redundant examples.

  7. Reversed the order of the ENCRYPTION BY PASSWORD and DECRYPTION BY PASSWORD clauses in "Example B" because it is probably more intuitive to have decryption first since that is actually what happens first.

  8. Fixed typo in "Example C": removed extraneous backslash in file path.

  9. Added links to CERTENCODED, CERTPRIVATEKEY, CERT_ID, and CERTPROPERTY functions in "See Also" section.

Both

For details / testing, please see: https://sqlquantumleap.com/2019/04/22/can-a-certificates-private-key-be-imported-restored-from-a-binary-literal-hex-bytes/

srutzky added some commits Apr 22, 2019

Fixed missing and misleading statements
1. Fixed description of "WITH PRIVATE KEY" to account for "BINARY =" option introduced in SQL Server 2012.
2. Fixed specification of BINARY for certificate (public key):
    1. added "BINARY ="
    2. moved above private key options to be properly grouped with public key options
    3. added "Applies to"
3. Updated remarks to indicate that certificates can also be loaded from a binary constant (as of SQL Server 2012).
4. Added note about "CLR strict security" server config option at the bottom of "Example C" (as of SQL Server 2017).
5. Added links to CERT_ID and CERTPROPERTY functions in "See Also" section.

For details /  testing, please see:  https://sqlquantumleap.com/2019/04/22/can-a-certificates-private-key-be-imported-restored-from-a-binary-literal-hex-bytes/
Update create-certificate-transact-sql.md
For the note after Example C regarding  'CLR strict security':

1. Fixed link to  'CLR strict security' documentation page
2. Improved wording.
Fixed missing and misleading statements
1. Fixed main description as it was both incomplete and misleading.

2. Fixed syntax

3. Fixed formatting in "Arguments" section to be consistent with formatting found in CREATE CERTIFICATE page (and I have been told to specifically _not_ escape underscores unless they are causing formatting issues).

4. Changed "password" in `DECRYPTION BY PASSWORD` and `ENCRYPTION BY PASSWORD` clauses to be "current_password" and "new_password", respectively, to hopefully be more intuitive to those who are less experienced with certificates.

5. Removed "from a file" wording in "Remarks" section since the private key can be imported from multiple sources

6. Changed "Example A" to be "Removing a private key". Previously it was "Changing the password of a certificate", which is identical to "Example B", except the title is slightly different and the order of the `ENCRYPTION BY PASSWORD` and `DECRYPTION BY PASSWORD` clauses is different. No need for redundant examples.

7. Reversed the order of the `ENCRYPTION BY PASSWORD` and `DECRYPTION BY PASSWORD` clauses in "Example B" because it is probably more intuitive to have decryption first since that is actually what happens first.

8. Fixed typo in "Example C": removed extraneous backslash in file path.

9. Added links to  CERTENCODED, CERTPRIVATEKEY, CERT_ID, and CERTPROPERTY functions in "See Also" section.


For details /  testing, please see:  https://sqlquantumleap.com/2019/04/22/can-a-certificates-private-key-be-imported-restored-from-a-binary-literal-hex-bytes/
@PRMerger13

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

@srutzky : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@PRMerger12

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

@VanMSFT : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@PRMerger12

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

@VanMSFT : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@VanMSFT

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

Hi @srutzky! Thanks for the detailed update. We really appreciate your testing and knowledge on this. I went through details, and everything looks applicable. Besides a slight change to syntax, I'll go ahead and approve. Thanks again!

#sign-off

srutzky added a commit to srutzky/sql-docs that referenced this pull request Apr 23, 2019

Fix missing / misleading statements; fix formatting
1. Updated "Arguments" section to conform to most other pages:
    1. Added "certname" argument.
    2. Added argument name to each option.
    3. Added "WITH PRIVATE KEY" argument as it mirrors `CREATE CERTIFICATE`.
    4. Corrected default path description (tested in SQL Server 2017 CU14 and 2019 CTP 2.4).
    5. Added LocalDB info to path description (tested in SQL Server Express LocalDB 2012 SP4).

2. Updated "Remarks" section:
    1. Passwords are to protect private keys, not certificates.
    2. Added info about private key file format.
    3. Clarified that restoring a certificate does not require the private key.
    4. Added info about restoring a private key to an existing certificate via `ALTER CERTIFICATE`.

3. Added links to CERTENCODED, CERTPRIVATEKEY, CERT_ID, and CERTPROPERTY functions in "See Also" section.


These updates are related to updates proposed for `CREATE CERTIFICATE` and `ALTER CERTIFICATE` via MicrosoftDocs#1972 , and to the following post: https://sqlquantumleap.com/2019/04/22/can-a-certificates-private-key-be-imported-restored-from-a-binary-literal-hex-bytes/
@srutzky

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Cool. Good catch on the syntax change. And you are quite welcome 😺

There are one or two minor additions I just thought of for ALTER CERTIFICATE, partially related to the PR I just submitted for BACKUP CERTIFICATE : #1976 . Is it too late to add them to this PR?

@VanMSFT

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

#hold-off

@srutzky - Go ahead and add to this PR if needed.

Minor additions for private keys
1. Moved the "REMOVE PRIVATE KEY" clause to the top to be in the same order as shown in the syntax (that is how most other pages are done)
2. Added clarification regarding what private keys are used for in "Remarks", in the paragraph about removing the private key.
3. Added note to explicitly state how to accomplish changing the password rather than importing, else it was only mentioned in the description and shown in an example.
4. Added `CERTPRIVATEKEY` link to "Important" note regarding backing up the private key before removing it (this became an option in SQL Server 2012).
@PRMerger7

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

@srutzky : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@srutzky

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Additional changes consist of:

  1. Moved the "REMOVE PRIVATE KEY" clause to the top to be in the same order as shown in the syntax (that is how most other pages are done)
  2. Added clarification regarding what private keys are used for in "Remarks", in the paragraph about removing the private key.
  3. Added note to explicitly state how to accomplish changing the password rather than importing, else it was only mentioned in the description and shown in an example.
  4. Added CERTPRIVATEKEY link to "Important" note regarding backing up the private key before removing it (this became an option in SQL Server 2012).
@PRMerger10

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

@srutzky : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@srutzky

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Final edit:

  1. Added info about default file location. Test case is in script linked in PR for BACKUP CERTIFICATE: #1976
@srutzky

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

@VanMSFT all done now...thanks..

@srutzky srutzky changed the title Fixed missing and misleading statements in CREATE and ALTER CERTIFICATE Fixed missing / misleading statements, and formatting in CREATE and ALTER CERTIFICATE Apr 23, 2019

@srutzky srutzky changed the title Fixed missing / misleading statements, and formatting in CREATE and ALTER CERTIFICATE Fix missing / misleading statements, and formatting in CREATE and ALTER CERTIFICATE Apr 23, 2019

@Jak-MS

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

@VanMSFT - if you approve, can you #sign-off ? After the PR is signed off, we'll check it against the PR review criteria before it gets merged. thanks.

@VanMSFT

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

Reviewed. Thanks @srutzky!
#sign-off

@srutzky

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Yer welcome.

@PRMerger9

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

@GitHubber17 : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@PRMerger9

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

@GitHubber17 : Thanks for your contribution! The author, @VanMSFT, has been notified to review your proposed change.

@GitHubber17 GitHubber17 merged commit f72cb3f into MicrosoftDocs:live Apr 24, 2019

1 check passed

license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.