RE: #390: More redactable properties

… and after tripping over a subtlety in the spec, and discussing it with Michael, I revised it again ( same link ).

The problem is this: the spec says that if an
artifactLocation  represents a nested artifact whose location within its parent  can only be specified by a byte offset and length  – that is, NOT by a path – then  uri
SHALL  be absent.

If we were now to specify that when
uri  is absent, it defaults to  "." , then an SDK would see  "."  as the
uri  for such a nested file, despite the fact that that nested file doesn’t  have  a URI.

Our solution is  not  to specify a default for
uri . If it’s absent, it’s absent. That does mean we have to add another special case to the part of the spec that says when  artifactLocation.uri  is and isn’t required: it’s not required if the  artifactLocation  object is a top-level value in  originalUriBaseIds . Michael and I feel we can accept that.

Larry
From:   <>  On Behalf Of  Larry Golding (Myriad Consulting Inc)

Sent:  Friday, April 26, 2019 4:57 PM
To:  OASIS SARIF TC Discussion List <>
Subject:  [sarif] RE: #390: More redactable properties

The
issue  is reactivated. Here is the
REVISED PROPOSAL .

Larry
From:  Larry Golding (Myriad Consulting Inc) < 
>
Sent:  Friday, April 26, 2019 4:42 PM
To:  Larry Golding (Myriad Consulting Inc) <  >; OASIS SARIF TC Discussion List < 
>
Subject:  RE: #390: More redactable properties

I should point out that there is no danger of thinking that the value of  PROJECTROOT  in that example actually
is
"." . That’s because the spec requires a top-level  uriBaseId  to be an absolute URI. When the SDK sees that it’s  "."  (coming from the default value), it will know it’s not the “real” value, and that the real value has been removed.

Larry
From:
  < 
>  On Behalf Of  Larry Golding (Myriad Consulting Inc)

Sent:  Friday, April 26, 2019 4:38 PM
To:  OASIS SARIF TC Discussion List < 
>
Subject:  [sarif] RE: #390: More redactable properties

After further discussion, Michael, Jim, and I still agree that allowing URI-valued properties to be redacted is too burdensome for SARIF consumers. Consumers would always need to be aware that any URI they handle might be redacted, and  so might not actually point to a file.

Furthermore, we already have a mechanism that allows the leading portion of a URI to be stripped off:  artifactLocation.uriBaseId  and
run.originalUriBaseIds . It is after all the leading portion of the URI, for example
file:///C:/Users/Larry  , that is most likely to contain security-sensitive information such as a host name or a user account name.

We can, however, make  originalUriBaseIds  a little more useful in the redaction scenario. Today, if you want to remove the sensitive information in  originalUriBaseIds , you have to remove it entirely, because  artifactLocation.uri  is required. That is, if you want to sanitize this:

"originalUriBaseIds": {
 "PROJECTROOT": {
    "uri": " file:///C:/Users/Larry/code/OurProject ",
    "description": "The root directory for all project files."
 },
  "SRCROOT": {
    "uri": "src",
    "uriBaseId": "PROJECTROOT",
    "description": "The root of the source tree."
  }
}

… then you have to do this:

"originalUriBaseIds": {
  "SRCROOT": {
    "uri": "src",
    "uriBaseId": "PROJECTROOT",
    "description": "The root of the source tree."
  }
}

… because you’re not allowed to do this:

"originalUriBaseIds": {
 "PROJECTROOT": {
    "description": "The root directory for all project files. (Look, no uri!)"
 },
  "SRCROOT": {
    "uri": "src",
    "uriBaseId": "PROJECTROOT",
    "description": "The root of the source tree."
  }
}

So we propose to make  artifactLocation.uri  optional, defaulting to  "." . Then we can write that last version, getting the security benefit of this kind of redaction, while still getting the documentation for the uriBaseId value.

I am going to take the unusual step of moving this morning’s change draft to the  Rejected  folder and sending out a new one. I will reactivate and update the issue.

Thanks,
Larry
From:  Larry Golding (Myriad Consulting Inc) < 
>
Sent:  Friday, April 26, 2019 11:21 AM
To:  Larry Golding (Myriad Consulting Inc) <  >; OASIS SARIF TC Discussion List < 
>
Subject:  RE: #390: More redactable properties

Late breaking: It turns out that we cannot allow URI-valued properties to be redactable.

The reason is that it would require a schema change to remove the constraints that these properties conform to the URI format. This in turn badly breaks the SDK (which declares these properties as System.Uri) in a way that we simply cannot  absorb.

This means:
A couple of the properties that Yekaterina asked to be redactable, such as invocation.workingDirectory, cannot be. This is not a serious problem because uriBaseIds can be used to hide  the sensitive leading portion of the URI (or the property can be omitted entirely).
Jim’s Issue #377, which concerned the treatment of URIs with redacted path segments, becomes moot.

Sorry for the churn. I will revise the provisional draft and close #377.

Thanks,
Larry
From:
  < 
>  On Behalf Of  Larry Golding (Myriad Consulting Inc)

Sent:  Friday, April 26, 2019 11:02 AM
To:  OASIS SARIF TC Discussion List < 
>
Subject:  [sarif] #390: More redactable properties
Importance:  High

I created and merged a change draft for Yekaterina’s
Issue #390 , “Make certain invocation and versionControlDetails properties redactable”:

https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ChangeDrafts/Accepted/sarif-v2.0-issue-390-more-redactable-properties.docx

Among the newly redactable properties are the values  and names  of the properties in  invocation.environmentVariables . But this requires that the string-valued  run.redactionToken  become an array
redactionTokens , because otherwise two redacted environment variable names would have the same JSON property name, which is not allowed.

The good news is that we were going to have to do this anyway to accommodate Jim’s
Issue #377 , “Each redaction token in an originalUriBaseIds represents a unique location,” so now we have two reasons to do it.

Furthermore, this change requires that we clarify what it means to redact a URI-valued property (because the result might not be a valid URI). This too was going to be needed for #377, so the good news is, I’ve got a big head start on writing  the draft for #377.

Please take a look  as we close down the spec today in preparation for Monday’s ballot.

Thanks,
Larry