Address review feedback

This commit is contained in:
vishalnayak 2016-10-04 14:57:04 -04:00
parent dda2e81895
commit 84c8caefca
3 changed files with 57 additions and 58 deletions

View File

@ -97,13 +97,14 @@ func pathConfigCertificate(b *backend) *framework.Path {
Description: "AWS Public cert required to verify PKCS7 signature of the EC2 instance metadata.",
},
"type": {
Type: framework.TypeString,
Type: framework.TypeString,
Default: "pkcs7",
Description: `
Takes the value of either "pkcs7" or "identity", indicating the type of
document which can be verified using the given certificate. The reason is that
the PKCS#7 document will have a DSA digest and the identity signature will have
an RSA signature, and accordingly the public certificates to verify those also
vary.`,
vary. Defaults to "pkcs7".`,
},
},
@ -351,18 +352,6 @@ func (b *backend) pathConfigCertificateCreateUpdate(
return logical.ErrorResponse("missing certificate name"), nil
}
certType := data.Get("type").(string)
if certType == "" {
return logical.ErrorResponse("missing certificate type"), nil
}
switch strings.ToLower(certType) {
case "pkcs7":
case "identity":
default:
return logical.ErrorResponse("invalid certificate type"), nil
}
b.configMutex.Lock()
defer b.configMutex.Unlock()
@ -375,6 +364,21 @@ func (b *backend) pathConfigCertificateCreateUpdate(
certEntry = &awsPublicCert{}
}
// Check if type information is provided
certTypeRaw, ok := data.GetOk("type")
if ok {
certEntry.Type = strings.ToLower(certTypeRaw.(string))
} else if req.Operation == logical.CreateOperation {
certEntry.Type = data.Get("type").(string)
}
switch certEntry.Type {
case "pkcs7":
case "identity":
default:
return logical.ErrorResponse(fmt.Sprintf("invalid certificate type %q", certEntry.Type)), nil
}
// Check if the value is provided by the client
certStrData, ok := data.GetOk("aws_public_cert")
if ok {
@ -403,9 +407,6 @@ func (b *backend) pathConfigCertificateCreateUpdate(
return logical.ErrorResponse("invalid certificate; failed to decode and parse certificate"), nil
}
// Add the certificate type to the storage entry
certEntry.Type = certType
// If none of the checks fail, save the provided certificate
if err := b.nonLockedSetAWSPublicCertificateEntry(req.Storage, certName, certEntry); err != nil {
return nil, err

View File

@ -221,9 +221,8 @@ func validateMetadata(clientNonce, pendingTime string, storedIdentity *whitelist
// Verifies the integrity of the instance identity document using its SHA256
// RSA signature. After verification, returns the unmarshaled instance identity
// document.
func (b *backend) verifyInstanceIdentitySignature(s logical.Storage, identity, signature string) (*identityDocument, error) {
identity = strings.TrimSpace(identity)
if identity == "" {
func (b *backend) verifyInstanceIdentitySignature(s logical.Storage, identityBytes []byte, signature string) (*identityDocument, error) {
if len(identityBytes) == 0 {
return nil, fmt.Errorf("missing instance identity document")
}
@ -248,10 +247,10 @@ func (b *backend) verifyInstanceIdentitySignature(s logical.Storage, identity, s
// Check if any of the certs registered at the backend can verify the
// signature
for _, cert := range publicCerts {
err := cert.CheckSignature(x509.SHA256WithRSA, []byte(identity), []byte(signature))
err := cert.CheckSignature(x509.SHA256WithRSA, identityBytes, []byte(signature))
if err == nil {
var identityDoc identityDocument
if decErr := jsonutil.DecodeJSON([]byte(identity), &identityDoc); decErr != nil {
if decErr := jsonutil.DecodeJSON(identityBytes, &identityDoc); decErr != nil {
return nil, decErr
}
return &identityDoc, nil
@ -321,13 +320,13 @@ func (b *backend) parseIdentityDocument(s logical.Storage, pkcs7B64 string) (*id
func (b *backend) pathLoginUpdate(
req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
identityDocB64 := data.Get("identity").(string)
identityDoc := ""
var identityDocBytes []byte
var err error
if identityDocB64 != "" {
identityDocBytes, err := base64.StdEncoding.DecodeString(identityDocB64)
if err != nil {
identityDocBytes, err = base64.StdEncoding.DecodeString(identityDocB64)
if err != nil || len(identityDocBytes) == 0 {
return logical.ErrorResponse("failed to base64 decode the instance identity document"), nil
}
identityDoc = string(identityDocBytes)
}
signatureB64 := data.Get("signature").(string)
@ -345,15 +344,14 @@ func (b *backend) pathLoginUpdate(
// Either the pkcs7 signature of the instance identity document, or
// the identity document itself along with its SHA256 RSA signature
// needs to be provided.
if pkcs7B64 == "" && (identityDoc == "" && signature == "") {
if pkcs7B64 == "" && (len(identityDocBytes) == 0 && signature == "") {
return logical.ErrorResponse("either pkcs7 or a tuple containing the instance identity document and its SHA256 RSA signature needs to be provided"), nil
} else if pkcs7B64 != "" && (identityDoc != "" && signature != "") {
} else if pkcs7B64 != "" && (len(identityDocBytes) != 0 && signature != "") {
return logical.ErrorResponse("both pkcs7 and a tuple containing the instance identity document and its SHA256 RSA signature is supplied; provide only one"), nil
}
// Verify the signature of the identity document and unmarshal it
var identityDocParsed *identityDocument
var err error
if pkcs7B64 != "" {
identityDocParsed, err = b.parseIdentityDocument(req.Storage, pkcs7B64)
if err != nil {
@ -363,7 +361,7 @@ func (b *backend) pathLoginUpdate(
return logical.ErrorResponse("failed to verify the instance identity document using pkcs7"), nil
}
} else {
identityDocParsed, err = b.verifyInstanceIdentitySignature(req.Storage, identityDoc, signature)
identityDocParsed, err = b.verifyInstanceIdentitySignature(req.Storage, identityDocBytes, signature)
if err != nil {
return nil, err
}

View File

@ -515,12 +515,12 @@ The response will be in JSON. For example:
<ul>
<li>
<span class="param">type</span>
<span class="param-flags">required</span>
Takes the value of either "pkcs7" or "identity", indicating the type of
document which can be verified using the given certificate. The reason is that
the PKCS#7 document will have a DSA digest and the identity signature will have
an RSA signature, and accordingly the public certificates to verify those also
vary.
<span class="param-flags">optional</span>
Takes the value of either "pkcs7" or "identity", indicating the type of
document which can be verified using the given certificate. The PKCS#7 document
will have a DSA digest and the identity signature will have an RSA signature,
and accordingly the public certificates to verify those also vary. Defaults to
"pkcs7".
</li>
</ul>
</dd>
@ -1142,11 +1142,11 @@ instance can be allowed to gain in a worst-case scenario.
<dl class="api">
<dt>Description</dt>
<dd>
Fetch a token. This endpoint verifies the pkcs7 signature of the instance
identity document. Verifies that the instance is actually in a running state.
Cross checks the constraints defined on the role with which the login is being
performed. As an alternative to pkcs7 signature, the identity document along
with its RSA digest can be supplied to this endpoint.
Fetch a token. This endpoint verifies the pkcs7 signature of the instance
identity document. Verifies that the instance is actually in a running state.
Cross checks the constraints defined on the role with which the login is being
performed. As an alternative to pkcs7 signature, the identity document along
with its RSA digest can be supplied to this endpoint.
</dd>
<dt>Method</dt>
@ -1171,41 +1171,41 @@ with its RSA digest can be supplied to this endpoint.
<li>
<span class="param">identity</span>
<span class="param-flags">required</span>
Base64 encoded EC2 instance identity document. This needs to be supplied along
with 'signature' parameter.
Base64 encoded EC2 instance identity document. This needs to be supplied along
with 'signature' parameter.
</li>
</ul>
<ul>
<li>
<span class="param">signature</span>
<span class="param-flags">required</span>
Base64 encoded SHA256 RSA signature of the instance identity document. This
needs to be supplied along with 'identity' parameter.
Base64 encoded SHA256 RSA signature of the instance identity document. This
needs to be supplied along with 'identity' parameter.
</li>
</ul>
<ul>
<li>
<span class="param">pkcs7</span>
<span class="param-flags">required</span>
PKCS7 signature of the identity document with all `\n` characters removed.
Either this needs to be set *OR* both `identity` and `signature` needs to be
set.
PKCS7 signature of the identity document with all `\n` characters removed.
Either this needs to be set *OR* both `identity` and `signature` need to be
set.
</li>
</ul>
<ul>
<li>
<span class="param">nonce</span>
<span class="param-flags">optional</span>
The nonce to be used for subsequent login requests. If this parameter is not
specified at all and if reauthentication is allowed, then the backend will
generate a random nonce, attaches it to the instance's identity-whitelist entry
and returns the nonce back as part of auth metadata. This value should be used
with further login requests, to establish client authenticity. Clients can
choose to set a custom nonce if preferred, in which case, it is recommended
that clients provide a strong nonce. If a nonce is provided but with an empty
value, it indicates intent to disable reauthentication. Note that, when
`disallow_reauthentication` option is enabled on either the role or the role
tag, the `nonce` holds no significance.
The nonce to be used for subsequent login requests. If this parameter is not
specified at all and if reauthentication is allowed, then the backend will
generate a random nonce, attaches it to the instance's identity-whitelist entry
and returns the nonce back as part of auth metadata. This value should be used
with further login requests, to establish client authenticity. Clients can
choose to set a custom nonce if preferred, in which case, it is recommended
that clients provide a strong nonce. If a nonce is provided but with an empty
value, it indicates intent to disable reauthentication. Note that, when
`disallow_reauthentication` option is enabled on either the role or the role
tag, the `nonce` holds no significance.
</li>
</ul>
</dd>