ietf-rats-wg / architecture

RATS Architecture
Other
17 stars 10 forks source link

Missing arrow in Figure 2? #344

Closed thomas-fossati closed 2 years ago

thomas-fossati commented 3 years ago

ISTM that the Attesting Environment in Figure 2 should also have an (optional) inbound arrow for Evidence from another Attesting Enviroment, maybe a sub-Attester. Otherwise the arrows from Attesters B & C into the lead Attesting Environment shown in Figure 4 might come a bit as a surprise. (And the arrow labelled "Evidence for bootloader" in Figure 3 too).

nedmsmith commented 3 years ago

I'm indifferent as to whether it needs fixing. The fact that someone raised a question suggests it isn't clear. Someone else however could look at the new content and be confused by it as well. Though, I generally error on the side of inclusion so long as the additional detail is correct and doesn't bias toward a particular implementation.

mcr commented 3 years ago

I don't think the original diagram needs to be fixed. It might be that a second diagram is needed somewhere else, but I think that changing this diagram will cause more confusion.

What I think we are doing is being overly mathematical, and thinking that the Architecture document is trying replace all the documents and work that has gone before, or that it is a Design Requirements document.

thomas-fossati commented 3 years ago

I don't think the original diagram needs to be fixed.

@laurencelundblade too, independently, got confused by the current layout in the figures -- see 2nd para of https://mailarchive.ietf.org/arch/msg/rats/UuFwOfLnRitCbnfKpE-IgZ9ANcU/

So there seems to be some kind of problem with it.

It might be that a second diagram is needed somewhere else, but I think that changing this diagram will cause more confusion.

I am not sure multiplying the number of pics would fix what seems to be like a tiny lack of internal coherency in what we currently have.

henkbirkholz commented 2 years ago

@thomas-fossati, if this is not resolved, could you fix that "tiny lack of internal coherency"?

thomas-fossati commented 2 years ago

hi @henkbirkholz ! #347 already fixes said "tiny lack of internal coherency"

henkbirkholz commented 2 years ago

Well, the PR outlived auto-merge... so what about the rest of PR, is that resolved? I'll add a fixes to that one so this here auto-closes.

thomas-fossati commented 2 years ago

[...] what about the rest of PR, is that resolved?

PR #347 fully resolves this issue.

mcr commented 2 years ago

Consensus to add a new diagram, at the place where the external attester is first mentioned.

thomas-fossati commented 2 years ago

@mcr, all: I have been staring at sections 3.1 and §3.2 (and in between) for a good 15 minutes now without being able to generate a satisfactory solution.

I have decided that the document is so good that it can come out of this "tiny lack of internal coherency" unscathed. I'm happy to withdraw the issue.

mcr commented 2 years ago

pull request abandoned by consensus of design team.