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.

Sign up for GitHub

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

add RP API More Secure Defaults Guidance #566

Draft
wants to merge 1 commit into
base: vNext
Choose a base branch
from
Draft

add RP API More Secure Defaults Guidance #566

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions azure/Guidelines.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,52 @@ In addition to distributed tracing, Azure also uses a set of common correlation
|x-ms-client-request-id |Both |Optional. Caller-specified value identifying the request, in the form of a GUID with no decoration such as curly braces (e.g. `x-ms-client-request-id: 9C4D50EE-2D56-4CD3-8152-34347DC9F2B0`). If the caller provides this header the service **must** include this in their log entries to facilitate correlation of log entries for a single request. Because this header can be client-generated, it should not be assumed to be unique by the service implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

To expand on this (and why it is important for services to not make assumptions about this value) - client request id is often used to group a set of "logical" client requests. Retries will use the same client request id. I believe that tools like the azure cli may use the same client request id for each request made in a single command execution.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Johan here. I consider this header and it's value purely for client benefit. I don't think it has to be a guid. I think a service should specify a max length and perhaps valid characters. But I don't think more than this is required. Also, what is motivating that a service MUST log this value?

|x-ms-request-id |Response |Required. Service generated correlation id identifying the request, in the form of a GUID with no decoration such as curly braces. In contrast to the the `x-ms-client-request-id`, the service **must** ensure that this value is globally unique. Services should log this value with their traces to facilitate correlation of log entries for a single request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I don't see why this must be a guid. A lot of people are using ULIDs instead of UUIDs now and we should allow a service to use whatever value is useful for the service. Other than this, I agree with the rest of this guideline.


<a href="#RP-API-MSD" name="RP-API-MSD"></a>
Copy link

Choose a reason for hiding this comment

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

Rename to a more presentable display name instead of RP-API-MSD?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do RP-specific guidelines go in the data-plane guidelines?
Maybe we want to adopt this for data-plane; I haven't reviewed this PR enough to know.

### RP API More Secure Defaults Guidance
Copy link

Choose a reason for hiding this comment

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

RP -> ARM Resource Provider?

Something like: "### ARM Resource Provider More Secure Default Parameter Guidance"

Copy link
Member

Choose a reason for hiding this comment

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

This is not the right document for ARM API guidelines.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, this document is not the right place for arm/rp guidelines. However, I think adding some of these guidelines to this document for data plane services is perfectly okay


The RP API MSD Guidelines are designed to ensure that any new RP API being developed is deployed from the outset with secure parameter default values. This proactive approach helps prevent non-secure defaults and maintains a high security standard from the very beginning of the API development process.
Copy link

Choose a reason for hiding this comment

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

...either more secure default parameter values or making the previously optional parameter as required"

Copy link
Contributor

@johanste johanste Feb 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Defaulting vs. explicitness is a difficult trade-off.

Default values allow the service to improve its security posture over time with new capabilities without forcing clients to change. Having clients explicitly "pin" values can slow down adoption of new capabilities. Adding new required parameters means breaking changes. Which slows down adoption of new api versions. Which in turn slows down the ability to use the new features.

In practical terms, we need to have a default value for new security related properties when we migrate previously existing resources. That doesn't have to mean that we use the same default values in the public api definitions, but it doesn't seem like an unreasonable thing to do either.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have had numerous conversations about services exposing default values for properties and have been meaning to write a guideline about it. The guideline would go something like this: when a structure/model is first introduced by a service (V1 of structure), very few (if any) of the properties should have default values. Customer must explicitly select values for the properties. This prevents breaking changes should the service want to change a default since there are no defaults. Any new properties that are added to the structure/model in the future (V2+) must have default values in order to avoid breaking changes.


<a href="#rp-parameter-defining" name="rp-parameter-defining">:white_check_mark:</a> **DO** define parameters as explicit as possible:
- Tagging parameters as required or not
Copy link

Choose a reason for hiding this comment

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

...as "required", especially if security-sensitive.

- If a parameter is not tagged as required and it has security implications, define a default value
Copy link

Choose a reason for hiding this comment

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

...define a default value which is deemed as more secure

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say that there just shouldn't be a default and that the property is required - period.

- Defining the default value as a `key: "value"` pair
Copy link

Choose a reason for hiding this comment

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

  • Providing a concise but descriptive description of the parameters in the API

Copy link
Contributor

Choose a reason for hiding this comment

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

I just want to be clear that changing a default value is a breaking change and requires approval from the breaking change review board. This can all be avoided by not having defaults and instead have customers explicitly select what they desire.


Example:

```json
"parameters": [
{
"name": "id",
"in": "path",
"required": true,
"type": "string"
},
{
"name": "requiredParam",
"in": "query",
"required": true,
"type": "string"
},
{
"name": "optionalParam",
"in": "query",
"required": false,
"type": "string"
},
{
"name": "paramWithDefault",
"in": "query",
"required": false,
"type": "string",
"default": "defaultValue"
}
],
```

<a href="#security-parameters-handling" name="security-parameters-handling">:ballot_box_with_check:</a> **YOU SHOULD** consider tagging a security parameter as required. If making it required is not feasible, refer to the Microsoft Cloud Security Benchmark in order to assign a meaningful value to avoid security gaps. If you need to deviate from this default value, discuss matter within your team and with the API Board to ensure awareness and understanding of the parameter.
Copy link
Member

Choose a reason for hiding this comment

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

What is a "security parameter"?


**Note**: Make sure the documentation and the spec files from [GitHub - Azure/azure-rest-api-specs: The source for REST API specifications for Microsoft Azure.](https://github.com/Azure/azure-rest-api-specs/tree/main) match and are kept up to date.

## Final thoughts
These guidelines describe the upfront design considerations, technology building blocks, and common patterns that Azure teams encounter when building an API for their service. There is a great deal of information in them that can be difficult to follow. Fortunately, at Microsoft, there is a team committed to ensuring your success.

Expand Down