EAD zeroconf: send full CRED_V in message_2 #117
Conversation
geonnave
force-pushed
the
flexible-creds
branch
from
e6d443a to
802806f
Compare
geonnave
force-pushed
the
flexible-creds
branch
4 times, most recently
from
bc66f7d to
06e1028
Compare
also refactor/simplify credential handling
geonnave
force-pushed
the
flexible-creds
branch
from
06e1028 to
be1ae2a
Compare
| @@ -36,7 +36,7 @@ jobs: | |||
| uses: actions/checkout@v3 | |||
|
|
|||
| - name: Run unit tests # note that we only add `--package edhoc-hacspec` when testing the hacspec version of the lib | |||
| run: cargo test --package edhoc-rs --package edhoc-crypto --package edhoc-ead --no-default-features --features="${{ matrix.crypto_backend }}, ${{ matrix.ead }}" --no-fail-fast | |||
| run: RUST_BACKTRACE=1 cargo test -p edhoc-rs -p edhoc-consts -p edhoc-ead-zeroconf --no-default-features --features="${{ matrix.crypto_backend }}, ${{ matrix.ead }}" --no-fail-fast -- --test-threads 1 | |||
There was a problem hiding this comment.
Why replacing edhoc-ead with edhoc-ead-zeroconf?
consts/src/lib.rs
Outdated
|
|
||
| pub fn parse_cred<'a>(cred: &'a [u8]) -> (BytesP256ElemLen, u8) { | ||
| let subject_len = (cred[2] - CBOR_MAJOR_TEXT_STRING) as usize; | ||
| let id_cred_offset: usize = 3 + subject_len + 8; |
There was a problem hiding this comment.
comment on magic numbers, ideally replace them with a macro
| ) | ||
| } | ||
|
|
||
| pub fn get_id_cred<'a>(cred: &'a [u8]) -> BytesIdCred { |
There was a problem hiding this comment.
Note to self: To verify if this way of constructing ID_CRED from CRED is fine.
There was a problem hiding this comment.
For context: it is hardcoded and assumes that ID_CRED is a kid in a COSE header map.
| @@ -281,4 +303,231 @@ mod helpers { | |||
|
|
|||
| (info, info_len) | |||
| } | |||
|
|
|||
| pub fn parse_cred<'a>(cred: &'a [u8]) -> (BytesP256ElemLen, u8) { | |||
There was a problem hiding this comment.
Please comment on the assumptions of this implementation
lib/src/edhoc.rs
Outdated
| id_cred_r = IdCred::FullCredential(&plaintext_2[4..4 + cred_len]); | ||
| 4 + cred_len | ||
| } else { | ||
| 0xff |
There was a problem hiding this comment.
Let's return an Error here instead of 0xff
| c_r = c_r_2; | ||
|
|
||
| // Step 3: If EAD is present make it available to the application | ||
| let ead_success = if let Some(ead_2) = ead_2 { | ||
| i_process_ead_2(ead_2, cred_r_expected, &h_message_1).is_ok() | ||
| let (ead_ok, r_authenticated_via_ead, cred_r) = if let Some(ead_2) = ead_2 { |
There was a problem hiding this comment.
If possible, rewrite to be more explicit, as discussed offline
| let ead_success = if let Some(ead_2) = ead_2 { | ||
| i_process_ead_2(ead_2, cred_r_expected, &h_message_1).is_ok() | ||
| let (ead_ok, r_authenticated_via_ead, cred_r) = if let Some(ead_2) = ead_2 { | ||
| // if EAD-zeroconf is present, then id_cred must contain a full credential |
There was a problem hiding this comment.
Ideally, this should check if the EAD handler is the zero touch draft handler. No action needed at this point, let's track.
| ID_CRED_R, | ||
| CRED_R, | ||
| ); | ||
| let mut responder = EdhocResponderState::new(state_responder, R, CRED_V_TV, Some(CRED_I)); |
There was a problem hiding this comment.
We currently instantiate the responder with CRED_I of the Initiator. This should ideally be None and the responder should fetch, for the time being through a mock up, the value of the CRED_I based on ID_CRED_I.
Included in this PR:
Sending CRED_R in message_3 would also make sense, but at this point we still don't need it.