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

[powershell-experimental] : http signature authentication implementation #6176

Merged
merged 7 commits into from
May 7, 2020

Conversation

Ghufz
Copy link
Contributor

@Ghufz Ghufz commented May 5, 2020

http signature authentication implementation for powershell sdk
Example :
Set-ConfigurationApiKey -Id 5e96bcd57564612d30148d2b/5e96c0f57564612d3014a20f/5ea03daa756461233050843a -ApiKey "C:\SecretKey.txt"
Set-Configuration -BaseUrl "https://<>/api/v1" -SkipCertificateCheck
Get-Configuration

Above cmdlet set the environment for http signature auth

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@wing328

@Ghufz Ghufz changed the title [powershell-experimental] : http signature authntication implementation [powershell-experimental] : http signature authentication implementation May 6, 2020
@@ -626,7 +629,7 @@ public void processOpts() {
@SuppressWarnings("static-method")
@Override
public String escapeText(String input) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove these empty spaces in my next PR to the PowerShell generator

@wing328
Copy link
Member

wing328 commented May 7, 2020

As discussed, we can go with this for initial implementation. I have suggestions later on how to pass the HTTP signature authentication key to the client

@wing328 wing328 added this to the 5.0.0 milestone May 7, 2020
@wing328 wing328 merged commit 13f329e into OpenAPITools:master May 7, 2020
Get the API key Id and API key file path.

.DESCRIPTION
Get the API key Id and API key file path. If no api prefix is provided then it use default api key prefix 'Signature'
Copy link
Contributor

@sebastien-rosset sebastien-rosset May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: document the meaning of api key prefix. Is this really needed?

It looks like this is used to control the prefix of the "Authorization" HTTP header, but I don't see in the "HTTP signature" spec any option to control the name of the attribute. This is supposed to be exactly "Signature". On the other hand, other parameters are currently hard-coded and should be exposed as configuration parameters.

Example:

Authorization: Signature keyId="rsa-key-1",algorithm="rsa-sha256",
   headers="(request-target) host date digest content-length",
   signature="Base64(RSA-SHA256(signing string))"

{{>partial_header}}
<#
.SYNOPSIS
Get the API key Id and API key file path.
Copy link
Contributor

@sebastien-rosset sebastien-rosset May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some overview description explaining this code is for HTTP signature. See python example

I just realized in Python we should give a better name instead of "private key" because in the HTTP signature spec, both asymmetric and symmetric keys are supported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about HttpSignatureKey?

{{>partial_header}}
<#
.SYNOPSIS
Get the API key Id and API key file path.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the same terminology as the HTTP signature spec:

The "signature" authentication scheme is based on the model that the client must authenticate itself with a digital signature produced by either a private asymmetric key (e.g., RSA) or a shared symmetric key (e.g., HMAC). The scheme is parameterized enough such that it is not bound to any particular key type or signing algorithm. However, it does explicitly assume that clients can send an HTTP Date header.

The spec does not talk about "API keys".

Gets the headers for http signed auth.

.DESCRIPTION
Gets the headers for the http signed auth. It use (targetpath), date, host and body digest to create authorization header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar: "it use"

Gets the headers for http signed auth.

.DESCRIPTION
Gets the headers for the http signed auth. It use (targetpath), date, host and body digest to create authorization header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HTTP signature spec does not mandate this specific sequence of HTTP headers. The headers are supposed to be provided by the caller.


<#
.SYNOPSIS
Gets the headers for http signed auth.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use case consistently (HTTP).

#Check for Authentication type
$apiKeyInfo = Get-{{{apiNamePrefix}}}APIKeyInfo
if ($null -eq $apiKeyInfo) {
throw "Unable to reterieve the api key info "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: reterieve

#Check for Authentication type
$apiKeyInfo = Get-{{{apiNamePrefix}}}APIKeyInfo
if ($null -eq $apiKeyInfo) {
throw "Unable to reterieve the api key info "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove space before double quote


#get the body digest
$bodyHash = Get-{{{apiNamePrefix}}}StringHash -String $Body
$Digest = [String]::Format("SHA-256={0}", [Convert]::ToBase64String($bodyHash))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HASH algorithm should be provided as an input parameter.


$hashedString = Get-{{{apiNamePrefix}}}StringHash -String $stringToSign
$signedHeader = Get-{{{apiNamePrefix}}}RSASHA256SignedString -APIKeyFilePath $apiKeyInfo.ApiKeyFilePath -DataToSign $hashedString
$authorizationHeader = [string]::Format("{0} keyId=""{1}"",algorithm=""rsa-sha256"",headers=""(request-target) date host digest"",signature=""{2}""",
Copy link
Contributor

@sebastien-rosset sebastien-rosset May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signature algorithm should not be hard-coded to "rsa-sha256" because 1) this algorithm has been deprecated in the HTTP signature spec and 2) the client should be able to decide which signature algorithm to use.

$dateTime = Get-Date
$currentDate = $dateTime.ToUniversalTime().ToString("r")

$requestTargetPath = [string]::Format("{0} {1}{2}",$Method.ToLower(),$UriBuilder.Path.ToLower(),$UriBuilder.Query)
Copy link
Contributor

@sebastien-rosset sebastien-rosset May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL must not be lowercase, per spec.


$HttpSignedHeader["Date"] = $currentDate
$HttpSignedHeader["Host"] = $TargetHost
$HttpSignedHeader["Content-Type"] = "application/json"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content-type header should not be hard-coded to application/json

$authorizationHeader = [string]::Format("{0} keyId=""{1}"",algorithm=""rsa-sha256"",headers=""(request-target) date host digest"",signature=""{2}""",
$apiKeyInfo.ApiKeyPrefix, $apiKeyInfo.ApiKeyId, $signedHeader)

$HttpSignedHeader["Date"] = $currentDate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list of signed HTTP headers should be configurable by the client.

)
try {

$rsa_provider_path = Join-Path -Path $PSScriptRoot -ChildPath "{{{apiNamePrefix}}}RSAEncryptionProvider.cs"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should support ECDSA keys.

.Outputs
String
#>
Function Get-{{{apiNamePrefix}}}StringHash([String] $String, $HashName = "SHA256") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be hard-coded to SHA256

$h_targetHost,$h_digest)

$hashedString = Get-{{{apiNamePrefix}}}StringHash -String $stringToSign
$signedHeader = Get-{{{apiNamePrefix}}}RSASHA256SignedString -APIKeyFilePath $apiKeyInfo.ApiKeyFilePath -DataToSign $hashedString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should make it clear which specific RSA signing algorithm is used. Is it RSASSA-PSS or RSA PKCS v1.5? Ideally this would be configurable by the client.

Get the API key Id and API key file path.

.DESCRIPTION
Get the API key Id and API key file path. If no api prefix is provided then it use default api key prefix 'Signature'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to document which of the methods are public versus which one are for internal use.

{
RSACryptoServiceProvider cipher = new RSACryptoServiceProvider();
cipher = GetRSAProviderFromPemFile(private_key_path);
RSAPKCS1SignatureFormatter RSAFormatter = new RSAPKCS1SignatureFormatter(cipher);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, finally I found the code location that configures the specific signature algorithm. This needs to be parameterized, with support for RSA PKCS v1.5 and RSA SSA. Also, this needs to be documentation in the user documentation.

jimschubert added a commit that referenced this pull request May 8, 2020
…-5.0

* origin/master: (78 commits)
  [powershell-experimental] : http signature authentication implementation (#6176)
  Add missing AnyType type mapping (#6196)
  remove pubspec.lock (#6208)
  Minor fixes post-release (#6204)
  [cpp][Qt5] Add the ability to pass QNetworkAccessManager as a parameter  (#6053)
  comment out dart2 test due to failure
  adds the missing typeMapping for AnyType (#6199)
  [Rust Server] Support boolean headers, and fix panic handling headers (#6056)
  update samples
  update 5.0.0 release date
  update readme with new release
  Prepare 4.3.1 release (#6187)
  Fix #6157: Updated native template to fix null async return (#6168)
  Show description when summary is missing (#6159)
  Make the array items optional (#6132)
  [aspnetcore] test petstore samples in drone.io (#6148)
  fix bearer auth in c# netcore async call (#6136)
  skip web.config for aspnetcore 3.x (#6147)
  Adds memoization and deserialization through 2 or more discriminators (#6124)
  Implement Asp.Net Core 3.0/3.1 generator (#6009) (#6025)
  ...
@wing328
Copy link
Member

wing328 commented May 8, 2020

@Ghufz for further enhancements, please pull the latest master and work on the powershell generator instead as we've replaced it with the powershell-experimental generator in the upcoming 5.0.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants